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.

Sign up for GitHub

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

Jump to bottom

integration/*: make e2e run without failure #36594

Merged
merged 1 commit into from
Mar 21, 2018
Merged

integration/*: make e2e run without failure #36594

merged 1 commit into from
Mar 21, 2018

Conversation

vdemeester
Copy link
Member

… 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 [email protected]

@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
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 😅

integration/build/build_test.go Outdated
"io"
"io/ioutil"
"strings"
"testing"

"github.com/gotestyourself/gotestyourself/skip"
Copy link
Member

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
Member Author

Choose a reason for hiding this comment

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

🙇‍♂️

integration/internal/swarm/service.go Outdated
@@ -5,6 +5,8 @@ import (
"fmt"
"testing"

"github.com/gotestyourself/gotestyourself/skip"
Copy link
Member

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
Member Author

Choose a reason for hiding this comment

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

🙇‍♂️

hack/test/e2e-run.sh
@@ -13,8 +13,8 @@ export DOCKER_ENGINE_GOARCH=${DOCKER_ENGINE_GOARCH:-${ARCH}}
: ${TESTDEBUG:=}

integration_api_dirs=${TEST_INTEGRATION_DIR:-"$(
find ./integration -type d |
grep -vE '(^./integration($|/internal)|/testdata)')"}
find /tests/integration -type d |
Copy link
Member

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
Member Author

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
Member

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
Member

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)

integration/plugin/authz/main_test.go Outdated
@@ -12,6 +12,8 @@ import (
"strings"
"testing"

"github.com/gotestyourself/gotestyourself/skip"
Copy link
Member

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
Member Author

Choose a reason for hiding this comment

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

🙇‍♂️

@codecov
Copy link

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
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
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 <[email protected]>
@vdemeester vdemeester mentioned this pull request Mar 19, 2018
Merged
@thaJeztah
Copy link
Member

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

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

@dnephin dnephin dnephin approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

@yongtang yongtang yongtang approved these changes

@cpuguy83 cpuguy83 Awaiting requested review from cpuguy83

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

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