rnp
rnp copied to clipboard
[#1498] Fix tmpdir cleanup
A lot of temporary directories are always left around after the tests even when all they all pass. There are a few sources of that, and this patchset addresses them all.
Bug: https://github.com/rnpgp/rnp/issues/1498
Codecov Report
Base: 81.76% // Head: 81.77% // Increases project coverage by +0.01% :tada:
Coverage data is based on head (
b0fec63) compared to base (fbfef64). Patch coverage: 95.65% of modified lines in pull request are covered.
:exclamation: Current head b0fec63 differs from pull request most recent head d5836ac. Consider uploading reports for the commit d5836ac to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## master #1866 +/- ##
==========================================
+ Coverage 81.76% 81.77% +0.01%
==========================================
Files 140 140
Lines 28974 28997 +23
==========================================
+ Hits 23690 23712 +22
- Misses 5284 5285 +1
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/tests/support.h | 100.00% <ø> (ø) |
|
| src/tests/support.cpp | 82.23% <93.33%> (+0.34%) |
:arrow_up: |
| src/tests/ffi.cpp | 86.30% <100.00%> (+0.01%) |
:arrow_up: |
| src/tests/issues/1030.cpp | 90.47% <100.00%> (+0.47%) |
:arrow_up: |
| src/tests/rnp_tests.cpp | 94.11% <100.00%> (+2.45%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
@andrey-utkin Could you please add check whether run is made under the GHA, so only local runs would be cleaned up automatically? (this should be helpful: https://docs.github.com/en/actions/learn-github-actions/environment-variables#default-environment-variables )
Basically, current centos8 failure is something because of what we should not cleanup CI automatically, and upload artifacts once #1430 is implemented.
For now I've added a way to preserve the tempdirs by setting an env var KEEPTEMP.
I'd rather make GHA workflows declare explicitly that they want this, by setting that env var, because this simple concept will likely outlive the distinction between GHA and local runs.
We can add this setting in .github/workflows/*.yml right now, basically reverting to the previous behaviour in GHA. But I imagine if we start writing actual artifact retention logic, we'd handle it from the context of a failed assertion, which should have the knowledge of what the workdir is, and before the workdir is removed, so the setting is not necessary.
For now I've added a way to preserve the tempdirs by setting an env var
KEEPTEMP.I'd rather make GHA workflows declare explicitly that they want this, by setting that env var, because this simple concept will likely outlive the distinction between GHA and local runs.
I doubt that it would be the heaviest problem once migrating away from GHA :-) But, okay, let's use this approach, but it would be better to rename variable to something starting with RNP_, like. RNP_KEEP_TEMP or so on.
I understand this PR has been approved, however, I can't help but wonder why the windows-2019 msvc tests are consistently timing out:
- https://github.com/rnpgp/rnp/actions/runs/3513330605/jobs/5907300545
- https://github.com/rnpgp/rnp/actions/runs/3513330605/jobs/5907300677
I've tried retrying them but same issue with timeout. Prior to this PR, they complete pretty fast:
- https://github.com/rnpgp/rnp/actions/runs/3514838439
@ni4 @antonsviridenko is this timeout an issue that has already been fixed in another PR and therefore safe to merge this?
@ronaldtse Thanks for bringing this up, somehow missed this. Should be related to test file leftovers...
yeah, probably some issue introduced by this PR...
Seems to hang on SHFileOperationA call, continuing with debugging...
@ronaldtse Now it should be good to go. Issue should be in some hanging GUI, displayed during the SHFileOperationA(), maybe some readonly file or similar.
Thank you @ni4 ! This is perfect, merging.