Skip to content

Conversation

@rsarm
Copy link
Contributor

@rsarm rsarm commented Oct 16, 2020

Closes #1222

EDIT @vkarak

This PR adds two new command-line options:

  1. --restore-session [REPORT], which can restore a previous testing session from a report file and
  2. --failed to select the failed tests.

To achieve the restoration of tests, it introduces a mechanism for serializing and deserializing arbitrary objects in JSON. Any class the derives from JSONSerializable can be encoded in JSON. If an attribute cannot be converted, it will be silently set to null. The deserialization recreates the objects from the JSON contents and based on some additional metadata that contains the module file and the class name.

Still todo:

  • Documentation
  • Improve the design of the JSON dumping of RegressionTest
  • Expand the dumping of RegressionTest attributes
  • More unit tests
  • Address remaining PR comments.

@pep8speaks
Copy link

pep8speaks commented Oct 16, 2020

Hello @rsarm, 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-12-07 18:30:11 UTC

@codecov-io
Copy link

codecov-io commented Oct 16, 2020

Codecov Report

Merging #1538 (985e61a) into master (39aa59a) will increase coverage by 0.14%.
The diff coverage is 90.79%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1538      +/-   ##
==========================================
+ Coverage   87.57%   87.71%   +0.14%     
==========================================
  Files          44       45       +1     
  Lines        7289     7475     +186     
==========================================
+ Hits         6383     6557     +174     
- Misses        906      918      +12     
Impacted Files Coverage Δ
reframe/utility/jsonext.py 82.75% <72.97%> (-12.48%) ⬇️
reframe/frontend/executors/__init__.py 98.10% <91.30%> (-0.61%) ⬇️
reframe/frontend/runreport.py 92.55% <92.55%> (ø)
reframe/frontend/cli.py 76.01% <93.18%> (+1.07%) ⬆️
reframe/core/config.py 90.95% <100.00%> (+0.04%) ⬆️
reframe/core/environments.py 97.08% <100.00%> (+0.02%) ⬆️
reframe/core/pipeline.py 92.48% <100.00%> (+0.03%) ⬆️
reframe/core/schedulers/__init__.py 97.77% <100.00%> (+0.01%) ⬆️
reframe/core/systems.py 88.53% <100.00%> (ø)
reframe/frontend/dependencies.py 98.48% <100.00%> (+0.03%) ⬆️
... and 9 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 39aa59a...985e61a. Read the comment docs.

@rsarm rsarm requested a review from victorusu October 16, 2020 07:47
@rsarm
Copy link
Contributor Author

rsarm commented Oct 16, 2020

For a quick test:

$> ./bin/reframe -r -c ./unittests/resources/checks_unlisted/deps_complex.py --max-retries=3
$> mv ~/.reframe/reports/run-report.json ~/.reframe/reports/run-report-deps-complex.json
$> ./bin/reframe -r -c ./unittests/resources/checks_unlisted/deps_complex.py --max-retries=3 --retry-failed ~/.reframe/reports/run-report-deps-complex.json 

@ekouts
Copy link
Contributor

ekouts commented Oct 16, 2020

I think you should also add the load function in reframe.utility.json and the option to have a custom __rfm_json_decode__. In our case the json.load is enough but a user might want to restore more.

@rsarm
Copy link
Contributor Author

rsarm commented Oct 26, 2020

@jenkins-cscs retry dom

Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will also need documentation.

@rsarm
Copy link
Contributor Author

rsarm commented Nov 3, 2020

Yesterday after adding the options --retry-failed with no argument, I noticed that when running twice with the default report file, it get's overwritten, and the successful dependencies are lost. Since we are overwriting by default the report, what could be the behavior in this case?

Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's on the right track, but still needs some refinement and the dumping must go deeper. I will look closer into the test dumps, and you can work on the other comments.

@vkarak vkarak added this to the ReFrame sprint 20.18 milestone Nov 23, 2020
@vkarak vkarak self-requested a review November 27, 2020 19:46
@vkarak
Copy link
Contributor

vkarak commented Dec 1, 2020

@jenkins-cscs retry dom

Copy link
Contributor

@teojgo teojgo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, I have tested and it works fine. I have a slight comment only if you agree.

@vkarak vkarak requested a review from teojgo December 7, 2020 18:11
@vkarak vkarak merged commit 2727d8c into reframe-hpc:master Dec 7, 2020
@rsarm rsarm deleted the feat/restart-from-jsonreport branch March 10, 2021 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a --rerun-failed option that will allow ReFrame to run only the failed test cases in a separate invocation

6 participants