Summary
src/ is now 17 modules with no loose files (#931, #940, #948, #949, ADR-011). tests/unit/
is still 71 flat files — assert_advanced_test.sh, coverage_engine_test.sh,
runner_exec_test.sh, … — with the module encoded in a filename prefix rather than in the
tree.
Mirror the source layout, the way PHPUnit projects mirror src/ into tests/:
src/coverage/engine.sh -> tests/unit/coverage/engine_test.sh
src/runner/exec.sh -> tests/unit/runner/exec_test.sh
Finding the tests for a module becomes ls tests/unit/<module>/ instead of remembering a
prefix convention.
Blocker: make test only globs one level
This must be fixed first, in its own PR, or tests silently stop running.
Makefile:67:
TEST_SCRIPTS = $(wildcard$(TEST_SCRIPTS_DIR)/*/*[tT]est.sh)
That matches tests/<dir>/<name>_test.sh and nothing deeper. Nest a test and make test
skips it and stays green — and make test is its own CI matrix entry plus the Linux and
macOS jobs.
./bashunit tests/ already recurses (verified: it finds unit/sub/deep_test.sh), so the other
three matrix entries would keep running everything. The two would silently disagree.
The fix is not simply "make the glob recursive"
A naive recursive glob sweeps in four fixture files that are inputs to other tests, not
tests:
tests/acceptance/fixtures/tests_path/a_test.sh
tests/acceptance/fixtures/tests_path/other_test.sh
tests/unit/fixtures/tests/example1_test.sh
tests/unit/fixtures/tests/example2_test.sh
(156 files today, 160 with a naive recursive glob.)
So:
TEST_SCRIPTS = $(shell find $(TEST_SCRIPTS_DIR) -name '*[tT]est.sh' -not -path '*/fixtures/*')
and a guard test asserting make test's collected list equals the set of real test files —
otherwise this regresses silently the next time someone nests a directory.
Proposed layout
One directory per source module, same names:
tests/unit/<dir>/ | Files today | From |
|---|
assert/ | 12 | assert_* (10), directory_test, file_test |
coverage/ | 9 | coverage_* |
runner/ | 6 | runner_* (5), setup_teardown_test |
cli/ | 5 | watch* (3), doc_test, upgrade_test |
config/ | 4 | env_test, env_deprecated_aliases_test, parallel_test, rerun_test |
api/ | 4 | globals_test, skip_todo_test, test_title_test, custom_assertions_test |
console/ | 4 | console_* (3), colors_test |
helper/ | 3 | helpers* |
system/ | 3 | check_os_test, dependencies_test, io_test |
util/ | 3 | clock_test, math_test, str_test |
reports/ | 2 | reports_test, reports_json_test |
doubles/ | 1 | test_doubles_test |
state/ | 1 | state_test |
main/ | 2 | main_test, completions_test |
learn/ | 1 | learn_test |
benchmark/ | 1 | benchmark_test |
The remainder does not mirror src/, and should not pretend to
Ten files test the project's own tooling and invariants, not a source module:
release_generation · release_sandbox · release_update · release_utilities ·
release_validation — cover tools/release.sh; zero src/ referencespackage_json_test — covers package.jsonbuild_test — covers build.shbash_version_test — covers the entrypoint's version gatebash_compatibility_test — greps all of src/, belongs to no single moduleredirect_error_test — behavioural, no single owner
Proposal: tests/unit/project/. Naming it something honest beats forcing it under a module
name that would be a lie.
Phasing
- Makefile + guard test. Recursive collection excluding
fixtures/, plus the test that
keeps it honest. No files move. This PR is a prerequisite for every one below. - One PR per module directory, largest first (
assert/, coverage/, runner/, …). tests/unit/project/ last, once only the remainder is left flat.
tests/functional/ and tests/acceptance/ stay flat for now — they are organised by scenario
rather than by source module, so the mirror argument does not apply. Revisit separately if it
ever does.
The verification that matters
The reported test total must not change. Before and after each PR:
./bashunit tests/ # Tests: N passed ... T total
make test# must collect the same set
If a file stops being collected, T drops. That single number is the safety net against the
silent-skip failure this whole issue is designed around — check it on every PR, not just the
first.
Also per PR: make sa · make lint · ./bashunit --parallel --simple --strict tests/ ·
bash build.sh bin -v.
Constraints
- Fixtures must never be collected as tests. They live under
fixtures/ and are excluded
by path. Fixture files are also named test_*.sh (prefix) rather than *_test.sh (suffix)
precisely so the old glob missed them — keep both guards. - Tests reference fixtures by relative path (
$(bashunit::current_dir)/fixtures/...). Moving a
test one directory deeper breaks those paths. Grep each file for current_dir,
BASH_SOURCE and fixtures/ before moving it. - Several tests reach into
src/ by path — tests/unit/build_test.sh,
tests/unit/state_test.sh, tests/unit/completions_test.sh,
tests/unit/env_deprecated_aliases_test.sh, tests/unit/helpers_test.sh,
tests/unit/check_os_test.sh. Those paths are repo-relative, so moving the test does not
break them — but confirm rather than assume. - Do not rename test functions; only file paths change.
Acceptance criteria (per PR)
Do not
- Do not move any test before the Makefile PR lands
- Do not make the glob recursive without excluding
fixtures/ - Do not create
tests/unit/<module>/<submodule>/ — one level of mirroring is the goal - Do not rename test functions
Summary
src/is now 17 modules with no loose files (#931, #940, #948, #949, ADR-011).tests/unit/is still 71 flat files —
assert_advanced_test.sh,coverage_engine_test.sh,runner_exec_test.sh, … — with the module encoded in a filename prefix rather than in thetree.
Mirror the source layout, the way PHPUnit projects mirror
src/intotests/:Finding the tests for a module becomes
ls tests/unit/<module>/instead of remembering aprefix convention.
Blocker:
make testonly globs one levelThis must be fixed first, in its own PR, or tests silently stop running.
Makefile:67:That matches
tests/<dir>/<name>_test.shand nothing deeper. Nest a test andmake testskips it and stays green — and
make testis its own CI matrix entry plus the Linux andmacOS jobs.
./bashunit tests/already recurses (verified: it findsunit/sub/deep_test.sh), so the otherthree matrix entries would keep running everything. The two would silently disagree.
The fix is not simply "make the glob recursive"
A naive recursive glob sweeps in four fixture files that are inputs to other tests, not
tests:
(156 files today, 160 with a naive recursive glob.)
So:
and a guard test asserting
make test's collected list equals the set of real test files —otherwise this regresses silently the next time someone nests a directory.
Proposed layout
One directory per source module, same names:
tests/unit/<dir>/assert/assert_*(10),directory_test,file_testcoverage/coverage_*runner/runner_*(5),setup_teardown_testcli/watch*(3),doc_test,upgrade_testconfig/env_test,env_deprecated_aliases_test,parallel_test,rerun_testapi/globals_test,skip_todo_test,test_title_test,custom_assertions_testconsole/console_*(3),colors_testhelper/helpers*system/check_os_test,dependencies_test,io_testutil/clock_test,math_test,str_testreports/reports_test,reports_json_testdoubles/test_doubles_teststate/state_testmain/main_test,completions_testlearn/learn_testbenchmark/benchmark_testThe remainder does not mirror
src/, and should not pretend toTen files test the project's own tooling and invariants, not a source module:
release_generation·release_sandbox·release_update·release_utilities·release_validation— covertools/release.sh; zerosrc/referencespackage_json_test— coverspackage.jsonbuild_test— coversbuild.shbash_version_test— covers the entrypoint's version gatebash_compatibility_test— greps all ofsrc/, belongs to no single moduleredirect_error_test— behavioural, no single ownerProposal:
tests/unit/project/. Naming it something honest beats forcing it under a modulename that would be a lie.
Phasing
fixtures/, plus the test thatkeeps it honest. No files move. This PR is a prerequisite for every one below.
assert/,coverage/,runner/, …).tests/unit/project/last, once only the remainder is left flat.tests/functional/andtests/acceptance/stay flat for now — they are organised by scenariorather than by source module, so the mirror argument does not apply. Revisit separately if it
ever does.
The verification that matters
The reported test total must not change. Before and after each PR:
If a file stops being collected,
Tdrops. That single number is the safety net against thesilent-skip failure this whole issue is designed around — check it on every PR, not just the
first.
Also per PR:
make sa·make lint·./bashunit --parallel --simple --strict tests/·bash build.sh bin -v.Constraints
fixtures/and are excludedby path. Fixture files are also named
test_*.sh(prefix) rather than*_test.sh(suffix)precisely so the old glob missed them — keep both guards.
$(bashunit::current_dir)/fixtures/...). Moving atest one directory deeper breaks those paths. Grep each file for
current_dir,BASH_SOURCEandfixtures/before moving it.src/by path —tests/unit/build_test.sh,tests/unit/state_test.sh,tests/unit/completions_test.sh,tests/unit/env_deprecated_aliases_test.sh,tests/unit/helpers_test.sh,tests/unit/check_os_test.sh. Those paths are repo-relative, so moving the test does notbreak them — but confirm rather than assume.
Acceptance criteria (per PR)
./bashunit tests/reports the same total as before the changemake testcollects the same set as./bashunit tests/make sa && make lintgreen./bashunit --parallel --simple --strict tests/greenbash build.sh bin -vprints✅ Build verified ✅Do not
fixtures/tests/unit/<module>/<submodule>/— one level of mirroring is the goal