Skip to content

Conversation

@rsarm
Copy link
Contributor

@rsarm rsarm commented Dec 3, 2018

Closes #554

@rsarm rsarm added this to the ReFrame sprint 2018w48 milestone Dec 3, 2018
@rsarm rsarm self-assigned this Dec 3, 2018
@rsarm rsarm requested a review from vkarak December 3, 2018 14:47
@vkarak vkarak added bugfix and removed bug labels Dec 3, 2018
@vkarak vkarak changed the title [bugfix] Add handling of bad tests to avoid frontend crashing [bugfix] Improve handling of bad tests to avoid frontend crashing Dec 3, 2018
@vkarak vkarak requested a review from teojgo December 3, 2018 18:28
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 think that we should give the option to print the stack traces on demand. When developing a test, this is really useful. We have an issue #491 about that. But the problem with the proposed implementation is that it will not be easy to print the stack trace, unless we make the program options globally available, which is not the best solution perhaps. I am not yet convinced that what we are trying here is best solution.

@pep8speaks
Copy link

pep8speaks commented Dec 10, 2018

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 on January 24, 2019 at 23:28 Hours UTC

@vkarak vkarak removed this from the ReFrame sprint 2018w48 milestone Dec 10, 2018
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

@vkarak
Copy link
Contributor

vkarak commented Jan 24, 2019

I have cleaned up slightly the implementation, but it is essentially the same. The stack trace is emitted at the verbose level and user is hinted to use the -v option to see it.

@codecov-io
Copy link

codecov-io commented Jan 24, 2019

Codecov Report

Merging #623 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #623      +/-   ##
==========================================
+ Coverage   91.75%   91.82%   +0.07%     
==========================================
  Files          76       77       +1     
  Lines        9326     9346      +20     
==========================================
+ Hits         8557     8582      +25     
+ Misses        769      764       -5
Impacted Files Coverage Δ
...ttests/resources/checks_unlisted/bad_init_check.py 100% <100%> (ø)
unittests/test_loader.py 100% <100%> (ø) ⬆️
reframe/core/decorators.py 98.57% <100%> (+0.29%) ⬆️
reframe/core/exceptions.py 83.94% <0%> (+3.64%) ⬆️

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 f349a3e...fcb30fc. Read the comment docs.

@vkarak vkarak assigned vkarak and unassigned rsarm Jan 24, 2019
@vkarak vkarak changed the title [bugfix] Improve handling of bad tests to avoid frontend crashing [WIP] [bugfix] Improve handling of bad tests to avoid frontend crashing Jan 24, 2019
@vkarak
Copy link
Contributor

vkarak commented Jan 24, 2019

I mark this as a WIP, because we still need an additional unit test to cover the case of the loading of a bad test.

@vkarak vkarak changed the title [WIP] [bugfix] Improve handling of bad tests to avoid frontend crashing [bugfix] Improve handling of bad tests to avoid frontend crashing Jan 24, 2019
@vkarak vkarak force-pushed the internals/bad-check-handling branch from 0234f4d to dd5e36a Compare January 24, 2019 23:22
@vkarak vkarak force-pushed the internals/bad-check-handling branch from dd5e36a to fcb30fc Compare January 24, 2019 23:28
@vkarak vkarak merged commit 9dc3655 into reframe-hpc:master Jan 24, 2019
@rsarm rsarm deleted the internals/bad-check-handling branch November 26, 2019 13:16
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.

5 participants