-
Notifications
You must be signed in to change notification settings - Fork 37
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
refactor: move thinpool setup to general test Setup/Teardown #223
Conversation
83fe0d5
to
cbae8fe
Compare
Codecov Report
@@ Coverage Diff @@
## main #223 +/- ##
==========================================
+ Coverage 55.81% 56.27% +0.45%
==========================================
Files 44 44
Lines 2012 1985 -27
==========================================
- Hits 1123 1117 -6
+ Misses 781 767 -14
+ Partials 108 101 -7
Continue to review full report at Codecov.
|
cbae8fe
to
8f92c53
Compare
8f92c53
to
6fd1d10
Compare
I had this being done in bash via a Docker Entrypoint + `trap` combination, but it was quite brittle, and sometimes the teardown would not run which then meant you had to run a manual step. This changes brings that into the test so the setup and teardown can be handled programatically.
6fd1d10
to
dd55e24
Compare
ffs sorry @richardcase can i get a re-click? |
gexec.CleanupBuildArtifacts() | ||
} | ||
|
||
func makeDirectories() { |
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 wonder if we can add this to a pkg
or somewhere, and reuse in cmd/dev-helper
and the runner, we could manage this setup from one place.
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.
gonna delete cmd/dev-helper
instead 😈
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.
It would be still better to manage those settings in one place, I like the Go version and we can use this as a replacement for the shell script (and update the documentation to use that instead)
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.
oh like you mean instead of all the copy-pastable commands we have in the quick-start docs?
for sure, would be good to have a (up to date) "setup all my stuff" command for people.
can i do that later tho :p
I had this being done in bash via a Docker Entrypoint +
trap
combination, but it was quite brittle, and sometimes the teardown would
not run which then meant you had to run a manual step to remove thinpools
and loop devices which was annoying.
This changes brings that into the test so the setup and teardown can be
handled programatically.
There is still a case where some manual cleanup may be required: when the tests
fail and devices cannot be removed. I have a thing on my list to have an aggressive
cleanup of all mvms etc on failure. Coming soon!
There is also a case where some "child" pool devices are not cleaned up. I have deliberately
not done anything about this as we should actually fix the code that is leaving those lying
around after a failed delete.
Checklist: