Reuse subunit_trace output/validation logic for load/run #333
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously `stestr run` and subunit_trace used slightly different output and validation logic. run wouldn't catch the case where a test exited without a status, while code using `last` and `history` would since they used subunit_trace. Refactor the subunit_trace output and validation logic and use it for `load`/`run`. Converge on the subunit_trace formatted output, in addition to the summary also show if something goes wrong. The primary motivation for this change is to better detect when a subprocess is killed and not all tests are run as expected (segfault, OOM etc.).
jogo
commented
Sep 16, 2022
| if post_fails: | ||
| print_fails(stdout) | ||
| if not no_summary: | ||
| print_summary(stdout, elapsed_time) | ||
|
|
||
| # NOTE(mtreinish): Ideally this should live in testtools streamSummary | ||
| # this is just in place until the behavior lands there (if it ever does) | ||
| if count_tests("status", "^success$") == 0: | ||
| if count_tests("status", "^xfail$") + count_tests("status", "^success$") == 0: |
There was a problem hiding this comment.
figured this is easier to read than join the two regex
There was a problem hiding this comment.
yeah, that's fine I think the regex would just be ^xfail|success$ or something like that, but this is definitely more legible.
Codecov Report
@@ Coverage Diff @@
## main #333 +/- ##
==========================================
+ Coverage 61.48% 61.69% +0.21%
==========================================
Files 30 30
Lines 2630 2624 -6
Branches 472 472
==========================================
+ Hits 1617 1619 +2
+ Misses 888 883 -5
+ Partials 125 122 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
mtreinish
approved these changes
Sep 22, 2022
| if post_fails: | ||
| print_fails(stdout) | ||
| if not no_summary: | ||
| print_summary(stdout, elapsed_time) | ||
|
|
||
| # NOTE(mtreinish): Ideally this should live in testtools streamSummary | ||
| # this is just in place until the behavior lands there (if it ever does) | ||
| if count_tests("status", "^success$") == 0: | ||
| if count_tests("status", "^xfail$") + count_tests("status", "^success$") == 0: |
There was a problem hiding this comment.
yeah, that's fine I think the regex would just be ^xfail|success$ or something like that, but this is definitely more legible.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously
stestr runand subunit_trace used slightly different output and validation logic. run wouldn't catch the case where a test exited without a status, while code usinglastandhistorywould since they used subunit_trace.Refactor the subunit_trace output and validation logic and use it for
load/run.Converge on the subunit_trace formatted output, in addition to the summary also show if something goes wrong.
The primary motivation for this change is to better detect when a subprocess is killed and not all tests are run as expected (segfault, OOM etc.).