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

integration: TestPingSwarmHeader(): fix incorrect ping, and cleanup #43658

Merged
merged 1 commit into from
May 30, 2022

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented May 29, 2022

While working on #43657, I noticed I made a mistake in the first ping ("before swarm init"), which was not specifying the daemon's socket path and because of that testing against the main integration daemon (not the locally spun up daemon).

While fixing that, I wondered why the test didn't actually use the client for the requests (to also verify the client converted the response), so I rewrote the test to use client.Ping() and to verify the ping response has the expected values set.

make BIND_DIR=. TEST_FILTER=TestPingSwarmHeader DOCKER_GRAPHDRIVER=vfs test-integration
...
=== RUN   TestPingSwarmHeader
=== RUN   TestPingSwarmHeader/before_swarm_init
=== RUN   TestPingSwarmHeader/after_swarm_init
=== RUN   TestPingSwarmHeader/after_swarm_leave
--- PASS: TestPingSwarmHeader (2.75s)
    --- PASS: TestPingSwarmHeader/before_swarm_init (0.00s)
    --- PASS: TestPingSwarmHeader/after_swarm_init (0.00s)
    --- PASS: TestPingSwarmHeader/after_swarm_leave (0.00s)
PASS

- A picture of a cute animal (not mandatory but encouraged)

I noticed I made a mistake in the first ping ("before swarm init"), which
was not specifying the daemon's socket path and because of that testing
against the main integration daemon (not the locally spun up daemon).

While fixing that, I wondered why the test didn't actually use the client
for the requests (to also verify the client converted the response), so
I rewrote the test to use `client.Ping()` and to verify the ping response
has the expected values set.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah added area/api area/testing kind/bugfix PR's that fix bugs kind/refactor PR's that refactor, or clean-up code labels May 29, 2022
@thaJeztah thaJeztah added this to the 22.06.0 milestone May 29, 2022
@thaJeztah
Copy link
Member Author

Windows failure is unrelated, and a known flaky test;

=== RUN   TestDockerSuite/TestSlowStdinClosing/0
    docker_cli_run_test.go:4168: d:\CI\PR-43658\1\binary\docker.exe: Error response from daemon: failed to create shim task: open \\.\pipe\containerd-cb38cd61da75cd41ad62422f0ef1dec6f083f5502534a4ed9b951fa492d93cfe-init-stdin: The system cannot find the file specified.: not found.
        
    docker_cli_run_test.go:4177: assertion failed: error is not nil: exit status 127
        --- FAIL: TestDockerSuite/TestSlowStdinClosing/0 (1.13s)

(I'll kick CI again)

@thaJeztah
Copy link
Member Author

Thx for reviewing! Let me bring this one in (it's a test-only change, so no risk w.r.t. production code); this header was added for the 22.06 release, so I'd like to have the related test fixed for that 😄

@thaJeztah thaJeztah merged commit 8752ec9 into moby:master May 30, 2022
@thaJeztah thaJeztah deleted the fix_TestPingSwarmHeader branch May 30, 2022 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api area/testing kind/bugfix PR's that fix bugs kind/refactor PR's that refactor, or clean-up code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants