Project

General

Profile

Actions
Copy link

action #69133

closed

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

Added by okurz over 4 years ago. Updated over 4 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
kraih
Category:
Regressions/Crashes
Target version:
Ready
Start date:
2020-07-20
Due date:
% Done:

0%

Estimated time:

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
Actions
Copy link
 #1

Updated by okurz over 4 years ago

  • Difficulty set to easy
Actions
Copy link
 #2

Updated by kraih over 4 years ago

  • Assignee set to kraih

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

Actions
Copy link
 #3

Updated by tinita over 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.

Actions
Copy link
 #4

Updated by kraih over 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. :)

Actions
Copy link
 #5

Updated by kraih over 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

Actions
Copy link
 #6

Updated by okurz over 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.

Actions
Copy link

Also available in: Atom PDF