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

Remove containers after tests complete #89

Merged
merged 2 commits into from
May 18, 2022
Merged

Conversation

amCap1712
Copy link
Member

Leaving containers up after tests is annoying and consumes memory unnecessarily so remove those after tests have run. Also, Take care to exit with correct code after removing containers.

Leaving containers up after tests is annoying and consumes memory unnecessarily
so remove those after tests have run. Also, Take care to exit with correct code
after removing containers.
@github-actions

This comment has been minimized.

@amCap1712
Copy link
Member Author

Tested locally by adding a failing test and ensuring return code was non-zero.

@alastair
Copy link
Collaborator

This looks OK, but it does have the effect of making repetitive tests take longer (as it means that the db needs to start up and shut down each time). Perhaps we should eventually replace it with something like the test.sh we have in LB (which allows you to bring up test containers once manually, and then take them down when you're done).

Running test.sh without any args will build, test, and bring down
db containers.
Running with -u will build and bring up db containers, and subsequent
runs of the script with no args will run tests without bringing down
containers, allowing for iterative development.
test.sh -d will bring down containers.
@alastair
Copy link
Collaborator

@amCap1712 I've updated the test script to copy the LB style, if the CI tests succeed then we can go ahead and merge this.

@github-actions

This comment has been minimized.

@alastair
Copy link
Collaborator

ah, looks like we run the build/test steps manually in order to publish the results file, so everything works with CI, looks good to me.

@alastair alastair merged commit 297351a into master May 18, 2022
@alastair alastair deleted the cleanup-after-test branch May 18, 2022 13:53
@github-actions
Copy link

Unit Test Results

  1 files  ±0    1 suites  ±0   39s ⏱️ ±0s
79 tests ±0  79 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 297351a. ± Comparison against base commit 297351a.

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