test: enhance cli related tests #1341

Letty5411 · 2018-05-17T03:25:49Z

Signed-off-by: letty letty.ll@alibaba-inc.com

Ⅰ. Describe what this PR did

This PR mainly enhances code in cli_xxx_test.go in the following two aspects:

using icmd.result.Stderr() instead of icmd.result.Error to print readable error message when stderr is not nil. Please notice that, if you wanna check running CMD return success, please user icmd.result.Assert() instead of using check.Assert(icmd.result.Error, check.IsNil). Because the following usage will not print readable error message.
using defer to do cleanup work. When create a container or volume or network, following a defer to make sure the resource will always be deleted even if test failed.

Ⅱ. Does this pull request fix one issue?

fix #1323

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Signed-off-by: letty <letty.ll@alibaba-inc.com>

codecov-io · 2018-05-17T05:26:00Z

Codecov Report

Merging #1341 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1341   +/-   ##
=======================================
  Coverage   17.32%   17.32%           
=======================================
  Files         189      189           
  Lines       11818    11818           
=======================================
  Hits         2048     2048           
- Misses       9622     9623    +1     
+ Partials      148      147    -1

Impacted Files	Coverage Δ
daemon/mgr/container.go	`0% <0%> (ø)`	⬆️
daemon/logger/jsonfile/utils.go	`70% <0%> (ø)`	⬆️

allencloud · 2018-05-17T05:44:48Z

LGTM

fuweid · 2018-05-17T05:44:38Z

test/cli_create_test.go

@@ -40,14 +40,13 @@ func (suite *PouchCreateSuite) TearDownTest(c *check.C) {
 func (suite *PouchCreateSuite) TestCreateName(c *check.C) {
 	name := "create-normal"
 	res := command.PouchRun("create", "--name", name, busyboxImage)
-
+	defer DelContainerForceMultyTime(c, name)
 	res.Assert(c, icmd.Success)

 	// create command should add newline at the end of result
 	digStr := strings.TrimSpace(res.Combined())
 	c.Assert(res.Combined(), check.Equals, fmt.Sprintf("%s\n", digStr))



please remove the blank line at the end of function

fuweid · 2018-05-17T05:46:02Z

test/cli_create_test.go

@@ -40,14 +40,13 @@ func (suite *PouchCreateSuite) TearDownTest(c *check.C) {
 func (suite *PouchCreateSuite) TestCreateName(c *check.C) {
 	name := "create-normal"
 	res := command.PouchRun("create", "--name", name, busyboxImage)
-
+	defer DelContainerForceMultyTime(c, name)


I think we should put the defer DelContainerForceMultyTime(c, name) after the res.Assert, because we can identify the issue if the create can still create container in failure.

commit 75aab2b Merge: 5c8993b 2f2e9c1 Author: Starnop <starnop@163.com> Date: Sat May 19 21:51:30 2018 +0800 Merge branch 'cri-compatibility' of github.com:Starnop/pouch into cri-compatibility commit 2f2e9c1 Merge: bd9c4fd acbd19a Author: Starnop <starnop@163.com> Date: Fri May 18 15:32:31 2018 +0800 Merge branch 'master' of https://github.com/alibaba/pouch into cri-compatibility commit acbd19a Merge: 6cef601 8e862f9 Author: Yao Zengzeng <yaozengzeng@zju.edu.cn> Date: Fri May 18 15:27:01 2018 +0800 Merge pull request AliyunContainerService#1351 from fuweid/feature_allow_use_image_by_digest_id feature: allow use image by digest id commit 6cef601 Merge: 4423313 caf45ec Author: Wei Fu <fhfuwei@163.com> Date: Fri May 18 15:10:30 2018 +0800 Merge pull request AliyunContainerService#1350 from Ace-Tang/version build: generate version information at build time commit caf45ec Author: Ace-Tang <aceapril@126.com> Date: Fri May 18 13:31:25 2018 +0800 build: generate version information at build time auto-generate git commit, build time at build time, fix go-version hard code. Signed-off-by: Ace-Tang <aceapril@126.com> commit 8e862f9 Author: Wei Fu <fhfuwei@163.com> Date: Fri May 18 13:50:42 2018 +0800 feature: allow use image by digest id Basically, the user can use sha256:xyz to inspect image or run container. Signed-off-by: Wei Fu <fhfuwei@163.com> commit 4423313 Merge: 8410064 f67cf08 Author: Allen Sun <shlallen1990@gmail.com> Date: Fri May 18 11:44:39 2018 +0800 Merge pull request AliyunContainerService#1326 from shaloulcy/core_volume feature: remote volume driver commit 8410064 Merge: 8af60a9 852d4f4 Author: Wei Fu <fhfuwei@163.com> Date: Fri May 18 10:39:15 2018 +0800 Merge pull request AliyunContainerService#1339 from rhinoceros/master docs: Update INSTALLATION.md apt-key fingerprint BE2F475F commit f67cf08 Author: Eric Li <lcy041536@gmail.com> Date: Mon May 14 12:19:46 2018 +0800 feature: remote volume driver Signed-off-by: Eric Li <lcy041536@gmail.com> commit bd9c4fd Merge: 8d01e30 521ca7c Author: Starnop <starnop@163.com> Date: Thu May 17 11:11:40 2018 +0800 Merge branch 'master' of https://github.com/alibaba/pouch into cri-compatibility commit 8af60a9 Merge: ecbe3b6 13a17b5 Author: Allen Sun <shlallen1990@gmail.com> Date: Fri May 18 09:43:40 2018 +0800 Merge pull request AliyunContainerService#1349 from pouchrobot/auto-doc-2018-05-18 docs: auto generate pouch cli/api docs via code commit 13a17b5 Author: pouchrobot <pouch-dev@list.alibaba-inc.com> Date: Fri May 18 01:28:51 2018 +0000 docs: auto generate pouch cli docs via code Signed-off-by: pouchrobot <pouch-dev@list.alibaba-inc.com> commit 5c8993b Merge: 8d01e30 521ca7c Author: Starnop <starnop@163.com> Date: Thu May 17 11:11:40 2018 +0800 Merge branch 'master' of https://github.com/alibaba/pouch into cri-compatibility commit ecbe3b6 Merge: 3374daf 7fc11df Author: Wei Fu <fhfuwei@163.com> Date: Thu May 17 18:53:20 2018 +0800 Merge pull request AliyunContainerService#1227 from Ace-Tang/full_spec_params feature: add pidslimit implement commit 7fc11df Author: Ace-Tang <aceapril@126.com> Date: Fri May 11 16:25:32 2018 +0800 feature: add pidslimit implement Signed-off-by: Ace-Tang <aceapril@126.com> commit 3374daf Merge: 89a5ae5 fd8e0f7 Author: Allen Sun <shlallen1990@gmail.com> Date: Thu May 17 13:44:52 2018 +0800 Merge pull request AliyunContainerService#1341 from Letty5411/0517-assertfix test: enhance cli related tests commit 89a5ae5 Merge: 521ca7c 8469cea Author: Wei Fu <fhfuwei@163.com> Date: Thu May 17 13:17:47 2018 +0800 Merge pull request AliyunContainerService#1331 from Letty5411/0516-doc doc: update test.md about how to run test commit fd8e0f7 Author: letty <letty.ll@alibaba-inc.com> Date: Thu May 17 11:23:16 2018 +0800 test: enhance test Signed-off-by: letty <letty.ll@alibaba-inc.com> commit 521ca7c Merge: 19c956b 6b2a8b4 Author: Yao Zengzeng <yaozengzeng@zju.edu.cn> Date: Thu May 17 10:39:06 2018 +0800 Merge pull request AliyunContainerService#1340 from ZouRui89/race_off bugfix: move IO part behind to ensure execConfig result assignment before IO closes commit 6b2a8b4 Author: Zou Rui <21751189@zju.edu.cn> Date: Wed May 16 18:03:27 2018 +0800 bugfix: move IO part behind to ensure execConfig result assignment before IO closes Signed-off-by: Zou Rui <21751189@zju.edu.cn> commit 8d01e30 Merge: 9339a70 19c956b Author: Starnop <starnop@163.com> Date: Wed May 16 12:57:14 2018 +0800 Merge branch 'master' of https://github.com/alibaba/pouch into cri-compatibility commit 852d4f4 Author: rhinoceros.xn <rhinoceros.xn@gmail.com> Date: Wed May 16 17:37:51 2018 +0800 Update INSTALLATION.md $ apt-key fingerprint BE2F475F pub 4096R/BE2F475F 2018-02-28 Key fingerprint = F443 EDD0 4A58 7E8B F645 9C40 CF68 F84A BE2F 475F uid opsx-admin <opsx@service.alibaba.com> commit 8469cea Author: letty <letty.ll@alibaba-inc.com> Date: Wed May 16 10:08:32 2018 +0800 doc: update test.md about how to run test Signed-off-by: letty <letty.ll@alibaba-inc.com> commit 9339a70 Author: Starnop <starnop@163.com> Date: Tue May 15 17:35:53 2018 +0800 cri com and up

pouchrobot added areas/test size/XL labels May 17, 2018

test: enhance test

fd8e0f7

Signed-off-by: letty <letty.ll@alibaba-inc.com>

Letty5411 force-pushed the 0517-assertfix branch from 1ee7c56 to fd8e0f7 Compare May 17, 2018 05:11

Letty5411 changed the title ~~[WIP] test: enhance test~~ test: enhance cli related tests May 17, 2018

allencloud merged commit 3374daf into AliyunContainerService:master May 17, 2018

pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label May 17, 2018

fuweid reviewed May 17, 2018

View reviewed changes

pouchrobot mentioned this pull request May 17, 2018

Weekly Report of Pouch from 2018-05-10 to 2018-05-17 #1345

Closed

pouchrobot mentioned this pull request May 24, 2018

Weekly Report of Pouch from 2018-05-17 to 2018-05-24 #1408

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: enhance cli related tests #1341

test: enhance cli related tests #1341

Letty5411 commented May 17, 2018 •

edited

Loading

codecov-io commented May 17, 2018

allencloud commented May 17, 2018

fuweid May 17, 2018

fuweid May 17, 2018

test: enhance cli related tests #1341

test: enhance cli related tests #1341

Conversation

Letty5411 commented May 17, 2018 • edited Loading

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov-io commented May 17, 2018

Codecov Report

allencloud commented May 17, 2018

fuweid May 17, 2018

Choose a reason for hiding this comment

fuweid May 17, 2018

Choose a reason for hiding this comment

Letty5411 commented May 17, 2018 •

edited

Loading