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

TestLinksEtcHostsContentMatch: use container.Exec() #36569

Merged
merged 1 commit into from
Mar 13, 2018

Conversation

kolyshkin
Copy link
Contributor

I am not quite sure why but this test is sometimes failing like this:

> 15:21:41 --- FAIL: TestLinksEtcHostsContentMatch (0.53s)
> 15:21:41 	assertions.go:226:
>
> 	Error Trace:	links_linux_test.go:46
> 15:21:41
> 	Error:      	Not equal:
> 15:21:41
> 	            	expected: "127.0.0.1\tlocalhost\n::1\tlocalhost
> ip6-localhost
> ip6-loopback\nfe00::0\tip6-localnet\nff00::0\tip6-mcastprefix\nff02::1\tip6-allnodes\nff02::2\tip6-allrouters\n172.17.0.2\tf53feb6df161\n"
> 15:21:41
> 	            	received: ""

To eliminate some possible failures (like ignoring stderr from cat or its exit code), let's use container.Exec() to read a file from a container.

I am not quite sure why but this test is sometimes failing like this:

> 15:21:41 --- FAIL: TestLinksEtcHostsContentMatch (0.53s)
> 15:21:41 	assertions.go:226:
>
> 	Error Trace:	links_linux_test.go:46
> 15:21:41
> 	Error:      	Not equal:
> 15:21:41
> 	            	expected: "127.0.0.1\tlocalhost\n::1\tlocalhost
> ip6-localhost
> ip6-loopback\nfe00::0\tip6-localnet\nff00::0\tip6-mcastprefix\nff02::1\tip6-allnodes\nff02::2\tip6-allrouters\n172.17.0.2\tf53feb6df161\n"
> 15:21:41
> 	            	received: ""

To eliminate some possible failures (like ignoring stderr from `cat` or
its exit code), let's use container.Exec() to read a file from a container.

Fixes: e6bd20e ("Migrate some integration-cli test to api tests")
Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin
Copy link
Contributor Author

@yongtang PTAL


var b bytes.Buffer
_, err = stdcopy.StdCopy(&b, ioutil.Discard, body)
cID := container.Run(t, ctx, client, container.WithNetworkMode("host"))
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could add

poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond))

so that to make sure the container is running?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like such a check is redundant.

As far as I can see from the source code, container.Run() is synchronous operation, i.e. a successful return from container.Run() means the container is running (and its status is set to running)

Surely, a container can exit in between calls to container.Run() and container.Exec(), but there's no need to check for that either, as this is done in Exec().

Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🌵

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! just wanted to open an issue for this test 😄

@thaJeztah
Copy link
Member

Windows is failing on another flaky test (being worked on in #36571), but otherwise green;

12:56:10 ----------------------------------------------------------------------
12:56:10 FAIL: docker_cli_run_test.go:4345: DockerSuite.TestSlowStdinClosing
12:56:10 
12:56:10 docker_cli_run_test.go:4361:
12:56:10     c.Fatal("running container timed out") // cleanup in teardown
12:56:10 ... Error: running container timed out
12:56:10 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants