Skip to content

integration/*: make e2e run without failure#36594

Merged
vdemeester merged 1 commit into
moby:masterfrom
vdemeester:e2e-more-run
Mar 21, 2018
Merged

integration/*: make e2e run without failure#36594
vdemeester merged 1 commit into
moby:masterfrom
vdemeester:e2e-more-run

Conversation

@vdemeester
Copy link
Copy Markdown
Member

@vdemeester vdemeester commented Mar 14, 2018

… mainly by skipping if daemon is remote.

docker build -f Dockerfile.e2e -t moby-e2e .
docker run -v /var/run/docker.sock:/var/run/docker.sock \
           -e DOCKER_API_VERSION=1.36 moby-e2e
# […]
PASS
# […]

🦁

Next is the integration-cli runs: currently it's 1017 passed, 570 skipped, 38 FAILED.

cc @chris-crone

Signed-off-by: Vincent Demeester vincent@sbr.pm

@vdemeester vdemeester requested a review from tianon as a code owner March 14, 2018 16:34
thaJeztah
thaJeztah requested changes Mar 14, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

Not fully reviewed the changes, but just some quick things I noticed 😅

Comment thread integration/build/build_test.go Outdated
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah Mar 14, 2018

Choose a reason for hiding this comment

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

Can you group this with other imports?

Copy link
Copy Markdown
Member Author

@vdemeester vdemeester Mar 14, 2018

Choose a reason for hiding this comment

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

🙇‍♂️

Comment thread integration/internal/swarm/service.go Outdated
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah Mar 14, 2018

Choose a reason for hiding this comment

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

Can you group this with other imports?

Copy link
Copy Markdown
Member Author

@vdemeester vdemeester Mar 14, 2018

Choose a reason for hiding this comment

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

🙇‍♂️

Comment thread hack/test/e2e-run.sh Outdated
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah Mar 14, 2018

Choose a reason for hiding this comment

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

Bit confused by this one; this forces the directory to be at the root?

Copy link
Copy Markdown
Member Author

@vdemeester vdemeester Mar 14, 2018

Choose a reason for hiding this comment

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

yeah that's how the e2e image is set-up.. I could change that though 😅

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah Mar 14, 2018

Choose a reason for hiding this comment

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

Yeah, I think it should be flexible, and not rely on a fixed path (ideally), perhaps either an env-var, or relative to where the script is run from?

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah Mar 16, 2018

Choose a reason for hiding this comment

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

not a blocker for now (this is only used for the e2e tests)

Comment thread integration/plugin/authz/main_test.go Outdated
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah Mar 14, 2018

Choose a reason for hiding this comment

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

Can you group this with other imports?

Copy link
Copy Markdown
Member Author

@vdemeester vdemeester Mar 14, 2018

Choose a reason for hiding this comment

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

🙇‍♂️

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 14, 2018 •
edited
Loading

Codecov Report

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

@@            Coverage Diff             @@
##           master   #36594      +/-   ##
==========================================
- Coverage   34.85%   34.81%   -0.04%     
==========================================
  Files         612      612              
  Lines       45445    45720     +275     
==========================================
+ Hits        15838    15917      +79     
- Misses      27537    27700     +163     
- Partials     2070     2103      +33

yongtang
yongtang approved these changes Mar 14, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@yongtang yongtang left a comment

Choose a reason for hiding this comment

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

LGTM

@kolyshkin kolyshkin mentioned this pull request Mar 16, 2018
Closed
thaJeztah
thaJeztah approved these changes Mar 16, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@vdemeester
integration/*: make e2e run without failure
6016e79 
… mainly by skipping if daemon is remote.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
@vdemeester vdemeester mentioned this pull request Mar 19, 2018
Merged
@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Mar 19, 2018

odd one on experimental https://jenkins.dockerproject.org/job/Docker-PRs-experimental/39863/console

09:35:34 2018/03/19 09:35:34 http2: server: error reading preface from client /go/src/github.com/docker/docker/bundles/test-integration/docker.sock: bogus greeting "\x00\x00\x00\x04\x01\x00\x00\x00\x00\x00\x00L\x01\x04\x00\x00\x00\x01\x83\x86E\x9ab\x93"
09:35:34 
09:35:34 ----------------------------------------------------------------------
09:35:34 FAIL: docker_api_build_test.go:518: DockerSuite.TestBuildWithSession
09:35:34 
09:35:34 assertion failed: 
09:35:34 {int}:
09:35:34 	-: 500
09:35:34 	+: 200
09:35:34 
09:35:34 assertion failed: string "{\"message\":\"failed to copy to /var/lib/docker/builder/731e9f77f1ff3a4c44cecce0f0aea695d12aec6ecbad1acb487e095670e11b0d: rpc error: code = Internal desc = transport is closing\"}\n" does not contain "Successfully built"
09:35:34 assertion failed: 0 (int) != 4 (int)
09:35:35

@guillaumerose
Copy link
Copy Markdown

guillaumerose commented Mar 20, 2018

In the Docker for Desktop CI, we have the same issue as @thaJeztah mentioned.

+ docker build -t trim_works .
    Sending build context to Docker daemon  3.584kB

    Step 1/2 : FROM alpine:3.6
    2018/03/20 11:27:34 http2: server: error reading preface from client /Users/administrator/Library/Containers/com.docker.docker/Data/s60: bogus greeting "\x00\x00\x00\x04\x01\x00\x00\x00\x00\x00\x00P\x01\x04\x00\x00\x00\x01\x83\x86E\x95bk"
    3.6: Pulling from library/alpine
    605ce1bd3f31: Pulling fs layer
    context canceled

This error appeared just after merging 18.04-ce-dev (84547b2).

We saw it already 5 times on different machines, different OSX version.

@vdemeester vdemeester merged commit 4460472 into moby:master Mar 21, 2018
@vdemeester vdemeester deleted the e2e-more-run branch March 21, 2018 07:54
@guillaumerose guillaumerose mentioned this pull request Mar 21, 2018
Closed
@thaJeztah thaJeztah mentioned this pull request Apr 18, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

@cpuguy83 cpuguy83 Awaiting requested review from cpuguy83

@tianon tianon Awaiting requested review from tianon tianon is a code owner

+2 more reviewers

@dnephin dnephin dnephin approved these changes

@yongtang yongtang yongtang approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/testing status/4-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@vdemeester @thaJeztah @guillaumerose @dnephin @yongtang @GordonTheTurtle