-
Notifications
You must be signed in to change notification settings - Fork 111
[feat] Include description (descr
) in detailed listing
#3513
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
[feat] Include description (descr
) in detailed listing
#3513
Conversation
I'm having a bit of difficulty getting the new tests to run locally. It's not clear to me what's different about these test classes vs others in the project. @vkarak, could you please give these a look and point out anything you see me doing incorrectly? Others seem to use the
|
descr
) in detailed listing
f51c326
to
db2ed46
Compare
Signed-off-by: Jack Morrison <[email protected]>
db2ed46
to
61c329a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3513 +/- ##
===========================================
+ Coverage 91.16% 91.26% +0.10%
===========================================
Files 62 62
Lines 13253 13364 +111
===========================================
+ Hits 12082 12197 +115
+ Misses 1171 1167 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d16f9e5
to
30a4bc5
Compare
Signed-off-by: Jack Morrison <[email protected]>
30a4bc5
to
4537a1f
Compare
Now with passing tests and codecov 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the unit tests are a bit over-engineered for such a trivial addition :-) This would lead us to maintain much more code. I would therefore suggest to simply extend the existing test_list_with_details
by setting/unsetting etc. the descr
field of some of the existing tests in frontend_checks.py
.
@@ -11,6 +11,7 @@ class stream_test(rfm.RunOnlyRegressionTest): | |||
valid_systems = ['*'] | |||
valid_prog_environs = ['*'] | |||
executable = 'stream.x' | |||
descr = 'Run the STREAM memory bandwidth benchmark.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is not relevant for this PR.
if hasattr(u.check, 'descr') and u.check.descr: | ||
details_fields.append(f'description: {u.check.descr}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would always add the description even if it's not set. I would do the following:
- If the attribute is not present (pretty uncommon, as it would only happen if a user specifically does
descr = required
in a test), I would print<undefined>
- If it's empty, I would print
<n/a>
.
Also as a style comment, we always leave a blank line after if/for/while etc. blocks.
To accommodate a description string, this includes a small adjustment to the details formatting for variant # and file path, putting each on a new line.
Example output:
Closes #3458.