[bugfix] Fix infinite loop bug when cleanup phase fails

[ekouts]

The task cleanup should be in a try block for both serial and asynchronous execution policies in order to have the correct _retired_tasks list.

The output of reframe for tests that fail on cleanup can also be misleading because both will be printed

[----------] waiting for spawned checks to finish
[       OK ] CleanupFailTest on generic:login using builtin-gcc
[     FAIL ] CleanupFailTest on generic:login using builtin-gcc

The line [ OK ] is printed just after performance.

To avoid the confusion now the OK is printed after the performance stage in both serial and asynchronous policy and a new message [ ERROR ] is printed for failure in the cleanup stage. The error message is different because the failure is probably due to a reframe/system error. New output looks like that:

serial:

[----------] started processing CleanupFailTest (CleanupFailTest)
[ RUN      ] CleanupFailTest on generic:login using builtin-gcc
[       OK ] CleanupFailTest on generic:login using builtin-gcc
[    ERROR ] CleanupFailTest on generic:login using builtin-gcc
[----------] finished processing CleanupFailTest (CleanupFailTest)

[----------] started processing SleepTest (SleepTest)
[ RUN      ] SleepTest on generic:login using builtin-gcc
[       OK ] SleepTest on generic:login using builtin-gcc
[----------] finished processing SleepTest (SleepTest)

async:

----------] started processing CleanupFailTest (CleanupFailTest)
[ RUN      ] CleanupFailTest on generic:login using builtin-gcc
[----------] finished processing CleanupFailTest (CleanupFailTest)

[----------] started processing SleepTest (SleepTest)
[ RUN      ] SleepTest on generic:login using builtin-gcc
[----------] finished processing SleepTest (SleepTest)

[----------] waiting for spawned checks to finish
[       OK ] CleanupFailTest on generic:login using builtin-gcc
[    ERROR ] CleanupFailTest on generic:login using builtin-gcc
[       OK ] SleepTest on generic:login using builtin-gcc
[----------] all spawned checks have finished

Fixes #1121

[pep8speaks]

Hello @ekouts, Thank you for updating!

Cheers! There are no PEP8 issues in this Pull Request!Do see the ReFrame Coding Style Guide

Comment last updated at 2020-01-27 17:25:33 UTC

[teojgo]

Just add one empty line separating the absolute from relative imports.

[codecov-io]

Codecov Report

Merging #1124 into master will not change coverage.
The diff coverage is 95.12%.

@@           Coverage Diff           @@
##           master    #1124   +/-   ##
=======================================
  Coverage   91.96%   91.96%           
=======================================
  Files          81       81           
  Lines       11653    11653           
=======================================
  Hits        10717    10717           
  Misses        936      936

Impacted Files	Coverage Δ
reframe/frontend/cli.py	80.87% <100%> (ø)	⬆️
unittests/test_pipeline.py	94.66% <100%> (ø)	⬆️
unittests/test_cli.py	93.45% <100%> (ø)	⬆️
reframe/__init__.py	80% <100%> (ø)	⬆️
reframe/core/pipeline.py	92.59% <83.33%> (ø)	⬆️
reframe/frontend/statistics.py	89.71% <92.3%> (ø)	⬆️
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 015a7a7...4ada425. Read the comment docs.

[vkarak]

We had a private chat with @ekouts about the changes need to be made. I'll re-review later.

[vkarak]

The implementation looks good. The only thing are the unit tests. Check my respective comments.