Skip to content

Conversation

@ZYecho
Copy link
Contributor

@ZYecho ZYecho commented Nov 25, 2018

Signed-off-by: zhangyue [email protected]

Ⅰ. Describe what this PR did

Add a util func to refactor volume test to make test code more readable

Ⅱ. Does this pull request fix one issue?

None.

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

None.

Ⅳ. Describe how to verify it

go test -gocheck.f PouchVolumeSuite

Ⅴ. Special notes for reviews

None.

@codecov
Copy link

codecov bot commented Nov 25, 2018

Codecov Report

Merging #2496 into master will decrease coverage by 16.53%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #2496       +/-   ##
===========================================
- Coverage   69.31%   52.78%   -16.54%     
===========================================
  Files         278      278               
  Lines       18387    18387               
===========================================
- Hits        12745     9705     -3040     
- Misses       4213     7598     +3385     
+ Partials     1429     1084      -345
Flag Coverage Δ
#criv1alpha1test ?
#criv1alpha2test 35.52% <ø> (+0.05%) ⬆️
#integrationtest ?
#nodee2etest 32.79% <ø> (-0.05%) ⬇️
#unittest 26.6% <ø> (-0.17%) ⬇️
Impacted Files Coverage Δ
cri/v1alpha1/cri_types.go 0% <0%> (-100%) ⬇️
pkg/httputils/http_utils.go 0% <0%> (-100%) ⬇️
daemon/logger/logmessage.go 0% <0%> (-100%) ⬇️
hookplugins/containerplugin/update_hook.go 0% <0%> (-100%) ⬇️
pkg/httputils/http_error.go 0% <0%> (-100%) ⬇️
apis/server/container_bridge.go 0% <0%> (-87.43%) ⬇️
apis/server/volume_bridge.go 0% <0%> (-86.67%) ⬇️
cri/v1alpha1/service/cri.go 0% <0%> (-84.62%) ⬇️
cri/v1alpha1/server.go 0% <0%> (-79.2%) ⬇️
apis/server/utils.go 3.84% <0%> (-71.16%) ⬇️
... and 87 more

@ZYecho
Copy link
Contributor Author

ZYecho commented Nov 25, 2018

linter fails with

Build-agent version 0.1.1073-1f69f340 (2018-11-20T18:07:03+0000)
Starting container pouchcontainer/pouchlinter:v0.1.2
  image cache not found on this host, downloading pouchcontainer/pouchlinter:v0.1.2

@allencloud
Copy link
Collaborator

allencloud commented Nov 25, 2018

I have rerun the linter CI. The linter CI passes, and while the travisCI fails:

----------------------------------------------------------------------
FAIL: /home/travis/gopath/src/github.com/alibaba/pouch/test/cli_rmi_test.go:68: PouchRmiSuite.TestRmiByImageDigestID
/home/travis/gopath/src/github.com/alibaba/pouch/test/cli_rmi_test.go:69:
    command.PouchRun("pull", helloworldImage).Assert(c, icmd.Success)
/home/travis/gopath/src/github.com/alibaba/pouch/vendor/github.com/gotestyourself/gotestyourself/icmd/command.go:61:
    t.Fatalf("at %s:%d - %s\n", filepath.Base(file), line, err.Error())
... Error: at cli_rmi_test.go:69 - 
Command:  /usr/local/bin/pouch pull registry.hub.docker.com/library/hello-world:linux
ExitCode: 1
Error:    exit status 1
Stdout:   registry.hub.docker.com/library/hello-world:linux: resolving
Stderr:   Error: failed to display progress: failed to pull image: registry.hub.docker.com/library/hello-world:linux not found
Failures:
ExitCode was 1 expected 0
Expected no error
----------------------------------------------------------------------

and

----------------------------------------------------------------------
FAIL: /home/travis/gopath/src/github.com/alibaba/pouch/test/api_checkpoint_test.go:18: APIContainerCheckpointSuite.SetUpTest
/home/travis/gopath/src/github.com/alibaba/pouch/test/api_checkpoint_test.go:20:
    PullImage(c, busyboxImage)
/home/travis/gopath/src/github.com/alibaba/pouch/test/util_api.go:281:
    c.Assert(discardPullStatus(resp.Body), check.IsNil)
... value *jsonstream.JSONError = &jsonstream.JSONError{Code:500, Message:"failed to pull image: httpReaderSeeker: failed open: could not fetch content descriptor sha256:141c253bc4c3fd0a201d32dc1f493bcf3fff003b6df416dea4f41046e0f37d47 (application/vnd.docker.distribution.manifest.list.v2+json) from remote: not found"} ("failed to pull image: httpReaderSeeker: failed open: could not fetch content descriptor sha256:141c253bc4c3fd0a201d32dc1f493bcf3fff003b6df416dea4f41046e0f37d47 (application/vnd.docker.distribution.manifest.list.v2+json) from remote: not found")
----------------------------------------------------------------------
PANIC: /home/travis/gopath/src/github.com/alibaba/pouch/test/api_checkpoint_test.go:35: APIContainerCheckpointSuite.TestCheckpointCreateAPI
... Panic: Fixture has panicked (see related PANIC)
----------------------------------------------------------------------

Is the checkpoint part related to your CRIU part? @Ace-Tang

I will re-run travis CI to check if this is a flaky test. @ZYecho

lines := volumesToKV(ret.Stdout())
for _, line := range lines {
c.Assert(line[0], check.Equals, "local")
if !strings.Contains(line[3], DefaultVolumeMountPath) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think when you were refactoring this part, you thought of add more checking rules which is described in #2497 😄 @ZYecho

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, check.Contains is really helpful.

Signed-off-by: zhangyue <[email protected]>
@ZYecho ZYecho force-pushed the refactor-volume-test branch from e97e0b4 to 3fa4039 Compare November 27, 2018 07:06
@allencloud
Copy link
Collaborator

Never mind the linkcheck failure:

FILE: ./docs/features/pouch_with_criu.md
[✖] https://criu.org/Main_Page
[✖] https://criu.org/Linux_kernel
[✖] https://criu.org/Packages

ERROR: dead links found!

@allencloud
Copy link
Collaborator

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Nov 28, 2018
@allencloud allencloud merged commit ef0efaa into AliyunContainerService:master Nov 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

areas/storage areas/test kind/refactor LGTM one maintainer or community participant agrees to merge the pull reuqest. size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants