Skip to content

Commit 83a58c2

Browse files
authored
ci-coach: Add test integrity safeguards and canary job integration (#12406)
1 parent a0b6ac4 commit 83a58c2

4 files changed

Lines changed: 140 additions & 9 deletions

File tree

.github/workflows/ci-coach.lock.yml

Lines changed: 42 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/ci-coach.md

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,15 @@ Follow the optimization strategies defined in the `ci-optimization-strategies` s
6868
- Check for orphaned tests not covered by any CI job
6969
- Verify catch-all matrix groups exist for packages with specific patterns
7070
- Identify coverage gaps and propose fixes if needed
71+
- **Use canary job outputs** to detect missing tests:
72+
- Review `test-coverage-analysis` artifact from the `canary_go` job
73+
- The canary job compares `all-tests.txt` (all tests in codebase) vs `executed-tests.txt` (tests that actually ran)
74+
- If canary job fails, investigate which tests are missing from the CI matrix
75+
- Ensure all tests defined in `*_test.go` files are covered by at least one test job pattern
76+
- **Verify test suite integrity**:
77+
- Check that the test suite FAILS when individual tests fail (not just reporting failures)
78+
- Review test job exit codes - ensure failed tests cause the job to exit with non-zero status
79+
- Validate that test result artifacts show actual test failures, not swallowed errors
7180
- **Analyze fuzz test performance**: Review fuzz test results in `/tmp/ci-artifacts/*/fuzz-results/`
7281
- Check for new crash inputs or interesting corpus growth
7382
- Evaluate fuzz test duration (currently 10s per test)
@@ -247,6 +256,33 @@ integration:
247256
248257
## Important Guidelines
249258
259+
### Test Code Integrity (CRITICAL)
260+
261+
**NEVER MODIFY TEST CODE TO HIDE ERRORS**
262+
263+
The CI Coach workflow must NEVER alter test code (`*_test.go` files) in ways that:
264+
- Swallow errors or suppress failures
265+
- Make failing tests appear to pass
266+
- Add error suppression patterns like `|| true`, `|| :`, or `|| echo "ignoring"`
267+
- Wrap test execution with `set +e` or similar error-ignoring constructs
268+
- Comment out failing assertions
269+
- Skip or disable tests without documented justification
270+
271+
**Test Suite Validation Requirements**:
272+
- The test suite MUST fail when individual tests fail
273+
- Failed tests MUST cause the CI job to exit with non-zero status
274+
- Test artifacts must accurately reflect actual test results
275+
- If tests are reported as failing, the entire test job must fail
276+
- Never sacrifice test integrity for optimization
277+
278+
**If tests are failing**:
279+
1. ✅ **DO**: Fix the root cause of the test failure
280+
2. ✅ **DO**: Update CI matrix patterns if tests are miscategorized
281+
3. ✅ **DO**: Investigate why tests fail and propose proper fixes
282+
4. ❌ **DON'T**: Modify test code to hide errors
283+
5. ❌ **DON'T**: Suppress error output from test commands
284+
6. ❌ **DON'T**: Change exit codes to make failures look like successes
285+
250286
### Quality Standards
251287
- **Evidence-based**: All recommendations must be based on actual data analysis
252288
- **Minimal changes**: Make surgical improvements, not wholesale rewrites
@@ -262,6 +298,12 @@ integration:
262298
- **Keep security** controls in place
263299
- **Document trade-offs** clearly
264300
- **Only create PR if validations pass** - don't propose broken changes
301+
- **NEVER change test code to hide errors**:
302+
- NEVER modify test files (`*_test.go`) to swallow errors or ignore failures
303+
- NEVER add `|| true` or similar patterns to make failing tests appear to pass
304+
- NEVER wrap test commands with error suppression (e.g., `set +e`, `|| echo "ignoring"`)
305+
- If tests are failing, fix the root cause or update the CI matrix, not the test code
306+
- Test code integrity is non-negotiable - tests must accurately reflect pass/fail status
265307
266308
### Analysis Discipline
267309
- **Use pre-downloaded data** - all data is already available

.github/workflows/dev.lock.yml

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)