Skip to content

Fix tests cleanup logic#1641

Closed
dubo-dubon-duponey wants to merge 2 commits into
dagger:mainfrom
dubo-dubon-duponey:fix1
Closed

Fix tests cleanup logic#1641
dubo-dubon-duponey wants to merge 2 commits into
dagger:mainfrom
dubo-dubon-duponey:fix1

Conversation

@dubo-dubon-duponey
Copy link
Copy Markdown
Contributor

Described first in #1640 : the cleanup logic does not seem right currently (generated files are not destroyed when asserts are failing, leading to situations where things are broken and the user has to go manually cleanup leftovers).

This make things more resilient, by unconditionally removing possible leftovers before (is there a better way to do that with bats? like a per-test teardown that would always execute?)

PTAL @aluzzardi

@dubo-dubon-duponey
Copy link
Copy Markdown
Contributor Author

Since I changed the tests, I would have expected the integration suite to run.
Adding a second commit here to address that as well.

@aluzzardi
Copy link
Copy Markdown
Contributor

Thank you @dubo-dubon-duponey!

Our CI was badly broken over the last few days, it's back to green now :) Would you mind rebasing this PR so it's finally green?

Signed-off-by: dubo-dubon-duponey <dubodubonduponey+github@pm.me>
If the tests fail, generated files should still be removed to avoid leaving the tree in an unrecoverable condition.

Signed-off-by: dubo-dubon-duponey <dubodubonduponey+github@pm.me>
@dubo-dubon-duponey
Copy link
Copy Markdown
Contributor Author

@aluzzardi closing for now.

I misdiagnosed the issue.
The problem seems to be deeper: different tests apparently manipulate the same files, which makes them inherently racy/flaky if bats is running them concurrently (--jobs 4).

I'll prepare another PR.

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