Skip to content

Conversation

@vkarak
Copy link
Contributor

@vkarak vkarak commented Nov 13, 2020

This PR adds a post-init validation step to the check loader. A test must fulfil the following requirements in order to be loaded:

  1. Define valid_systems.
  2. Define valid_prog_environs.
  3. Be (deep) copyable.

Implementing (3) without copying the object was quite tricky. I had to implement a validator that recursively descends into the object and checks that every attribute fulfils a condition. The validator examines also any built-in container types element-by-element. The second tricky part was to implement the is_copyable() condition, i.e., understanding that an object can be copied without doing the actual copy. For this, I had to sneak in the copy.deepcopy()'s implementation, because, essentially, we want to return True iff the object can be deep-copied.

The validation step does not check whether sanity_patterns is set, because this can be set in a pipeline hook.

Fixes #701.
Fixes #795.
Fixes #55.

Vasileios Karakasis added 4 commits November 13, 2020 16:12
- Tests must provide `valid_systems` and `valid_prog_environs`
- Tests must not use generator objects or file objects inside their
  constructors.
@vkarak vkarak added this to the ReFrame sprint 20.17 milestone Nov 13, 2020
@vkarak vkarak requested review from ekouts and teojgo November 13, 2020 22:54
@vkarak vkarak self-assigned this Nov 13, 2020
@pep8speaks
Copy link

pep8speaks commented Nov 13, 2020

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 2020-11-16 15:14:45 UTC

@vkarak vkarak requested a review from victorusu November 13, 2020 22:55
@codecov-io
Copy link

codecov-io commented Nov 13, 2020

Codecov Report

Merging #1599 (b9142ac) into master (3f93ea4) will increase coverage by 0.09%.
The diff coverage is 94.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1599      +/-   ##
==========================================
+ Coverage   87.48%   87.57%   +0.09%     
==========================================
  Files          45       45              
  Lines        7127     7244     +117     
==========================================
+ Hits         6235     6344     +109     
- Misses        892      900       +8     
Impacted Files Coverage Δ
reframe/utility/__init__.py 91.79% <92.85%> (+0.23%) ⬆️
reframe/core/pipeline.py 92.06% <100.00%> (ø)
reframe/frontend/loader.py 90.90% <100.00%> (-0.27%) ⬇️
reframe/utility/typecheck.py 95.45% <0.00%> (+0.90%) ⬆️

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 3f93ea4...b9142ac. Read the comment docs.

@vkarak vkarak merged commit 70daadc into reframe-hpc:master Nov 16, 2020
@vkarak vkarak deleted the feat/more-informative-messages branch November 16, 2020 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

4 participants