scripts: twister: Enhance TestCase/Instance information and presentation #77488
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
LukaszMrugala
force-pushed
the
lmrugalx_expand_execution_counter
branch
3 times, most recently
from
cc8c848
to
5dca931
Compare
zephyrbot
requested review from
gchwier,
golowanow,
hakehuang,
KamilxPaszkiet,
nashif and
PerMac
mdubielx
previously approved these changes
Sep 2, 2024
arikgreen
previously approved these changes
Sep 2, 2024
KamilxPaszkiet
previously approved these changes
Sep 2, 2024
golowanow
reviewed
Sep 2, 2024
| # updated in report_out | ||
| self._cases = Value('i', 0) | ||
|
|
||
| # updated by update_counting_before_pipeline() and report_out() | ||
| self._skipped_cases = Value('i', 0) |
There was a problem hiding this comment.
please update __init__() comment (lines 64--69) accordingly
| print(f"Executed test cases: {self.cases - self.skipped_cases}") | ||
| print(f"Skipped test cases: {self.skipped_cases}") | ||
| print(f"Completed test suites: {self.done}") | ||
| print(f"Added to pipeline test suites: {self.done}") |
There was a problem hiding this comment.
hmm, I might be lost in the example from PR description, but the New style is still something I need to translate in my mind (ordered more like as it happened):
Test Suites:
selected(32) = filtered(17) + built(15)
executed(15) = filtered(1) + errored(1) + failed(1) + passed(12)
Test Cases in Suites - built(148):
not executed (30) = filtered(28) + skipped(2)
executed (118) = no status(6) + blocked(1) + errored(0) + failed(1) + passed(110)
There was a problem hiding this comment.
I've taken a page out of Linux tree command and designed this summary:
--------------------------------------------------
Total test suites: 32
Processed test suites: 32
├─ Skipped test suites (static filter): 17
└─ Completed test suites: 15
├─ Passing test suites: 12
├─ Failing test suites: 1
├─ Errors in test suites: 1
└─ Skipped test suites (at runtime): 1
Skipped test suites: 18
├─ Skipped test suites (at runtime): 1
└─ Skipped test suites (static filter): 17
---------------------- ----------------------
Total test cases: 148
├─ Filtered test cases: 28
├─ Skipped test cases: 2
└─ Executed test cases: 118
├─ Failed test cases: 2
├─ Error test cases: 0
├─ Blocked test cases: 1
├─ Passed test cases: 110
└─ Test cases without a status: 6
--------------------------------------------------
It duplicates some information (test suite skips), but I think it is worth it for the clarity of the connections between the statistics. Would you agree that this is a clear summary?
There was a problem hiding this comment.
Just few suggestions:
- let's always think this output will be parsed by someone with a script using regular expressions, etc.
- use same words in same form (like 'Passed', not 'Passing').
- keep rows ordered in sequence of state transitions.
Total Skipped test Suites: 18
├─ Skipped test Suites (static filter): 17
└─ Skipped test Suites (at runtime): 1
And naive questions:
-
Why
Executed test cases: 118don't match to the sum at its sub-items ? -
what to do with this ?
Test cases without a status: 6
There was a problem hiding this comment.
In order:
- OK; new output should be easily processed by relying on common breaking symbols - there is always colon, then whitespace, then number and the first word of each line is unique if grouped by tree.
- OK; Naming is more consistent and a bit shorter now.
- OK; now the order is roughly conforming to the processing order.
- This is because we have retries. The failed instance is retried and its cases counted twice. I should reset it like the test suites are when a retry is attempted. From my code analysis, a retry assumes the previous run null and void and should not be reported in the summary. I've changed that so failed instance testcases are not double-counted
- This is an indication that the current status system has its problems.
NONEandSTARTEDstatuses are not proper terminal statuses. However, this change did not cause them, but merely extracted them so they can be viewed by the user. I have added an additional line shortly explaining that to the end user, easily discarded by an automatic user.
With those comments in mind and with formatting changes (right-alignment for numbers, common column length), here is a new design:
--------------------------------------------------
Total test suites: 32
Processed test suites: 32
├─ Skipped test suites (static filter): 17
└─ Completed test suites: 15
├─ Skipped test suites (at runtime): 1
├─ Passed test suites: 12
├─ Failed test suites: 1
└─ Errors in test suites: 1
Skipped test suites: 18
├─ Skipped test suites (static filter): 17
└─ Skipped test suites (at runtime): 1
---------------------- ----------------------
Total test cases: 148
├─ Filtered test cases: 28
├─ Skipped test cases: 2
└─ Executed test cases: 118
├─ Passed test cases: 110
├─ Blocked test cases: 1
├─ Failed test cases: 1
├─ Errors in test cases: 0
├──── The following test case statuses should not appear in a proper execution ───
└─ Statusless test cases: 6
--------------------------------------------------
99cd9a3
LukaszMrugala
force-pushed
the
lmrugalx_expand_execution_counter
branch
2 times, most recently
from
2f54f04
to
a5fc74b
Compare
LukaszMrugala
added
the
Enhancement
LukaszMrugala
force-pushed
the
lmrugalx_expand_execution_counter
branch
2 times, most recently
from
ade1020
to
579fd3b
Compare
arikgreen
previously approved these changes
Oct 21, 2024
golowanow
reviewed
Oct 23, 2024
LukaszMrugala
force-pushed
the
lmrugalx_expand_execution_counter
branch
3 times, most recently
from
4662fcd
to
a88f339
Compare
ExecutionCounter has been expanded and now hold i.a. more information on the statuses of TestCases. This information is now incorporated in relevant summaries - runner.py and reports.py. Layout of those was changed to present that and previous information in a clear and concise way. TestInstance execution counter now is more intuitive. Instances filtered out before running are no longer included there. Retries now properly reset the counter. TestCases with None and other incorrect final statuses are logged as errors, but do not exit Twister with a nonzero exit code. This is because None statuses, although incorrect, are currently common. Inconsistent spacing in ERROR and FAILED fixed. Signed-off-by: Lukasz Mrugala <lukaszx.mrugala@intel.com> scripts: Dmitri fix Fix of a problem noticed by Dmitri Removed unnecessary additional spaces when printing FAILED and ERROR status. Now TwisterStatus.get_color is used more. Signed-off-by: Lukasz Mrugala <lukaszx.mrugala@intel.com>
LukaszMrugala
force-pushed
the
lmrugalx_expand_execution_counter
branch
from
a88f339
to
0a31f6e
Compare
golowanow
approved these changes
Oct 24, 2024
nashif
approved these changes
Oct 28, 2024
|
@mmahadevan108 @dkalowsk This one fixes a few issues I found in twister, while it does add some additional information in the output (what was considered an enhancement) the issues it fixes are significant to be in the 4.0 release. Thanks. |
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.
Description
ExecutionCounterhas been expanded and now hold i.a. more information on the statuses ofTestCases. This information is now incorporated in relevant summaries - runner.py and reports.py. Layout of those was changed to present that and previous information in a clear and concise way.TestInstanceexecution counter now is more intuitive. Instances filtered out before running are no longer included there. Retries now properly reset the counter.TestCaseswithNONEand other incorrect final statuses are logged as errors, but do not exit Twister with a nonzero exit code. This is becauseNONEstatuses, although incorrect, are currently common.Note that this PR does not concern itself with the logic of status assignments and will not be fixing some peculiarities of current status regime.
Demonstration
To illustrate the difference, I have run the
tests/ztesttests with an additionaltests/ztest/tempdirectory, containing the following testsuites, allowing for a wider status spread:testing.ztest.temp.error- a test that always errors outtesting.ztest.temp.fail- a test that always failstesting.ztest.temp.filter- a test skipped by a filter staticallytesting.ztest.temp.runtime_filter- a test skipped by a filter at runtimetesting.ztest.temp.pass- a simple test that should passtesting.ztest.temp.build_only_skip- a test that is build_onlyrunner.pysummaryOld style:
New style:
Full run log
Old style:
New style: