[feat] Add built-ins to manage test variables and parameters

[jjotero]

This PR introduces some changes suggested in #1568. Building on the same concept as used to implement the extensible parameterized tests, we introduce the var directive, which injects a new variable in the RegressionTest class.

class Spam(rfm.RegressionTest):
    var('myTestVariable', int, float, value=3.141592)
    
    def __init__(self):
        do_something_with_my_own_var(self.myTestVariable)

This variable is injected into Spam as a data descriptor (TypedField) though the __new__ method. If an alternative Field were needed, the var directive accepts thefield argument. The value passed as this argument must be a subclass of the Field class located in reframe/core/fields.py.

On this var directive, the value argument is entirely optional, and one might choose to leave the variable undefined. Logically, if that is the case and the variable is referenced in the class, an exception will be raised. However, If one inherits from a class with an undefined variable and need to set its value, or simply overwriting an old value, one can use the set_var directive.

class Ham(Spam):
    set_var('myTestVariable', 1.5)

Note that, when using the set_var directive, the variable myTestVariable must have been declared through the var directive in a parent class. If that were not the case, another error would be raised.

Now with the converse. Suppose you're writing a library extending the work of someone else, but that "base" library has set a value for a variable. Instead of using such value, you can force the users to set the value for themselves by using the require_var directive. This directive will effectively undefine the variable, which means that an error will be raised if it hasn't been set.

class Eggs(Ham):
    require_var('myTestVariable')

    def __init__(self):
        self.myTestVariable = 20 # We set the variable first ...
        super().__init__()       # and now we can call Spam's __init__ method.

Same as with set_var, the variable referred to by require_var must have been declared with the var directive in any of the parent classes. Flagging a variable that is not present in the test as required is implemented as a no-op (a variable that is not present in the test has no meaning to it).

There are still a few minor things to be addressed:

• Fix pipeline unit tests
• Add var unit tests
• Refactor the extensible parameter spaces to use the same base classes as the variables
• Add the docs
• Implement a namespace management to track all attributes introduced through directives

This PR will also:

• Make illegal any name clashes between variables, parameters and other existing class attributes in the regression test.
• Make illegal to call more than one directive on the same var in the same class (the three var directives have excluding actions).
• Make illegal to redeclare a variable.
• Make illegal for a test to inherit from multiple classes derived from the RegressionTest class (direct consequence of the previous item).
• Introduce the RegressionMixin class. This class supports reframe hooks and directives, solving the limitation introduced by the previous item.

Closes #1568.

[pep8speaks]

Hello @jjotero, 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-02-18 11:39:59 UTC

[codecov-io]

Codecov Report

Merging #1699 (fa211ba) into master (ab18ed7) will increase coverage by 0.19%.
The diff coverage is 96.20%.

@@            Coverage Diff             @@
##           master    #1699      +/-   ##
==========================================
+ Coverage   87.27%   87.46%   +0.19%     
==========================================
  Files          46       49       +3     
  Lines        7708     7900     +192     
==========================================
+ Hits         6727     6910     +183     
- Misses        981      990       +9     

Impacted Files	Coverage Δ
reframe/core/namespaces.py	93.10% <93.10%> (ø)
reframe/core/variables.py	94.93% <94.93%> (ø)
reframe/core/parameters.py	98.36% <97.22%> (+4.24%)	⬆️
reframe/core/meta.py	100.00% <100.00%> (ø)
reframe/core/pipeline.py	91.62% <100.00%> (-0.55%)	⬇️
reframe/frontend/cli.py	75.98% <0.00%> (-0.81%)	⬇️
reframe/core/exceptions.py	94.20% <0.00%> (ø)
reframe/frontend/ci.py	93.33% <0.00%> (ø)
... and 5 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 ab18ed7...fa211ba. Read the comment docs.

[vkarak]

There seems to be a strange side effect:

./bin/reframe -C config/cscs.py -c cscs-checks/ -n MemoryOverconsumptionCheck -r

[  FAILED  ] Ran 2/2 test case(s) from 1 check(s) (2 failure(s))

And this generates this in the build script:

module load daint-gpu
module load PrgEnv-cray
cd posts/cuda-aware-mpi-example/src
cc eatmemory.c -o ./MemoryOverconsumptionCheck

Which is apparently very strange :-D

[jjotero]

I've added some more comments into the metaclass and also extended the docs.

Before we move forward with this, I think we can already address the naming issue in this PR for the parameterized tests with the built-in parameter (see comment).

[jjotero]

There seems to be a strange side effect:

./bin/reframe -C config/cscs.py -c cscs-checks/ -n MemoryOverconsumptionCheck -r

[  FAILED  ] Ran 2/2 test case(s) from 1 check(s) (2 failure(s))

And this generates this in the build script:

module load daint-gpu
module load PrgEnv-cray
cd posts/cuda-aware-mpi-example/src
cc eatmemory.c -o ./MemoryOverconsumptionCheck

Which is apparently very strange :-D

😱😱😱😱😱😱😱😱😱😱
I just did ./bin/reframe -C config/cscs.py -c cscs-checks/system/slurm/slurm.py -r and it ran fine 🤔🤯

[jjotero]

The issue with the MemoryOverconsumptionCheck had its origin in the CudaAwareMPICheck, which had a += on the prebuild_cmds variable.

self.prebuild_cmds += ['cd posts/cuda-aware-mpi-example/src']

This changed default value of prebuild_cmds, which is a reference to the very default value set in the RegressionTest class. This means that test that run after this is executed, will also see this "modified" prebuild_cmds variable.

All this would not be a problem if all tests do the first assignment as a new list, but that is simply too dangerous. So to fix this, the values that get injected into the variables are now deep-copies of the original class-default values, which totally removes this cross-contamination in the tests. The same could have happened for the parameters, so another fix has been implemented in there. To make sure these changes stick, I've added two new unit tests that check this for the variables and parameters.

I've also improved the docs on the built-in section.

[vkarak]

I've only cosmetic suggestions and it's ready to go. I will also give it a try with a full production run.

[jjotero]

Did the full production run pass?

[vkarak]

Perfect! 🎉 Just fix the conflicts with the master and change all the versionadded annotations to 3.4.2, because it's going in today!

[vkarak]

For the conflicts, just keep your version. I have simply fixed the styling in a previous PR.

[jjotero]

Updated version notes to 3.4.2

[vkarak]

Did the full production run pass?

Yes. The failures were just ordinary test failures, not related to framework problems.

[victorusu]

lgtm

[vkarak]

@jenkins-cscs retry daint