Skip to content

Conversation

@vkarak
Copy link
Contributor

@vkarak vkarak commented Apr 17, 2021

Fixes #1849.

Implementation details

Since I had to convert everything to the built-in parameter in the unit tests, I introduced the notion of force loading the test, in which case the loader first unloads the test module if it has already been loaded and then reloads it. This change is implemented in a larger extent in #1848, but here I am only using it in one unit test.

@vkarak vkarak added this to the ReFrame sprint 21.04.1 milestone Apr 17, 2021
@vkarak vkarak requested review from jjotero and teojgo April 17, 2021 21:57
@vkarak vkarak self-assigned this Apr 17, 2021
@pep8speaks
Copy link

pep8speaks commented Apr 17, 2021

Hello @vkarak, 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-04-19 13:33:29 UTC

@codecov-commenter
Copy link

codecov-commenter commented Apr 17, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@91f481f). Click here to learn what that means.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1934   +/-   ##
=========================================
  Coverage          ?   87.88%           
=========================================
  Files             ?       50           
  Lines             ?     8610           
  Branches          ?        0           
=========================================
  Hits              ?     7567           
  Misses            ?     1043           
  Partials          ?        0           
Impacted Files Coverage Δ
reframe/core/decorators.py 83.19% <100.00%> (ø)
reframe/frontend/loader.py 92.50% <100.00%> (ø)
reframe/utility/__init__.py 92.82% <100.00%> (ø)

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 91f481f...57008da. Read the comment docs.

Copy link
Contributor

@jjotero jjotero 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 we should also add a few fixmes or perhaps even create an issue to do the cleanup after the deprecation is done.
The main thing that I have in mind is the _register_test function, which can be massively simplified to just forward whichever *args and **kwargs from the decorator to the test constructor.
Also, (this one is for 4.0) I think it would also make sense to change the simple_test decorator to just @test.

@vkarak
Copy link
Contributor Author

vkarak commented Apr 19, 2021

@jjotero Yes, all these should be addressed properly by #1833 which is scheduled for 4.x

@vkarak vkarak merged commit d40f8b1 into reframe-hpc:master Apr 19, 2021
@vkarak vkarak deleted the feat/deprecate-parameterized_test branch April 19, 2021 13:50
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.

Deprecate the use of the @parameterized_test decorator

4 participants