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

Make REUSE_CONTAINERS the default for running tests. #13569

Merged
merged 3 commits into from
Apr 26, 2024

Conversation

samwho
Copy link
Collaborator

@samwho samwho commented Apr 26, 2024

Description

@PClmnt raised that he keeps running in to the following error when using the --watch flag to run tests locally:

Multiple containers found starting with image: "budibase/couchdb"

This was an error I put in place to prevent a situation I found where tests can spin up multiple CouchDB containers but only end up connecting to 1 of them. When you run with --watch, testcontainers never tears any containers down and so when the tests try to bring up another, it fails.

I looked to see if there's a way to detect when Jest is running in --watch mode but it doesn't seem trivial to do so. I had a play with making a globalTeardown.ts file as well but that doesn't run until you fully cancel the --watch tests.

So I've gone for the simple route of just making container reuse the default and getting rid of the REUSE_CONTAINERS environment variable.

Potential problems with this approach:

  1. It will leave containers lying around when the tests have finished running, which developers may not expect.
  2. These containers will use up resources, which may annoy developers.
  3. The containers will slowly accumulate more data in them the more you run tests, which could consume disk space and perhaps fill up the volumes attached to each container.

I think we can cross those bridges if/when they arise, for now this solves an immediate annoyance.

@samwho samwho added the firestorm Data/Infra/Revenue Team label Apr 26, 2024
@samwho samwho requested a review from a team as a code owner April 26, 2024 10:39
Copy link
Collaborator

@PClmnt PClmnt 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 for sorting this!

@samwho samwho requested a review from PClmnt April 26, 2024 13:58
@samwho samwho merged commit 0624ba4 into master Apr 26, 2024
10 checks passed
@samwho samwho deleted the make-reuse-containers-default branch April 26, 2024 14:02
@github-actions github-actions bot locked and limited conversation to collaborators Apr 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
firestorm Data/Infra/Revenue Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants