Nailed native test numbers so PRs don't forget to increment#11117
Nailed native test numbers so PRs don't forget to increment#11117NomDeTom wants to merge 6 commits into
Conversation
platformio test discovers and runs whatever test_* directories exist, so it never notices when test/native-suite-count drifts from the actual directory count. That reconciliation lived only in bin/run-tests.sh, which CI does not invoke - so a stale or unbumped count sailed through PRs (develop itself shipped 38 dirs against a 37 file). Add a standalone suite-count-check job to test_native.yml that mirrors run-tests.sh's exact counting logic and fails on a mismatch. It runs on every PR via main_matrix's test-native job, with no build step so it fails fast. Bump native-suite-count to 38 to match the current suites. clod helped too
📝 WalkthroughWalkthroughAdds a native test workflow job that validates ChangesNative suite count validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In @.github/workflows/test_native.yml:
- Line 24: Update the actions/checkout step to set persist-credentials to false,
ensuring the workflow does not retain the GITHUB_TOKEN in local Git
configuration while preserving the existing checkout behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c589e66-b0c8-4fd2-b7a4-a0ada549d1c0
📒 Files selected for processing (1)
.github/workflows/test_native.yml
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds a CI guard to ensure the manually maintained test/native-suite-count stays in sync with the actual number of test/test_* suite directories, preventing PRs from adding/removing suites without updating the count.
Changes:
- Introduces a new GitHub Actions job to reconcile
test/native-suite-countwith discoveredtest/test_*directories. - Fails CI with a targeted
::errormessage when the count drifts, including actionable hints.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/test_native.yml (1)
60-60: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSkip report generation when the suite-count gate fails.
When
suite-count-checkfails, both dependent test jobs are skipped. Becausegenerate-reportsstill runs withif: always(), it can attempt to download artifacts that were never produced and fail with a secondary, misleading error. Addsuite-count-checktogenerate-reports.needsand require that gate to succeed while preservingalways()for actual test failures.Suggested fix
needs: + - suite-count-check - simulator-tests - platformio-tests - if: always() + if: ${{ always() && needs.suite-count-check.result == 'success' }}Also applies to: 135-135
🤖 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 at line 60, Update the generate-reports job’s needs configuration to include suite-count-check, and adjust its condition so the suite-count gate must succeed while always() still allows report generation after test-job failures. Preserve the existing dependent test-job requirements and artifact-reporting 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.
Nitpick comments:
In @.github/workflows/test_native.yml:
- Line 60: Update the generate-reports job’s needs configuration to include
suite-count-check, and adjust its condition so the suite-count gate must succeed
while always() still allows report generation after test-job failures. Preserve
the existing dependent test-job requirements and artifact-reporting behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0a04584f-92ac-48ca-b972-bd23530b15c1
📒 Files selected for processing (1)
.github/workflows/test_native.yml
⚡ Try this PR in the Web FlasherWarning 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-21. Updated for |
There's a manual count of the number of native tests - this is needed because previously tests were skipped silently, and so local testing wouldn't mirror the CI testing.
Now more tests are being added, but it's not being incremented - presumably people are running their own test suites. This PR prevents people adding tests without incrementing the number.
🤝 Attestations
Summary by CodeRabbit
test_*suites.