action #69133
closedos-autoinst self-tests fail to remove tempdir on test failure
Description
Observation¶
Running 99-full-stack.t when it fails I observe an error like the following:
# Failed test 'Result in testresults/result-reload_needles.json is ok'
# at 99-full-stack.t line 82.
# got: 'fail'
# expected: 'ok'
Bailout called. Further testing stopped: testresults/result-reload_needles.json failed
cannot remove path when cwd is /tmp/99-full-stack.t-KMSs/pool for /tmp/99-full-stack.t-KMSs: at /usr/lib/perl5/5.26.1/File/Temp.pm line 1616.
so the temporary directory can not be removed in this case
Acceptance criteria¶
- AC1: No error about failing to remove the temporary directory if the test fails
- AC2: No error about failing to remove the temporary directory if no tests fail
- AC3: The temporary directory is removed after the test exits regardless of the result (fail or pass)
Suggestions¶
- Reproduce the failure locally, e.g. with an artificial error within t/99-full-stack.t in os-autoinst
- Compare how we handle the temporary directory within other test modules within os-autoinst as well as openQA
- Apply a fix and where necessary enhance all other uses of the temp dir in both os-autoinst and openQA
Updated by kraih about 4 years ago
- Assignee set to kraih
I'll take a look, seems like a good excuse to learn more about os-autoinst tests.
Updated by tinita about 4 years ago
If possible, just make sure the current working directory is changed before the bail out.
OTOH I think BAIL_OUT should be avoided and only called if things go really wrong. In that case it is IMHO not that important that all tmp files are removed. They are tmp files after all.
Updated by kraih about 4 years ago
I can probably just add an my $fix = scope_guard sub { chdir 'somehwere/else'; undef $tmpdir };
in the beginning and it will fix the problem. But i've not really worked on os-autoinst yet and do want to get more familiar with the code. :)
Updated by kraih about 4 years ago
Yes, it was that simple. A few other tests had the same problem. https://github.com/os-autoinst/os-autoinst/pull/1495
Updated by okurz about 4 years ago
- Status changed from Workable to Resolved
PR merged. As that concerns only local tests no problems on production can surely be triggered by this. Also verified manually locally.