Skip to content

Conversation

@vkarak
Copy link
Contributor

@vkarak vkarak commented Feb 15, 2022

This PR adds the loggable argument to the variable and parameter interface to allow you to mark a test variable or parameter as loggable. By default, no test attribute (variable or parameter) is loggable. This contrasts with the current default behaviour where everything is loggable, which can lead to problems, such as the one described in #2380. Test properties can also be logged by decorating them with either the @loggable or @loggable_as decorators. A loggable attribute will become a log record attribute with the name check_NAME, where NAME is the name of the variable, parameter or property.

Almost all previously loggable attributes of RegressionTest and Job are also marked explicitly as loggable, except any attributes containing deferred expressions, such as the sanity_patterns and the perf_patterns. This PR breaks the logging of custom attributes that are not marked as loggable.

This PR does not set the loggable attributes in the various library tests. A separate PR will follow for that.

Fixes #2380.

@pep8speaks
Copy link

pep8speaks commented Feb 15, 2022

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 2022-02-18 10:48:44 UTC

@codecov-commenter
Copy link

codecov-commenter commented Feb 15, 2022

Codecov Report

Merging #2428 (8dbbbcd) into master (da295ca) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2428      +/-   ##
==========================================
+ Coverage   85.66%   85.69%   +0.03%     
==========================================
  Files          56       56              
  Lines       10458    10510      +52     
==========================================
+ Hits         8959     9007      +48     
- Misses       1499     1503       +4     
Impacted Files Coverage Δ
reframe/core/logging.py 77.04% <100.00%> (-0.80%) ⬇️
reframe/core/meta.py 99.15% <100.00%> (+0.04%) ⬆️
reframe/core/parameters.py 95.68% <100.00%> (+0.11%) ⬆️
reframe/core/pipeline.py 93.36% <100.00%> (+0.22%) ⬆️
reframe/core/variables.py 96.30% <100.00%> (+0.04%) ⬆️
reframe/core/schedulers/local.py 92.43% <0.00%> (-0.85%) ⬇️
reframe/core/schedulers/__init__.py 97.82% <0.00%> (-0.55%) ⬇️
reframe/core/schedulers/slurm.py 52.30% <0.00%> (-0.28%) ⬇️

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 da295ca...8dbbbcd. 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 vkarak merged commit be25242 into reframe-hpc:master Feb 18, 2022
@vkarak vkarak deleted the feat/loggable-variables branch February 18, 2022 12:11
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.

Control which test fields can be logged or not

5 participants