Skip to content

Conversation

@jgphpc
Copy link
Contributor

@jgphpc jgphpc commented Jun 16, 2021

Closes #1947

@pep8speaks
Copy link

pep8speaks commented Jun 16, 2021

Hello @jgphpc, 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 2021-07-14 20:45:07 UTC

@teojgo teojgo requested review from teojgo and victorusu June 16, 2021 12:30
ekouts
ekouts previously requested changes Jun 23, 2021
Copy link
Contributor

@ekouts ekouts left a comment

Choose a reason for hiding this comment

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

I am not sure this was what @victorusu originally asked for in issue #1947. Victor maybe you can shed some light:

  1. I think he meant to print something like this
    Rerun the failed tests using '--restore-session <report-file> -n <name1> -p <partition1> --system <system> -n <name2> -p <partition2> ...'. Or we can make it simpler with
    Rerun the failed tests using '--restore-session --failed -r'.
  2. I don't agree to print it only in the failure statistic, when we pass --failure-stats. This is not really part of the statistics and I prefer that we print it always. What do you guys think?

@jjotero jjotero added this to the ReFrame Sprint 21.06.2 milestone Jun 28, 2021
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.

I agree with @ekouts. I don't think that this is what @victorusu requested in the corresponding issue. I interpret it as a simple line at the vey end (after the failure statistics) saying the something like "Rerun all failed tests with reframe --restore-session=report-file --failed. I personally also thing that the report-file is not needed. But @victorusu, please explain more what's your feature request is about.

@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2021

Codecov Report

Merging #2016 (4e434cd) into master (9fad84c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2016   +/-   ##
=======================================
  Coverage   86.74%   86.74%           
=======================================
  Files          52       52           
  Lines        9269     9270    +1     
=======================================
+ Hits         8040     8041    +1     
  Misses       1229     1229           
Impacted Files Coverage Δ
reframe/frontend/cli.py 76.00% <100.00%> (+0.04%) ⬆️

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 9fad84c...4e434cd. Read the comment docs.

Copy link
Contributor

@victorusu victorusu left a comment

Choose a reason for hiding this comment

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

lgtm

@vkarak
Copy link
Contributor

vkarak commented Jul 14, 2021

I updated this PR to print the report file always at the end of the session. I am against adding a Rerun with type of message, cos it introduces unnecessary complexity to the implementation. We report the file here, and somebody can use it with the --restore-session option.

@vkarak vkarak dismissed ekouts’s stale review July 14, 2021 20:47

PR comments addressed

@vkarak vkarak changed the title [feat] Add rerun to the failure statistics [feat] Print the run report location at the end of a session Jul 14, 2021
@vkarak vkarak merged commit 38c8ba9 into reframe-hpc:master Jul 14, 2021
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.

Display the report file in the failure statistics

7 participants