Run native PlatformIO tests by area for readable failures#11130
Conversation
The native test job ran every test_* suite in a single platformio invocation, so a failure in the growing suite set could land past the viewable log limit. Build the test programs once, then run the suites grouped by area in sequential invocations, each with its own JUnit report and collapsible log. The runs share one build dir, so gcov coverage still accumulates and a single capture holds the union. Areas are ordered regex rules with a catch-all, so a new suite always runs.
caveman99
added
trunk
github_actions
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe native CI workflow now builds PlatformIO test programs once, runs discovered test suites sequentially by area, consolidates per-area JUnit reports into ChangesNative CI workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant PlatformIO
participant TestAreas
participant JUnitReports
participant CoverageFiles
GitHubActions->>PlatformIO: Build test programs once
PlatformIO-->>GitHubActions: Return shared build artifacts
GitHubActions->>TestAreas: Discover and group test_* suites
GitHubActions->>PlatformIO: Run each area without rebuilding
PlatformIO->>JUnitReports: Write per-area XML reports
GitHubActions->>JUnitReports: Merge reports into testreport.xml
GitHubActions->>JUnitReports: Upload consolidated testreport.xml
GitHubActions->>CoverageFiles: Merge coverage_*.info with lcov
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/test_native.yml (1)
157-179: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve the downstream
testreport.xmlcontract.This now produces/uploads only
testreport-*.xml, but.github/workflows/pr_tests.yml:83-113parses onlytestreport.xml. Its detailed-results summary will therefore be skipped. Update that consumer to aggregate the glob, or upload a compatible consolidated report.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test_native.yml around lines 157 - 179, The test workflow no longer preserves the testreport.xml contract expected by the downstream consumer. Update the “Save test results” step or the corresponding consumer in pr_tests.yml to aggregate or otherwise provide all testreport-*.xml files as the compatible testreport.xml input, while retaining the detailed-results summary behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/test_native.yml:
- Around line 157-179: The test workflow no longer preserves the testreport.xml
contract expected by the downstream consumer. Update the “Save test results”
step or the corresponding consumer in pr_tests.yml to aggregate or otherwise
provide all testreport-*.xml files as the compatible testreport.xml input, while
retaining the detailed-results summary behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6a19ba0d-af74-4183-affe-59e579edf1cf
📒 Files selected for processing (1)
.github/workflows/test_native.yml
⚡ Try this PR in the Web Flasher
Warning This is an automated, unreviewed CI test build. Back up your device configuration Supported boards built by this PR (30)
Build artifacts expire on 2026-08-20. Updated for |
|
Good catch, fixed in the latest commit. The per-area split was only meant to make the logs readable; I unintentionally also split the JUnit report, breaking the single-file The per-area runs still write Verified the merge preserves the downstream parse: two area files with three suites merge to one root, and |
2 participants
The native test job ran every
test_*suite in a singleplatformio test -e coverageinvocation. As the suite set grows, a failure deep in that one run can land past the log's viewable limit, so the failing assertion is not visible without downloading the raw log.Change
Same job, same runner, same coverage. The run is split by area:
platformio test -e coverage --without-testingcompiles src and every test program a single time.--without-buildinginvocations that reuse that build. Each area writes its own JUnit report (testreport-<area>.xml) and its own collapsible log, so a failure is in a small named section instead of buried..pio/build/coverage/src; a single capture holds the union.generate-reportsmerges base + integration + tests as before.Grouping
Areas are ordered
name:regexrules, first match wins, with a catch-allmisc, so a newly added suite always runs even before it is placed in an area. Add a suite to an area by extending its regex; add an area with one rule line. All 38 current suites map with none dropped.Sequential, so no build isolation or gcov races - this trades nothing for the readability, unlike a concurrent or matrix split which would each cost extra builds or runners.
Summary by CodeRabbit
testreport.xml.coverage_*.infoinputs into a unified coverage result.