Project

General

Profile

action #69133

os-autoinst self-tests fail to remove tempdir on test failure

Added by okurz 6 months ago. Updated 6 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Concrete Bugs
Target version:
Start date:
2020-07-20
Due date:
% Done:

0%

Estimated time:
Difficulty:
easy

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

History

#1 Updated by okurz 6 months ago

  • Difficulty set to easy

#2 Updated by kraih 6 months ago

  • Assignee set to kraih

I'll take a look, seems like a good excuse to learn more about os-autoinst tests.

#3 Updated by tinita 6 months 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.

#4 Updated by kraih 6 months 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. :)

#5 Updated by kraih 6 months ago

Yes, it was that simple. A few other tests had the same problem. https://github.com/os-autoinst/os-autoinst/pull/1495

#6 Updated by okurz 6 months 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.

Also available in: Atom PDF