-
Notifications
You must be signed in to change notification settings - Fork 18.7k
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-cli: make some tests less noisy and easier debuggable #40115
Conversation
494d3a3
to
479e5dc
Compare
welp! why does CI not show the Is something in the filter we use in the Jenkinsfile causing them to not be found? Line 321 in eda98ad
|
OK something definitely weird; running locally, the tests are included;
While on this PR, the only tests that are run from the TestDockerSuite are:
|
Problem seems to have resolved itself, only now validate is failing; looks like master is broken because a PR was merged that didn't add godoc to exported methods (which wasn't previously caught by the linter rules) |
Before: make TEST_FILTER=TestServiceLogs test-integration ... --- PASS: TestDockerSwarmSuite/TestServiceLogs (14.63s) docker_cli_service_logs_test.go:49: log for "TestServiceLogs1": "TestServiceLogs1.1.rjyqj1v08llu@625d614f901a | hello1\n" docker_cli_service_logs_test.go:49: log for "TestServiceLogs2": "TestServiceLogs2.1.fyaljkh9tmp3@625d614f901a | hello2\n" After: make TEST_FILTER=TestServiceLogs test-integration ... --- PASS: TestDockerSwarmSuite/TestServiceLogs (14.63s) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Before: --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate (24.21s) docker_api_containers_test.go:2100: case 0 - config: {volume /foo false <nil> <nil> <nil>} docker_api_containers_test.go:2100: case 1 - config: {volume /foo/ false <nil> <nil> <nil>} docker_api_containers_test.go:2100: case 2 - config: {volume test1 /foo false <nil> <nil> <nil>} docker_api_containers_test.go:2100: case 3 - config: {volume test2 /foo true <nil> <nil> <nil>} docker_api_containers_test.go:2100: case 4 - config: {volume test3 /foo false <nil> 0xc000876640 <nil>} docker_api_containers_test.go:2100: case 5 - config: {bind /tmp/test-mounts-api-1770842294 /foo false <nil> <nil> <nil>} docker_api_containers_test.go:2100: case 6 - config: {bind /tmp/test-mounts-api-1770842294 /foo true <nil> <nil> <nil>} docker_api_containers_test.go:2100: case 7 - config: {bind /tmp/test-mounts-api-3832384157 /foo false <nil> <nil> <nil>} docker_api_containers_test.go:2100: case 8 - config: {bind /tmp/test-mounts-api-3832384157 /foo true <nil> <nil> <nil>} docker_api_containers_test.go:2100: case 9 - config: {bind /tmp/test-mounts-api-3832384157 /foo true 0xc000876aa0 <nil> <nil>} docker_api_containers_test.go:2100: case 10 - config: {volume /foo false <nil> 0xc000876ac0 <nil>} docker_api_containers_test.go:2100: case 11 - config: {volume /foo/ false <nil> 0xc000876ae0 <nil>} docker_api_containers_test.go:2100: case 12 - config: {volume test4 /foo false <nil> 0xc000876b00 <nil>} docker_api_containers_test.go:2100: case 13 - config: {volume test5 /foo true <nil> 0xc000876b20 <nil>} After: --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate (63.59s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/0_config:_{volume__/foo_false__<nil>_<nil>_<nil>} (2.98s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/1_config:_{volume__/foo/_false__<nil>_<nil>_<nil>} (2.11s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/2_config:_{volume_test1_/foo_false__<nil>_<nil>_<nil>} (2.26s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/3_config:_{volume_test2_/foo_true__<nil>_<nil>_<nil>} (7.78s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/4_config:_{volume_test3_/foo_false__<nil>_0xc00000ecc0_<nil>} (25.19s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/5_config:_{bind_/tmp/test-mounts-api-1123034866_/foo_false__<nil>_<nil>_<nil>} (2.21s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/6_config:_{bind_/tmp/test-mounts-api-1123034866_/foo_true__<nil>_<nil>_<nil>} (2.21s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/7_config:_{bind_/tmp/test-mounts-api-3533158313_/foo_false__<nil>_<nil>_<nil>} (2.16s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/8_config:_{bind_/tmp/test-mounts-api-3533158313_/foo_true__<nil>_<nil>_<nil>} (2.18s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/9_config:_{bind_/tmp/test-mounts-api-3533158313_/foo_true__0xc00000f760_<nil>_<nil>} (2.18s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/10_config:_{volume__/foo_false__<nil>_0xc00000f780_<nil>} (2.25s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/11_config:_{volume__/foo/_false__<nil>_0xc00000f7e0_<nil>} (2.37s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/12_config:_{volume_test4_/foo_false__<nil>_0xc00000f800_<nil>} (2.28s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsCreate/13_config:_{volume_test5_/foo_true__<nil>_0xc00000f820_<nil>} (2.44s) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Before: --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation (0.52s) docker_api_containers_test.go:1927: case 0 docker_api_containers_test.go:1927: case 1 docker_api_containers_test.go:1927: case 2 docker_api_containers_test.go:1927: case 3 docker_api_containers_test.go:1927: case 4 docker_api_containers_test.go:1927: case 5 docker_api_containers_test.go:1927: case 6 docker_api_containers_test.go:1927: case 7 docker_api_containers_test.go:1927: case 8 docker_api_containers_test.go:1927: case 9 docker_api_containers_test.go:1927: case 10 docker_api_containers_test.go:1927: case 11 docker_api_containers_test.go:1927: case 12 docker_api_containers_test.go:1927: case 13 docker_api_containers_test.go:1927: case 14 docker_api_containers_test.go:1927: case 15 After: --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation (1.13s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_0 (0.01s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_1 (0.00s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_2 (0.00s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_3 (0.00s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_4 (0.00s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_5 (0.11s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_6 (0.12s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_7 (0.13s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_8 (0.00s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_9 (0.06s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_10 (0.08s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_11 (0.10s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_12 (0.18s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_13 (0.12s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_14 (0.14s) --- PASS: TestDockerSuite/TestContainersAPICreateMountsValidation/case_15 (0.00s) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Now that we marked these utilities as helpers, it should be possible to find which test-case failed (if any), and we can skip logging in the "happy path". This makes these tests less noisy, which makes it easier to find actually important information in the output: --- PASS: TestDockerSuite/TestCpFromCaseC (0.96s) docker_cli_cp_utils_test.go:244: checking that file "/tmp/test-cp-from-case-c450122079/file2" contains "file2\n" docker_cli_cp_utils_test.go:192: running `docker cp 962b1f3311e742b0842e13b2ad350214cea25883999fd26e87e8c9ddf40d5eb4:/root/file1 /tmp/test-cp-from-case-c450122079/file2` docker_cli_cp_utils_test.go:244: checking that file "/tmp/test-cp-from-case-c450122079/file2" contains "file1\n" Some of these tests should probably be rewritten to use subtests, but that's something for a follow-up. Signed-off-by: Sebastiaan van Stijn <[email protected]>
479e5dc
to
3ddb410
Compare
@tiborvass @kolyshkin ptal; all green now 👍 |
} else { | ||
assert.NilError(c, err) | ||
} | ||
x := x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we enable Parallel, otherwise you can get surprising failures (ran into that a while back); see https://blog.golang.org/subtests, which also shows this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I learned the hard way. Scratching my head why things were randomly failing 😅
@kolyshkin @cpuguy83 ptal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
see individual commits for details