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

test: deflake the test start_containers_in_parallel #748

Merged
merged 3 commits into from
Oct 21, 2024

Conversation

hovinen
Copy link
Contributor

@hovinen hovinen commented Oct 16, 2024

The test often fails, especially in GitHub actions. The test attempts to start four containers on the same image and sets a deadline to prove that they start in parallel rather than in serial. The failure messages indicate tha this deadline is exceeded.

This appears to happen when the image of the container being started has to be pulled. Sometimes the pull operation takes so much time that the deadline is exceeded just due to that.

This adds some lines to the test which start and then stop a container on the same image before attempting to start the containers in parallel. Doing so ensures that the image has already been pulled when the real test starts running. Then the containers should start in close to the intended two seconds, so the test should pass more reliably.

Manual experiments appear to support this thesis: Before this change, running

$ docker image rm hello-world
$ cargo test --test async_runner --no-default-features --features blocking start_containers_in_parallel

would consistently fail due to an exceeded deadline. With this change, the test consistently passes.

Note: This and other tests still depend on access to the Docker Hub and fail when they cannot pull the images they need. This can also lead to flakiness.

The test often fails, especially in GitHub actions. The test attempts to
start four containers on the same image and sets a deadline to prove
that they start in parallel rather than in serial. The failure messages
indicate tha this deadline is exceeded.

This appears to happen when the image of the container being started has
to be pulled. Sometimes the pull operation takes so much time that the
deadline is exceeded just due to that.

This adds some lines to the test which start and then stop a container
on the same image before attempting to start the containers in parallel.
Doing so ensures that the image has already been pulled when the real
test starts running. Then the containers should start in close to the
intended two seconds, so the test should pass more reliably.

Manual experiments appear to support this thesis: Before this change,
running

```sh
$ docker image rm hello-world
$ cargo test --test async_runner --no-default-features --features blocking start_containers_in_parallel
```

would consistently fail due to an exceeded deadline. With this change,
the test consistently passes.

Note: This and other tests still depend on access to the Docker Hub and
fail when they cannot pull the images they need. This can also lead to
flakiness.
Copy link

netlify bot commented Oct 16, 2024

Deploy Preview for testcontainers-rust ready!

Name Link
🔨 Latest commit 6d488f4
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-rust/deploys/67160e9ad0a6920008cd4f44
😎 Deploy Preview https://deploy-preview-748--testcontainers-rust.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@DDtKey
Copy link
Collaborator

DDtKey commented Oct 20, 2024

Thank you for the contribution! 👍
In this test, it makes sense to pull the image in advance, because we start them in parallel.

I've left just one comment to pull the image in more appropriate way.

@DDtKey DDtKey changed the title Attempt to deflake the test start_containers_in_parallel test: deflake the test start_containers_in_parallel Oct 20, 2024
@DDtKey DDtKey merged commit 7711714 into testcontainers:main Oct 21, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants