Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: change container create output #1034

Merged
merged 1 commit into from
Apr 3, 2018
Merged

bugfix: change container create output #1034

merged 1 commit into from
Apr 3, 2018

Conversation

ZouRui89
Copy link
Contributor

@ZouRui89 ZouRui89 commented Apr 2, 2018

Signed-off-by: Zou Rui [email protected]

Ⅰ. Describe what this PR did

change the output of container create
old way:

#pouch create --cpuset-cpus=0-1 --memory=4m --memory-swap 4m busybox top
container ID: c7f1fc453f6efbbd400e7d587d1277cc00a52efc893ee9c15cd2759cf71e049d, name: c7f1fc
now:
#pouch create --cpuset-cpus=0-1 --memory=4m --memory-swap 4m busybox top
c7f1fc453f6efbbd400e7d587d1277cc00a52efc893ee9c15cd2759cf71e049d

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@pouchrobot pouchrobot added kind/bug This is bug report for project size/XS labels Apr 2, 2018
@rudyfly
Copy link
Collaborator

rudyfly commented Apr 2, 2018

LGTM

@pouchrobot pouchrobot added LGTM one maintainer or community participant agrees to merge the pull reuqest. size/S and removed size/XS labels Apr 2, 2018
@Letty5411
Copy link
Contributor

CI failed, you may also need to change the corresponding tests:

FAIL: /go/src/github.com/alibaba/pouch/test/cli_create_test.go:37: PouchCreateSuite.TestCreateName


/go/src/github.com/alibaba/pouch/test/cli_create_test.go:43:

    c.Fatalf("unexpected output %s expected %s\n", out, name)

... Error: unexpected output f47c9360281257ce6fd593c31dbd87c59ff24a48192e33d403c44424d26469d3 expected create-normal




----------------------------------------------------------------------

FAIL: /go/src/github.com/alibaba/pouch/test/cli_create_test.go:50: PouchCreateSuite.TestCreateNameByImageID


/go/src/github.com/alibaba/pouch/test/cli_create_test.go:61:

    c.Fatalf("unexpected output %s expected %s\n", out, name)

... Error: unexpected output d50978a316442edc09a8a023a125c37358721118c7784ff9cab466ef3400c402 expected create-normal-by-image-id

@codecov-io
Copy link

codecov-io commented Apr 3, 2018

Codecov Report

Merging #1034 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1034   +/-   ##
=======================================
  Coverage   15.71%   15.71%           
=======================================
  Files         139      139           
  Lines        8483     8483           
=======================================
  Hits         1333     1333           
  Misses       7050     7050           
  Partials      100      100
Impacted Files Coverage Δ
cli/create.go 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46c67ec...58d04c9. Read the comment docs.

res := command.PouchRun("create", "--name", name, busyboxImage)

res.Assert(c, icmd.Success)
if out := res.Combined(); !strings.Contains(out, name) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to remove all the test case. Maybe we just remove the string comparing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds reasonable. Thx.

res = command.PouchRun("create", "--name", name, imageID)

res.Assert(c, icmd.Success)
if out := res.Combined(); !strings.Contains(out, name) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as the following review comment.

@allencloud
Copy link
Collaborator

LGTM

@allencloud allencloud merged commit 126105b into AliyunContainerService:master Apr 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug This is bug report for project LGTM one maintainer or community participant agrees to merge the pull reuqest. size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants