Uh oh!
There was an error while loading. Please reload this page.
build(make): collect tests recursively so tests/ can mirror src/ - #958
Merged
Conversation
Prerequisite for #957. src/ is seventeen modules with no loose files; tests/unit is still 71 flat files with the module encoded in a filename prefix. Mirroring the layout needs `make test` to find nested tests first. The Makefile collected with $(wildcard tests/*/*[tT]est.sh), which matches exactly one level. `./bashunit tests/` already recurses, so a nested test would have been run by three of the four CI matrix entries and silently skipped by `make test` -- green, and quietly testing less. Recursive collection alone is not the fix. A naive recursive glob also picks up four files under fixtures/ that are inputs to other tests rather than tests: tests/acceptance/fixtures/tests_path/{a,other}_test.sh tests/unit/fixtures/tests/example{1,2}_test.sh so fixtures/ is excluded by path. Collection is unchanged today: the same 156 files, byte for byte, verified against the old glob's output. tests/unit/project/collection_test.sh keeps it honest, and is itself nested -- the old glob would not have collected it. It asserts that every test file on disk reaches `make test`, that no fixture does, and that fixtures which would be swept in still exist, so the second assertion cannot pass vacuously. Both failure modes were verified by mutation: reverting to the one-level glob reddens the first test, dropping the fixtures exclusion reddens the second. The suite total goes 1626 -> 1629, which is exactly the three new tests.
Run from inside `make test`, the sub-make in collection_from_make inherits MAKEFLAGS and wraps its output in `make[1]: Entering/Leaving directory`. The positional `tail -n +2` stripped the Entering line instead of the header, so the comparison saw both the header and the trailing Leaving line and failed on the four make-driven CI jobs while every `./bashunit tests/` job passed. Filter on a `tests/` prefix instead of stripping a fixed number of lines, and clear MAKEFLAGS so the sub-make cannot inherit -j or a parent goal. Reproduced locally by running the guard through a wrapper Makefile, which fails before this change and passes after.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤔 Background
Related #957 — prerequisite for every PR in that issue.
src/is 17 modules with no loose files.tests/unit/is still 71 flat files with the module encoded in a filename prefix. Mirroring the layout needsmake testto find nested tests first.Makefile:67collected with$(wildcard tests/*/*[tT]est.sh)— exactly one level../bashunit tests/already recurses (verified). So the moment a test is nested, three of the four CI matrix entries would run it andmake testwould silently skip it — green, and quietly testing less.💡 Why "just make it recursive" is wrong
A naive recursive glob also collects four files under
fixtures/that are inputs to other tests, not tests:So
fixtures/is excluded by path. Collection is unchanged today — the same 156 files, verified byte-for-byte against the old glob's output. This PR moves nothing; it only makes nesting possible.✅ The guard
tests/unit/project/collection_test.sh— itself nested, so the old glob wouldn't have collected it. Three assertions:make testBoth failure modes mutation-verified: reverting to the one-level glob reddens the first, dropping the
fixtures/exclusion reddens the second.✅ Verification
Suite total 1626 → 1629 — exactly the three new tests, nothing lost.
(
make testreports a different total from./bashunit tests/for pre-existing mode-dependent skips, not collection differences — which is why the guard compares file lists rather than totals.)Green: sequential ·
--parallel --simple --strict·make sa·make lint·bash build.sh bin -v→✅ Build verified ✅.