[Tests] Reorganize final exception handling#71557
[Tests] Reorganize final exception handling#71557mergify[bot] merged 2 commits intoproject-chip:masterfrom
Conversation
- Avoid printing stacktrace multiple times. - Don't pollute the stacktrace when reraising a single exception. - Add name of resource that failed cleaning to the log. Signed-off-by: Marek Pikuła <m.pikula@partner.samsung.com>
There was a problem hiding this comment.
Code Review
This pull request refactors the exception handling in the test suite runner to aggregate errors from both execution and cleanup phases using BaseExceptionGroup. While the goal is to improve error reporting, the implementation introduces several issues: the use of the match statement and BaseExceptionGroup breaks compatibility with Python versions older than 3.10 and 3.11 respectively. Furthermore, catching BaseException incorrectly captures SystemExit, which is then inappropriately wrapped in a RuntimeError, and the use of raise ... from error adds unnecessary layers to the stack trace. Finally, removing exc_info=True from cleanup logs makes debugging resource termination failures more difficult.
There was a problem hiding this comment.
Pull request overview
Refines exception handling in run_test_suite.py to reduce duplicated stack traces and improve cleanup logging for YAML test runs.
Changes:
- Consolidates execution exception capture into a single
errorslist and defers final raising logic until after cleanup. - Improves cleanup logging by including the resource name being terminated.
- Adds special-case handling for single errors (KeyboardInterrupt / ResultError) and uses
BaseExceptionGroupwhen multiple errors occur.
Signed-off-by: Marek Pikuła <m.pikula@partner.samsung.com>
|
PR #71557: Size comparison from 04ec448 to e998699 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #71557 +/- ##
==========================================
+ Coverage 54.34% 54.38% +0.04%
==========================================
Files 1577 1580 +3
Lines 108287 108385 +98
Branches 13399 13388 -11
==========================================
+ Hits 58844 58950 +106
+ Misses 49443 49435 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* Reorganize final exception handling - Avoid printing stacktrace multiple times. - Don't pollute the stacktrace when reraising a single exception. - Add name of resource that failed cleaning to the log. Signed-off-by: Marek Pikuła <m.pikula@partner.samsung.com> * Fix reraising single exception Signed-off-by: Marek Pikuła <m.pikula@partner.samsung.com> --------- Signed-off-by: Marek Pikuła <m.pikula@partner.samsung.com>
Summary
Small improvements to the final exception handling in YAML tests:
Related issues
Submitted along:
Testing
Manual tests with
run_test_suite.pyand injected exceptions in various places to check for the behavior.Readability checklist
The checklist below will help the reviewer finish PR review in time and keep the
code readable:
descriptive
“When in Rome…”
rule (coding style)
See: Pull Request Guidelines