-
-
Notifications
You must be signed in to change notification settings - Fork 346
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
Update default pytest output #1034
Conversation
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.
Thanks @ischoegl This looks good to me. Just one small question.
I'll wait for @speth to review, since he was specifically requested. |
Codecov Report
@@ Coverage Diff @@
## main #1034 +/- ##
==========================================
+ Coverage 72.54% 72.92% +0.38%
==========================================
Files 355 357 +2
Lines 46422 47120 +698
==========================================
+ Hits 33675 34363 +688
- Misses 12747 12757 +10
Continue to review full report at Codecov.
|
@bryanwweber and @speth ... I updated the interface so the verbose output is optional, e.g.
So, the changes are as follows:
One caveat is that verbosity only affects |
Introducing the
The |
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 implementation of the new options here is fine.
Given that the initial purpose of printing out test durations was as a diagnostic for the CI runs, I think both show_long_test
and verbose_tests
should be set to y
for all CI runs.
@speth ... thank you for your feedback! Since I initially looked at this, I have started to to believe that there is a benefit to having shorter output by default for users that compile Cantera from source. For development, it is a different issue, where we often need more detailed information, but the flags are easy to toggle.
As an alternative to changing the default (where suppressing detailed output imho makes sense), I just flipped the flags for GH actions work flows. What do you think? |
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.
As an alternative to changing the default (where suppressing detailed output imho makes sense), I just flipped the flags for GH actions work flows. What do you think?
Yes, that's exactly what I was suggesting. Sorry if that wasn't clear. With this update, I think this is good.
Changes proposed in this pull request
pytest
output to verbose (-v
option)show_long_tests
The rationale for switching to verbose output is that segfaults are difficult to locate with the progress bar. Further, the longest running tests can still be displayed using
While
test_convert.py
(scons test-python-convert
) creates numerousWARNING
messages from captured log calls - which may confuse some users - I am leaving those for another PR as those may or may not be a feature.If applicable, fill in the issue number this pull request is fixing
Fixes #1026
Checklist
scons build
&scons test
) and unit tests address code coverage