ninja test exits 0 when tests fail to register, so a change that breaks compilation of a test file silently removes those tests from the run and still reports success. Nothing in the harness or in CI compares the number of tests executed against a baseline.
Concrete case
While verifying PR #1812 on x86-64 Linux (--build-mode=bytecode) I hit this:
| tree |
Successes: |
exit |
aborted compilation units |
upstream/main |
1963 |
0 |
2 (both expected) |
| PR #1812 |
1846 |
0 |
3 |
117 fewer successes, and ninja test reports success in both cases. The cause was a compile-file abort in a regression-test file, so ELT-*, EQUALP-*, ASSOC-*, BUTLAST-* and CAN-MAP-TO-SPECIALIZED-VECTORS-* — 122 tests — never registered. Cross-checking the actual Passed <NAME> lines against the baseline confirmed it: 122 present on main and absent on the PR, 5 new ones added by the PR.
Four CI jobs on that PR were green while roughly 6% of the suite did not run.
Why the current check misses it
run-all.lisp fails the run on tests in *unexpected-failed-tests*. A test that was never defined cannot be in that list, so it contributes nothing to the exit status. The summary does print Successes:, but nothing consumes it, and CI does not diff it against anything.
*files-failed-to-compile* appears to exist for roughly this purpose — but a compilation unit aborted during test-file compilation did not surface as a failure in my runs.
Suggested fixes, roughly in order of value
- Fail the run if any test file fails to compile. This is the direct cause and the sharpest signal.
compilation unit aborted while loading a suite should be fatal, or at minimum reported in the summary rather than only in the log.
- Print the expected test count and fail on a shortfall. Even a coarse floor — a committed baseline count, or "fail if successes dropped by more than N since the last run" — would have caught this immediately.
- Report the count of registered tests per suite so a diff against a previous run is mechanical rather than requiring
Passed-line extraction.
Diagnostic that works today
For anyone comparing runs by hand, grep -c 'compilation unit aborted' is a usable proxy: 2 is normal on a healthy tree (the deliberate LATIN2-CHECK decoding-error test and a LOAD-STREAM case), and 3 or more means a file died and took its tests with it.
Filing this separately from the PR because it is independent of that change — any future PR that breaks compilation of a test file will pass CI the same way.
ninja testexits 0 when tests fail to register, so a change that breaks compilation of a test file silently removes those tests from the run and still reports success. Nothing in the harness or in CI compares the number of tests executed against a baseline.Concrete case
While verifying PR #1812 on x86-64 Linux (
--build-mode=bytecode) I hit this:Successes:upstream/main117 fewer successes, and
ninja testreports success in both cases. The cause was acompile-fileabort in a regression-test file, soELT-*,EQUALP-*,ASSOC-*,BUTLAST-*andCAN-MAP-TO-SPECIALIZED-VECTORS-*— 122 tests — never registered. Cross-checking the actualPassed <NAME>lines against the baseline confirmed it: 122 present onmainand absent on the PR, 5 new ones added by the PR.Four CI jobs on that PR were green while roughly 6% of the suite did not run.
Why the current check misses it
run-all.lispfails the run on tests in*unexpected-failed-tests*. A test that was never defined cannot be in that list, so it contributes nothing to the exit status. The summary does printSuccesses:, but nothing consumes it, and CI does not diff it against anything.*files-failed-to-compile*appears to exist for roughly this purpose — but acompilation unit abortedduring test-file compilation did not surface as a failure in my runs.Suggested fixes, roughly in order of value
compilation unit abortedwhile loading a suite should be fatal, or at minimum reported in the summary rather than only in the log.Passed-line extraction.Diagnostic that works today
For anyone comparing runs by hand,
grep -c 'compilation unit aborted'is a usable proxy: 2 is normal on a healthy tree (the deliberateLATIN2-CHECKdecoding-error test and aLOAD-STREAMcase), and 3 or more means a file died and took its tests with it.Filing this separately from the PR because it is independent of that change — any future PR that breaks compilation of a test file will pass CI the same way.