Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude/rules/architecture-map.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,7 +48,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| Module | Owns |
|--------|------|
| `bashunit` + `main.sh` | entry, subcommand routing, flag parsing, run lifecycle, exit codes, cleanup calls |
| `runner.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/index.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/context.sh` | workdir restore, test identity/location exports, title interpolation, capability probes |
| `runner/payload.sh` | the `_BASHUNIT_RUNNER_*_OUT` return slots; encode/decode of the per-test result payload |
| `runner/diagnostics.sh` | runtime-error detection, kill-signal classification, profiling, verbose/file headers |
Expand All@@ -68,7 +68,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| `clock.sh` | time impl selection (EPOCHREALTIME > date > perl > …), return-slot reads |
| `str.sh` / `math.sh` / `io.sh` / `globals.sh` | pure-bash utilities; `globals.sh` has `temp_file`/`temp_dir` (public test API) |
| `test_doubles.sh` | spy/mock state via `_BASHUNIT_SPY_*` globals + files |
| `coverage.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/index.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) |
| `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches |
| `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` |
Expand Down
11 changes: 6 additions & 5 deletions .claude/rules/bash-style.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -120,7 +120,7 @@ local thing=$_BASHUNIT_PKG_THING_OUT

Examples in tree: `src/runner/payload.sh` (`_BASHUNIT_RUNNER_FIELD_OUT`,
`_BASHUNIT_RUNNER_TOTAL_OUT`, `_BASHUNIT_RUNNER_TYPE_OUT`, `_BASHUNIT_RUNNER_OUTPUT_OUT`),
`src/coverage.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).
`src/coverage/branches.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).

### When the helper builds dynamic variable names (mock/spy state)

Expand DownExpand Up@@ -158,10 +158,11 @@ function bashunit::pkg::do_thing() {

### Intentional dynamic-scope mutation is a separate pattern

The coverage branch helpers in `src/coverage.sh` (`_branch_push_if` and friends) deliberately
mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). That is
documented inline at `src/coverage.sh:818-821` and is **not** the outvar pattern — the
helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
The coverage branch helpers in `src/coverage/branches.sh` (`_branch_push_if` and friends)
deliberately mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`).
That is documented inline above the helpers in that file and is **not** the outvar pattern —
the helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
It is also why those helpers and `extract_branches` must stay in one file (ADR-010).
Don't introduce new instances of this pattern without an inline justification comment.

## ShellCheck
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,7 +3,7 @@
## Unreleased

### Changed
- Internal: `src/runner.sh` is split into a `src/runner/` module of ten single-responsibility filesbehind a `source`-only aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924)
- Internal: `src/runner.sh` and `src/coverage.sh` are split into `src/runner/` and `src/coverage/` modules of single-responsibility files, each behind a `source`-only `index.sh` aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924, #925)

### Fixed
- `build.sh` dedupes embedded files by repo-relative path. The previous basename key compared the top-level loop's relative paths against the recursion's absolute ones, so a file reached from two places could be bundled twice in the released binary; it also collided for same-named files in different directories (#923)
Expand Down
120 changes: 85 additions & 35 deletions adrs/adr-010-src-module-directories.md
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
# Splitting large `src/` files into module directories

* Status: accepted
* Status: accepted, amended 2026-08-01
* Deciders: Chemaclass
* Date: 2026-07-30

Technical Story: https://github.com/TypedDevs/bashunit/issues/924

> **Amendment, 2026-08-01.** The aggregator moves *inside* the module directory
> as `index.sh`, replacing the original `src/<module>.sh` beside it. The
> reasoning that first chose "beside" was wrong, and one of the two arguments
> that overturn it is a correctness bug rather than a preference — see
> [Aggregator placement](#aggregator-placement-amended-2026-08-01).

## Context and Problem Statement

`src/runner.sh` had grown to 2145 lines and 57 functions covering five unrelated
Expand DownExpand Up@@ -36,24 +42,60 @@ directory without changing what the released single-file binary does?

## Decision Outcome

Chosen option: **`src/runner/` directory behind a thin aggregator beside it**.

`src/runner.sh` keeps its single `source` line in the `bashunit` entrypoint and
becomes ten `source` lines plus comments. `build.sh` needs no per-module
knowledge: it already recurses into `source` lines, so the module children are
discovered through the aggregator.

The aggregator sits **beside** the directory rather than inside it as an
`index.sh`. Both are identical to `build.sh` — see the next section, the build
follows the `source` graph and cannot tell the difference — so the choice is
about the source tree, not the artifact. Beside wins on precedent: it is exactly
how `src/assertions.sh` has aggregated the flat `src/assert_*.sh` files since
long before module directories existed, so there is one aggregator convention in
`src/` rather than two. Inside would make each module a self-contained directory
and keep `ls src/` free of aggregator/directory pairs, which is the real argument
for it; it was rejected as not worth renaming shipped modules and rewriting
entrypoint lines for a cosmetic gain. Revisit only with a deliberate amendment
here — not per module, or `src/` ends up with both conventions.
Chosen option: **`src/runner/` directory with the aggregator inside it as
`index.sh`** (originally "beside it"; amended 2026-08-01, below).

The entrypoint keeps a single `source` line per module —
`src/runner/index.sh` — and that file is ten `source` lines plus comments.
`build.sh` needs no per-module knowledge: it already recurses into `source`
lines, so the module children are discovered through the aggregator.

### Aggregator placement (amended 2026-08-01)

The aggregator lives **inside** the module directory as `index.sh`:

```
src/runner/index.sh <- the module's entry point
src/runner/exec.sh
src/coverage/index.sh
src/coverage/engine.sh
```

Both placements are identical to `build.sh` — it follows the `source` graph and
cannot tell the difference (next section) — so this is a source-tree decision,
not an artifact one.

The original decision was "beside" (`src/runner.sh` + `src/runner/*.sh`), on the
grounds that it matched the `src/assertions.sh` → `src/assert_*.sh` precedent and
kept one aggregator convention in `src/`. **That reasoning was wrong.**
`src/assertions.sh` aggregates *flat* files; there is no `src/assert/` directory.
It is not a module aggregator at all, it is unaffected by this choice, and it
would remain exactly as it is under either placement. There was never a
"two conventions" risk to avoid.

Two arguments settle it for inside:

* **A hand-maintained aggregator list drifts; a glob cannot.** The rule that
aggregators hold only `source` lines is enforced by
`test_module_aggregators_hold_only_source_lines_and_comments`, which named its
aggregators in a string: `"src/assertions.sh src/runner.sh"`. `src/coverage.sh`
was added in #928 and never appended, so the rule silently stopped covering it
within one module of being introduced. With the aggregator at a predictable
`src/*/index.sh`, the test discovers modules by glob and cannot drift. This is
the deciding argument: it is a correctness property, not a preference.
* **The cost only grows.** Converting two shipped modules costs two renames and
two entrypoint lines. #931 adds nine more; converting after it costs eleven.

Alongside those, the ordinary case for treating a directory as the module: `mv`
or `rm -r` moves it as one unit, an orphaned `src/runner.sh` left behind by a
deleted `src/runner/` becomes impossible, and `ls src/` lists modules rather than
aggregator/directory pairs. The convention also matches what most ecosystems do —
`index.ts`, `__init__.py`, `mod.rs`.

`index.sh` over `main.sh` or `init.sh`: both of those collide with existing
concepts here (`src/main.sh`, and `src/init.sh` implementing `bashunit init`).

Apply this to every module. Mixing placements is the outcome worth avoiding.

This required fixing `build.sh` first (#923). Its embed dedupe was keyed on a
file's *basename*, which both hid a genuine double-embed (the top-level loop
Expand DownExpand Up@@ -161,27 +203,35 @@ context · payload · diagnostics → parallel · hooks · result → provider
grouping in a filename prefix rather than in the directory structure.
* Bad, because it does not generalise — `src/coverage.sh` would add nine more.

### `src/runner/` directory behind an aggregator beside it
### `src/runner/` directory behind an aggregator beside it (rejected on amendment)

* Good, because it mirrors the existing `src/assertions.sh` → `src/assert_*.sh`
aggregator precedent, and `src/dev/debug.sh` already proved `src/` can nest.
* Good, because `build.sh` discovers children through recursion, so adding a
module file needs no build change.
* Good, because the entrypoint keeps one `source` line per module, unchanged in
shape from when the module was a single file.
* Good, because the entrypoint line keeps the shape it had when the module was a
single file.
* Bad, because the aggregator's path is unpredictable, so the test enforcing the
"only `source` lines" rule needs a hand-maintained list — which drifted the
first time a second module appeared (#928 added `src/coverage.sh` and never
appended it, silently un-enforcing the rule for that file).
* Bad, because `ls src/` shows an aggregator/directory pair per module, and the
pair can desync: deleting `src/runner/` leaves an orphaned `src/runner.sh`.
* Bad, because it required fixing the build's dedupe key first (#923).
* Bad, because `ls src/` shows an aggregator/directory pair per module.

### `src/runner/index.sh` inside the directory

* Good, because a module becomes one self-contained directory — moving or
deleting it is a single `mv`/`rm -r`, and `ls src/` shows modules, not pairs.
* Good, because it scales more cleanly if `src/` ever reaches a dozen modules.
* ~~Good, because it mirrors the `src/assertions.sh` → `src/assert_*.sh`
precedent~~ — withdrawn. `src/assertions.sh` aggregates flat files and has no
directory, so it is not a module aggregator and is unaffected either way.

### `src/runner/index.sh` inside the directory (chosen on amendment)

* Good, because the aggregator sits at a predictable `src/*/index.sh`, so the
enforcement test discovers modules by glob and cannot drift.
* Good, because a module becomes one self-contained directory — `mv`/`rm -r`
moves it as a unit, no orphan is possible, and `ls src/` lists modules.
* Good, because it matches the convention most ecosystems already use
(`index.ts`, `__init__.py`, `mod.rs`), so the entry point is where a reader
looks for it.
* Neutral on the build: identical to beside, which walks the `source` graph.
* Bad, because it would rename the two shipped modules and rewrite their
entrypoint lines for no behavioural gain.
* Bad, because `src/assertions.sh` would stay a beside-aggregator (it has no
directory), leaving `src/` with two aggregator conventions.
* Bad, because it cost renaming the two shipped modules and their entrypoint
lines — a cost that only grows, hence doing it before #931 adds nine more.

## Links

Expand Down
4 changes: 2 additions & 2 deletions bashunit
Original file line numberDiff line numberDiff line change
Expand Up@@ -73,7 +73,7 @@ source "$BASHUNIT_ROOT_DIR/src/io.sh"
source "$BASHUNIT_ROOT_DIR/src/math.sh"
source "$BASHUNIT_ROOT_DIR/src/parallel.sh"
source "$BASHUNIT_ROOT_DIR/src/env.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage/index.sh"
source "$BASHUNIT_ROOT_DIR/src/clock.sh"
source "$BASHUNIT_ROOT_DIR/src/state.sh"
source "$BASHUNIT_ROOT_DIR/src/colors.sh"
Expand All@@ -87,7 +87,7 @@ source "$BASHUNIT_ROOT_DIR/src/assertions.sh"
source "$BASHUNIT_ROOT_DIR/src/doc.sh"
source "$BASHUNIT_ROOT_DIR/src/reports.sh"
source "$BASHUNIT_ROOT_DIR/src/rerun.sh"
source "$BASHUNIT_ROOT_DIR/src/runner.sh"
source "$BASHUNIT_ROOT_DIR/src/runner/index.sh"
source "$BASHUNIT_ROOT_DIR/src/benchmark.sh"
source "$BASHUNIT_ROOT_DIR/src/bashunit.sh"
source "$BASHUNIT_ROOT_DIR/src/init.sh"
Expand Down
2 changes: 1 addition & 1 deletion src/coverage.sh → src/coverage/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/coverage/ module: only `source` lines and comments
# Entry point for the src/coverage/ module: only `source` lines and comments
# belong here. build.sh emits a file's body before recursing into its `source`
# lines, so any statement here would run before its dependencies in the built
# binary (adrs/adr-010-src-module-directories.md).
Expand Down
2 changes: 1 addition & 1 deletion src/runner.sh → src/runner/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/runner/ module: only `source` lines and comments belong
# Entry point for the src/runner/ module: only `source` lines and comments belong
# here. build.sh emits a file's body before recursing into its `source` lines, so
# any statement here would run before its dependencies in the built binary.
#
Expand Down
34 changes: 28 additions & 6 deletions tests/unit/build_test.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -91,22 +91,44 @@ function test_build_embed_docs_fails_on_missing_markers() {

# build::process_file emits a file's body and *then* recurses into its `source`
# lines, so an aggregator holding anything else at top level would run that code
# before its dependencies in the built binary but after them in dev mode. Add any
# new aggregator here.
function test_module_aggregators_hold_only_source_lines_and_comments() {
local aggregators="src/assertions.sh src/runner.sh"
# before its dependencies in the built binary but after them in dev mode.
#
# Discovered by glob, never by a hand-maintained list: the previous list named
# src/assertions.sh and src/runner.sh, and src/coverage.sh was added in #928
# without being appended, so the rule silently stopped covering it. A module's
# aggregator is src/<module>/index.sh (ADR-010); src/assertions.sh is the one
# flat-file aggregator, which has no directory of its own.
function build_aggregators() {
echo "src/assertions.sh"
local index
for index in "$ROOT_DIR"/src/*/index.sh; do
[ -f "$index" ] || continue
echo "${index#"$ROOT_DIR"/}"
done
}

function test_module_aggregators_hold_only_source_lines_and_comments() {
local offenders=""
local aggregator
for aggregator in $aggregators; do
while IFS= read -r aggregator; do
if grep -qvE '^[[:space:]]*(#|source |$)' "$ROOT_DIR/$aggregator"; then
offenders="$offenders $aggregator"
fi
done
done < <(build_aggregators)

assert_empty "$offenders"
}

# The glob above is only a safety net if it actually finds the modules.
function test_module_aggregator_discovery_finds_every_module() {
local found
found=$(build_aggregators | tr '\n' ' ')

assert_contains "src/runner/index.sh" "$found"
assert_contains "src/coverage/index.sh" "$found"
assert_contains "src/assertions.sh" "$found"
}

function test_build_process_file_embeds_a_file_only_once() {
local dir
dir=$(bashunit::temp_dir)
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
refactor(src): move module aggregators inside their directory as index.sh by Chemaclass · Pull Request #934 · TypedDevs/bashunit · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude/rules/architecture-map.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,7 +48,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| Module | Owns |
|--------|------|
| `bashunit` + `main.sh` | entry, subcommand routing, flag parsing, run lifecycle, exit codes, cleanup calls |
| `runner.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/index.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/context.sh` | workdir restore, test identity/location exports, title interpolation, capability probes |
| `runner/payload.sh` | the `_BASHUNIT_RUNNER_*_OUT` return slots; encode/decode of the per-test result payload |
| `runner/diagnostics.sh` | runtime-error detection, kill-signal classification, profiling, verbose/file headers |
Expand All@@ -68,7 +68,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| `clock.sh` | time impl selection (EPOCHREALTIME > date > perl > …), return-slot reads |
| `str.sh` / `math.sh` / `io.sh` / `globals.sh` | pure-bash utilities; `globals.sh` has `temp_file`/`temp_dir` (public test API) |
| `test_doubles.sh` | spy/mock state via `_BASHUNIT_SPY_*` globals + files |
| `coverage.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/index.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) |
| `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches |
| `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` |
Expand Down
11 changes: 6 additions & 5 deletions .claude/rules/bash-style.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -120,7 +120,7 @@ local thing=$_BASHUNIT_PKG_THING_OUT

Examples in tree: `src/runner/payload.sh` (`_BASHUNIT_RUNNER_FIELD_OUT`,
`_BASHUNIT_RUNNER_TOTAL_OUT`, `_BASHUNIT_RUNNER_TYPE_OUT`, `_BASHUNIT_RUNNER_OUTPUT_OUT`),
`src/coverage.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).
`src/coverage/branches.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).

### When the helper builds dynamic variable names (mock/spy state)

Expand DownExpand Up@@ -158,10 +158,11 @@ function bashunit::pkg::do_thing() {

### Intentional dynamic-scope mutation is a separate pattern

The coverage branch helpers in `src/coverage.sh` (`_branch_push_if` and friends) deliberately
mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). That is
documented inline at `src/coverage.sh:818-821` and is **not** the outvar pattern — the
helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
The coverage branch helpers in `src/coverage/branches.sh` (`_branch_push_if` and friends)
deliberately mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`).
That is documented inline above the helpers in that file and is **not** the outvar pattern —
the helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
It is also why those helpers and `extract_branches` must stay in one file (ADR-010).
Don't introduce new instances of this pattern without an inline justification comment.

## ShellCheck
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,7 +3,7 @@
## Unreleased

### Changed
- Internal: `src/runner.sh` is split into a `src/runner/` module of ten single-responsibility filesbehind a `source`-only aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924)
- Internal: `src/runner.sh` and `src/coverage.sh` are split into `src/runner/` and `src/coverage/` modules of single-responsibility files, each behind a `source`-only `index.sh` aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924, #925)

### Fixed
- `build.sh` dedupes embedded files by repo-relative path. The previous basename key compared the top-level loop's relative paths against the recursion's absolute ones, so a file reached from two places could be bundled twice in the released binary; it also collided for same-named files in different directories (#923)
Expand Down
120 changes: 85 additions & 35 deletions adrs/adr-010-src-module-directories.md
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
# Splitting large `src/` files into module directories

* Status: accepted
* Status: accepted, amended 2026-08-01
* Deciders: Chemaclass
* Date: 2026-07-30

Technical Story: https://github.com/TypedDevs/bashunit/issues/924

> **Amendment, 2026-08-01.** The aggregator moves *inside* the module directory
> as `index.sh`, replacing the original `src/<module>.sh` beside it. The
> reasoning that first chose "beside" was wrong, and one of the two arguments
> that overturn it is a correctness bug rather than a preference — see
> [Aggregator placement](#aggregator-placement-amended-2026-08-01).

## Context and Problem Statement

`src/runner.sh` had grown to 2145 lines and 57 functions covering five unrelated
Expand DownExpand Up@@ -36,24 +42,60 @@ directory without changing what the released single-file binary does?

## Decision Outcome

Chosen option: **`src/runner/` directory behind a thin aggregator beside it**.

`src/runner.sh` keeps its single `source` line in the `bashunit` entrypoint and
becomes ten `source` lines plus comments. `build.sh` needs no per-module
knowledge: it already recurses into `source` lines, so the module children are
discovered through the aggregator.

The aggregator sits **beside** the directory rather than inside it as an
`index.sh`. Both are identical to `build.sh` — see the next section, the build
follows the `source` graph and cannot tell the difference — so the choice is
about the source tree, not the artifact. Beside wins on precedent: it is exactly
how `src/assertions.sh` has aggregated the flat `src/assert_*.sh` files since
long before module directories existed, so there is one aggregator convention in
`src/` rather than two. Inside would make each module a self-contained directory
and keep `ls src/` free of aggregator/directory pairs, which is the real argument
for it; it was rejected as not worth renaming shipped modules and rewriting
entrypoint lines for a cosmetic gain. Revisit only with a deliberate amendment
here — not per module, or `src/` ends up with both conventions.
Chosen option: **`src/runner/` directory with the aggregator inside it as
`index.sh`** (originally "beside it"; amended 2026-08-01, below).

The entrypoint keeps a single `source` line per module —
`src/runner/index.sh` — and that file is ten `source` lines plus comments.
`build.sh` needs no per-module knowledge: it already recurses into `source`
lines, so the module children are discovered through the aggregator.

### Aggregator placement (amended 2026-08-01)

The aggregator lives **inside** the module directory as `index.sh`:

```
src/runner/index.sh <- the module's entry point
src/runner/exec.sh
src/coverage/index.sh
src/coverage/engine.sh
```

Both placements are identical to `build.sh` — it follows the `source` graph and
cannot tell the difference (next section) — so this is a source-tree decision,
not an artifact one.

The original decision was "beside" (`src/runner.sh` + `src/runner/*.sh`), on the
grounds that it matched the `src/assertions.sh` → `src/assert_*.sh` precedent and
kept one aggregator convention in `src/`. **That reasoning was wrong.**
`src/assertions.sh` aggregates *flat* files; there is no `src/assert/` directory.
It is not a module aggregator at all, it is unaffected by this choice, and it
would remain exactly as it is under either placement. There was never a
"two conventions" risk to avoid.

Two arguments settle it for inside:

* **A hand-maintained aggregator list drifts; a glob cannot.** The rule that
aggregators hold only `source` lines is enforced by
`test_module_aggregators_hold_only_source_lines_and_comments`, which named its
aggregators in a string: `"src/assertions.sh src/runner.sh"`. `src/coverage.sh`
was added in #928 and never appended, so the rule silently stopped covering it
within one module of being introduced. With the aggregator at a predictable
`src/*/index.sh`, the test discovers modules by glob and cannot drift. This is
the deciding argument: it is a correctness property, not a preference.
* **The cost only grows.** Converting two shipped modules costs two renames and
two entrypoint lines. #931 adds nine more; converting after it costs eleven.

Alongside those, the ordinary case for treating a directory as the module: `mv`
or `rm -r` moves it as one unit, an orphaned `src/runner.sh` left behind by a
deleted `src/runner/` becomes impossible, and `ls src/` lists modules rather than
aggregator/directory pairs. The convention also matches what most ecosystems do —
`index.ts`, `__init__.py`, `mod.rs`.

`index.sh` over `main.sh` or `init.sh`: both of those collide with existing
concepts here (`src/main.sh`, and `src/init.sh` implementing `bashunit init`).

Apply this to every module. Mixing placements is the outcome worth avoiding.

This required fixing `build.sh` first (#923). Its embed dedupe was keyed on a
file's *basename*, which both hid a genuine double-embed (the top-level loop
Expand DownExpand Up@@ -161,27 +203,35 @@ context · payload · diagnostics → parallel · hooks · result → provider
grouping in a filename prefix rather than in the directory structure.
* Bad, because it does not generalise — `src/coverage.sh` would add nine more.

### `src/runner/` directory behind an aggregator beside it
### `src/runner/` directory behind an aggregator beside it (rejected on amendment)

* Good, because it mirrors the existing `src/assertions.sh` → `src/assert_*.sh`
aggregator precedent, and `src/dev/debug.sh` already proved `src/` can nest.
* Good, because `build.sh` discovers children through recursion, so adding a
module file needs no build change.
* Good, because the entrypoint keeps one `source` line per module, unchanged in
shape from when the module was a single file.
* Good, because the entrypoint line keeps the shape it had when the module was a
single file.
* Bad, because the aggregator's path is unpredictable, so the test enforcing the
"only `source` lines" rule needs a hand-maintained list — which drifted the
first time a second module appeared (#928 added `src/coverage.sh` and never
appended it, silently un-enforcing the rule for that file).
* Bad, because `ls src/` shows an aggregator/directory pair per module, and the
pair can desync: deleting `src/runner/` leaves an orphaned `src/runner.sh`.
* Bad, because it required fixing the build's dedupe key first (#923).
* Bad, because `ls src/` shows an aggregator/directory pair per module.

### `src/runner/index.sh` inside the directory

* Good, because a module becomes one self-contained directory — moving or
deleting it is a single `mv`/`rm -r`, and `ls src/` shows modules, not pairs.
* Good, because it scales more cleanly if `src/` ever reaches a dozen modules.
* ~~Good, because it mirrors the `src/assertions.sh` → `src/assert_*.sh`
precedent~~ — withdrawn. `src/assertions.sh` aggregates flat files and has no
directory, so it is not a module aggregator and is unaffected either way.

### `src/runner/index.sh` inside the directory (chosen on amendment)

* Good, because the aggregator sits at a predictable `src/*/index.sh`, so the
enforcement test discovers modules by glob and cannot drift.
* Good, because a module becomes one self-contained directory — `mv`/`rm -r`
moves it as a unit, no orphan is possible, and `ls src/` lists modules.
* Good, because it matches the convention most ecosystems already use
(`index.ts`, `__init__.py`, `mod.rs`), so the entry point is where a reader
looks for it.
* Neutral on the build: identical to beside, which walks the `source` graph.
* Bad, because it would rename the two shipped modules and rewrite their
entrypoint lines for no behavioural gain.
* Bad, because `src/assertions.sh` would stay a beside-aggregator (it has no
directory), leaving `src/` with two aggregator conventions.
* Bad, because it cost renaming the two shipped modules and their entrypoint
lines — a cost that only grows, hence doing it before #931 adds nine more.

## Links

Expand Down
4 changes: 2 additions & 2 deletions bashunit
Original file line numberDiff line numberDiff line change
Expand Up@@ -73,7 +73,7 @@ source "$BASHUNIT_ROOT_DIR/src/io.sh"
source "$BASHUNIT_ROOT_DIR/src/math.sh"
source "$BASHUNIT_ROOT_DIR/src/parallel.sh"
source "$BASHUNIT_ROOT_DIR/src/env.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage/index.sh"
source "$BASHUNIT_ROOT_DIR/src/clock.sh"
source "$BASHUNIT_ROOT_DIR/src/state.sh"
source "$BASHUNIT_ROOT_DIR/src/colors.sh"
Expand All@@ -87,7 +87,7 @@ source "$BASHUNIT_ROOT_DIR/src/assertions.sh"
source "$BASHUNIT_ROOT_DIR/src/doc.sh"
source "$BASHUNIT_ROOT_DIR/src/reports.sh"
source "$BASHUNIT_ROOT_DIR/src/rerun.sh"
source "$BASHUNIT_ROOT_DIR/src/runner.sh"
source "$BASHUNIT_ROOT_DIR/src/runner/index.sh"
source "$BASHUNIT_ROOT_DIR/src/benchmark.sh"
source "$BASHUNIT_ROOT_DIR/src/bashunit.sh"
source "$BASHUNIT_ROOT_DIR/src/init.sh"
Expand Down
2 changes: 1 addition & 1 deletion src/coverage.sh → src/coverage/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/coverage/ module: only `source` lines and comments
# Entry point for the src/coverage/ module: only `source` lines and comments
# belong here. build.sh emits a file's body before recursing into its `source`
# lines, so any statement here would run before its dependencies in the built
# binary (adrs/adr-010-src-module-directories.md).
Expand Down
2 changes: 1 addition & 1 deletion src/runner.sh → src/runner/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/runner/ module: only `source` lines and comments belong
# Entry point for the src/runner/ module: only `source` lines and comments belong
# here. build.sh emits a file's body before recursing into its `source` lines, so
# any statement here would run before its dependencies in the built binary.
#
Expand Down
34 changes: 28 additions & 6 deletions tests/unit/build_test.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -91,22 +91,44 @@ function test_build_embed_docs_fails_on_missing_markers() {

# build::process_file emits a file's body and *then* recurses into its `source`
# lines, so an aggregator holding anything else at top level would run that code
# before its dependencies in the built binary but after them in dev mode. Add any
# new aggregator here.
function test_module_aggregators_hold_only_source_lines_and_comments() {
local aggregators="src/assertions.sh src/runner.sh"
# before its dependencies in the built binary but after them in dev mode.
#
# Discovered by glob, never by a hand-maintained list: the previous list named
# src/assertions.sh and src/runner.sh, and src/coverage.sh was added in #928
# without being appended, so the rule silently stopped covering it. A module's
# aggregator is src/<module>/index.sh (ADR-010); src/assertions.sh is the one
# flat-file aggregator, which has no directory of its own.
function build_aggregators() {
echo "src/assertions.sh"
local index
for index in "$ROOT_DIR"/src/*/index.sh; do
[ -f "$index" ] || continue
echo "${index#"$ROOT_DIR"/}"
done
}

function test_module_aggregators_hold_only_source_lines_and_comments() {
local offenders=""
local aggregator
for aggregator in $aggregators; do
while IFS= read -r aggregator; do
if grep -qvE '^[[:space:]]*(#|source |$)' "$ROOT_DIR/$aggregator"; then
offenders="$offenders $aggregator"
fi
done
done < <(build_aggregators)

assert_empty "$offenders"
}

# The glob above is only a safety net if it actually finds the modules.
function test_module_aggregator_discovery_finds_every_module() {
local found
found=$(build_aggregators | tr '\n' ' ')

assert_contains "src/runner/index.sh" "$found"
assert_contains "src/coverage/index.sh" "$found"
assert_contains "src/assertions.sh" "$found"
}

function test_build_process_file_embeds_a_file_only_once() {
local dir
dir=$(bashunit::temp_dir)
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' refactor(src): move module aggregators inside their directory as index.sh by Chemaclass · Pull Request #934 · TypedDevs/bashunit · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude/rules/architecture-map.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,7 +48,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| Module | Owns |
|--------|------|
| `bashunit` + `main.sh` | entry, subcommand routing, flag parsing, run lifecycle, exit codes, cleanup calls |
| `runner.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/index.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/context.sh` | workdir restore, test identity/location exports, title interpolation, capability probes |
| `runner/payload.sh` | the `_BASHUNIT_RUNNER_*_OUT` return slots; encode/decode of the per-test result payload |
| `runner/diagnostics.sh` | runtime-error detection, kill-signal classification, profiling, verbose/file headers |
Expand All@@ -68,7 +68,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| `clock.sh` | time impl selection (EPOCHREALTIME > date > perl > …), return-slot reads |
| `str.sh` / `math.sh` / `io.sh` / `globals.sh` | pure-bash utilities; `globals.sh` has `temp_file`/`temp_dir` (public test API) |
| `test_doubles.sh` | spy/mock state via `_BASHUNIT_SPY_*` globals + files |
| `coverage.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/index.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) |
| `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches |
| `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` |
Expand Down
11 changes: 6 additions & 5 deletions .claude/rules/bash-style.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -120,7 +120,7 @@ local thing=$_BASHUNIT_PKG_THING_OUT

Examples in tree: `src/runner/payload.sh` (`_BASHUNIT_RUNNER_FIELD_OUT`,
`_BASHUNIT_RUNNER_TOTAL_OUT`, `_BASHUNIT_RUNNER_TYPE_OUT`, `_BASHUNIT_RUNNER_OUTPUT_OUT`),
`src/coverage.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).
`src/coverage/branches.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).

### When the helper builds dynamic variable names (mock/spy state)

Expand DownExpand Up@@ -158,10 +158,11 @@ function bashunit::pkg::do_thing() {

### Intentional dynamic-scope mutation is a separate pattern

The coverage branch helpers in `src/coverage.sh` (`_branch_push_if` and friends) deliberately
mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). That is
documented inline at `src/coverage.sh:818-821` and is **not** the outvar pattern — the
helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
The coverage branch helpers in `src/coverage/branches.sh` (`_branch_push_if` and friends)
deliberately mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`).
That is documented inline above the helpers in that file and is **not** the outvar pattern —
the helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
It is also why those helpers and `extract_branches` must stay in one file (ADR-010).
Don't introduce new instances of this pattern without an inline justification comment.

## ShellCheck
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,7 +3,7 @@
## Unreleased

### Changed
- Internal: `src/runner.sh` is split into a `src/runner/` module of ten single-responsibility filesbehind a `source`-only aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924)
- Internal: `src/runner.sh` and `src/coverage.sh` are split into `src/runner/` and `src/coverage/` modules of single-responsibility files, each behind a `source`-only `index.sh` aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924, #925)

### Fixed
- `build.sh` dedupes embedded files by repo-relative path. The previous basename key compared the top-level loop's relative paths against the recursion's absolute ones, so a file reached from two places could be bundled twice in the released binary; it also collided for same-named files in different directories (#923)
Expand Down
120 changes: 85 additions & 35 deletions adrs/adr-010-src-module-directories.md
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
# Splitting large `src/` files into module directories

* Status: accepted
* Status: accepted, amended 2026-08-01
* Deciders: Chemaclass
* Date: 2026-07-30

Technical Story: https://github.com/TypedDevs/bashunit/issues/924

> **Amendment, 2026-08-01.** The aggregator moves *inside* the module directory
> as `index.sh`, replacing the original `src/<module>.sh` beside it. The
> reasoning that first chose "beside" was wrong, and one of the two arguments
> that overturn it is a correctness bug rather than a preference — see
> [Aggregator placement](#aggregator-placement-amended-2026-08-01).

## Context and Problem Statement

`src/runner.sh` had grown to 2145 lines and 57 functions covering five unrelated
Expand DownExpand Up@@ -36,24 +42,60 @@ directory without changing what the released single-file binary does?

## Decision Outcome

Chosen option: **`src/runner/` directory behind a thin aggregator beside it**.

`src/runner.sh` keeps its single `source` line in the `bashunit` entrypoint and
becomes ten `source` lines plus comments. `build.sh` needs no per-module
knowledge: it already recurses into `source` lines, so the module children are
discovered through the aggregator.

The aggregator sits **beside** the directory rather than inside it as an
`index.sh`. Both are identical to `build.sh` — see the next section, the build
follows the `source` graph and cannot tell the difference — so the choice is
about the source tree, not the artifact. Beside wins on precedent: it is exactly
how `src/assertions.sh` has aggregated the flat `src/assert_*.sh` files since
long before module directories existed, so there is one aggregator convention in
`src/` rather than two. Inside would make each module a self-contained directory
and keep `ls src/` free of aggregator/directory pairs, which is the real argument
for it; it was rejected as not worth renaming shipped modules and rewriting
entrypoint lines for a cosmetic gain. Revisit only with a deliberate amendment
here — not per module, or `src/` ends up with both conventions.
Chosen option: **`src/runner/` directory with the aggregator inside it as
`index.sh`** (originally "beside it"; amended 2026-08-01, below).

The entrypoint keeps a single `source` line per module —
`src/runner/index.sh` — and that file is ten `source` lines plus comments.
`build.sh` needs no per-module knowledge: it already recurses into `source`
lines, so the module children are discovered through the aggregator.

### Aggregator placement (amended 2026-08-01)

The aggregator lives **inside** the module directory as `index.sh`:

```
src/runner/index.sh <- the module's entry point
src/runner/exec.sh
src/coverage/index.sh
src/coverage/engine.sh
```

Both placements are identical to `build.sh` — it follows the `source` graph and
cannot tell the difference (next section) — so this is a source-tree decision,
not an artifact one.

The original decision was "beside" (`src/runner.sh` + `src/runner/*.sh`), on the
grounds that it matched the `src/assertions.sh` → `src/assert_*.sh` precedent and
kept one aggregator convention in `src/`. **That reasoning was wrong.**
`src/assertions.sh` aggregates *flat* files; there is no `src/assert/` directory.
It is not a module aggregator at all, it is unaffected by this choice, and it
would remain exactly as it is under either placement. There was never a
"two conventions" risk to avoid.

Two arguments settle it for inside:

* **A hand-maintained aggregator list drifts; a glob cannot.** The rule that
aggregators hold only `source` lines is enforced by
`test_module_aggregators_hold_only_source_lines_and_comments`, which named its
aggregators in a string: `"src/assertions.sh src/runner.sh"`. `src/coverage.sh`
was added in #928 and never appended, so the rule silently stopped covering it
within one module of being introduced. With the aggregator at a predictable
`src/*/index.sh`, the test discovers modules by glob and cannot drift. This is
the deciding argument: it is a correctness property, not a preference.
* **The cost only grows.** Converting two shipped modules costs two renames and
two entrypoint lines. #931 adds nine more; converting after it costs eleven.

Alongside those, the ordinary case for treating a directory as the module: `mv`
or `rm -r` moves it as one unit, an orphaned `src/runner.sh` left behind by a
deleted `src/runner/` becomes impossible, and `ls src/` lists modules rather than
aggregator/directory pairs. The convention also matches what most ecosystems do —
`index.ts`, `__init__.py`, `mod.rs`.

`index.sh` over `main.sh` or `init.sh`: both of those collide with existing
concepts here (`src/main.sh`, and `src/init.sh` implementing `bashunit init`).

Apply this to every module. Mixing placements is the outcome worth avoiding.

This required fixing `build.sh` first (#923). Its embed dedupe was keyed on a
file's *basename*, which both hid a genuine double-embed (the top-level loop
Expand DownExpand Up@@ -161,27 +203,35 @@ context · payload · diagnostics → parallel · hooks · result → provider
grouping in a filename prefix rather than in the directory structure.
* Bad, because it does not generalise — `src/coverage.sh` would add nine more.

### `src/runner/` directory behind an aggregator beside it
### `src/runner/` directory behind an aggregator beside it (rejected on amendment)

* Good, because it mirrors the existing `src/assertions.sh` → `src/assert_*.sh`
aggregator precedent, and `src/dev/debug.sh` already proved `src/` can nest.
* Good, because `build.sh` discovers children through recursion, so adding a
module file needs no build change.
* Good, because the entrypoint keeps one `source` line per module, unchanged in
shape from when the module was a single file.
* Good, because the entrypoint line keeps the shape it had when the module was a
single file.
* Bad, because the aggregator's path is unpredictable, so the test enforcing the
"only `source` lines" rule needs a hand-maintained list — which drifted the
first time a second module appeared (#928 added `src/coverage.sh` and never
appended it, silently un-enforcing the rule for that file).
* Bad, because `ls src/` shows an aggregator/directory pair per module, and the
pair can desync: deleting `src/runner/` leaves an orphaned `src/runner.sh`.
* Bad, because it required fixing the build's dedupe key first (#923).
* Bad, because `ls src/` shows an aggregator/directory pair per module.

### `src/runner/index.sh` inside the directory

* Good, because a module becomes one self-contained directory — moving or
deleting it is a single `mv`/`rm -r`, and `ls src/` shows modules, not pairs.
* Good, because it scales more cleanly if `src/` ever reaches a dozen modules.
* ~~Good, because it mirrors the `src/assertions.sh` → `src/assert_*.sh`
precedent~~ — withdrawn. `src/assertions.sh` aggregates flat files and has no
directory, so it is not a module aggregator and is unaffected either way.

### `src/runner/index.sh` inside the directory (chosen on amendment)

* Good, because the aggregator sits at a predictable `src/*/index.sh`, so the
enforcement test discovers modules by glob and cannot drift.
* Good, because a module becomes one self-contained directory — `mv`/`rm -r`
moves it as a unit, no orphan is possible, and `ls src/` lists modules.
* Good, because it matches the convention most ecosystems already use
(`index.ts`, `__init__.py`, `mod.rs`), so the entry point is where a reader
looks for it.
* Neutral on the build: identical to beside, which walks the `source` graph.
* Bad, because it would rename the two shipped modules and rewrite their
entrypoint lines for no behavioural gain.
* Bad, because `src/assertions.sh` would stay a beside-aggregator (it has no
directory), leaving `src/` with two aggregator conventions.
* Bad, because it cost renaming the two shipped modules and their entrypoint
lines — a cost that only grows, hence doing it before #931 adds nine more.

## Links

Expand Down
4 changes: 2 additions & 2 deletions bashunit
Original file line numberDiff line numberDiff line change
Expand Up@@ -73,7 +73,7 @@ source "$BASHUNIT_ROOT_DIR/src/io.sh"
source "$BASHUNIT_ROOT_DIR/src/math.sh"
source "$BASHUNIT_ROOT_DIR/src/parallel.sh"
source "$BASHUNIT_ROOT_DIR/src/env.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage/index.sh"
source "$BASHUNIT_ROOT_DIR/src/clock.sh"
source "$BASHUNIT_ROOT_DIR/src/state.sh"
source "$BASHUNIT_ROOT_DIR/src/colors.sh"
Expand All@@ -87,7 +87,7 @@ source "$BASHUNIT_ROOT_DIR/src/assertions.sh"
source "$BASHUNIT_ROOT_DIR/src/doc.sh"
source "$BASHUNIT_ROOT_DIR/src/reports.sh"
source "$BASHUNIT_ROOT_DIR/src/rerun.sh"
source "$BASHUNIT_ROOT_DIR/src/runner.sh"
source "$BASHUNIT_ROOT_DIR/src/runner/index.sh"
source "$BASHUNIT_ROOT_DIR/src/benchmark.sh"
source "$BASHUNIT_ROOT_DIR/src/bashunit.sh"
source "$BASHUNIT_ROOT_DIR/src/init.sh"
Expand Down
2 changes: 1 addition & 1 deletion src/coverage.sh → src/coverage/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/coverage/ module: only `source` lines and comments
# Entry point for the src/coverage/ module: only `source` lines and comments
# belong here. build.sh emits a file's body before recursing into its `source`
# lines, so any statement here would run before its dependencies in the built
# binary (adrs/adr-010-src-module-directories.md).
Expand Down
2 changes: 1 addition & 1 deletion src/runner.sh → src/runner/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/runner/ module: only `source` lines and comments belong
# Entry point for the src/runner/ module: only `source` lines and comments belong
# here. build.sh emits a file's body before recursing into its `source` lines, so
# any statement here would run before its dependencies in the built binary.
#
Expand Down
34 changes: 28 additions & 6 deletions tests/unit/build_test.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -91,22 +91,44 @@ function test_build_embed_docs_fails_on_missing_markers() {

# build::process_file emits a file's body and *then* recurses into its `source`
# lines, so an aggregator holding anything else at top level would run that code
# before its dependencies in the built binary but after them in dev mode. Add any
# new aggregator here.
function test_module_aggregators_hold_only_source_lines_and_comments() {
local aggregators="src/assertions.sh src/runner.sh"
# before its dependencies in the built binary but after them in dev mode.
#
# Discovered by glob, never by a hand-maintained list: the previous list named
# src/assertions.sh and src/runner.sh, and src/coverage.sh was added in #928
# without being appended, so the rule silently stopped covering it. A module's
# aggregator is src/<module>/index.sh (ADR-010); src/assertions.sh is the one
# flat-file aggregator, which has no directory of its own.
function build_aggregators() {
echo "src/assertions.sh"
local index
for index in "$ROOT_DIR"/src/*/index.sh; do
[ -f "$index" ] || continue
echo "${index#"$ROOT_DIR"/}"
done
}

function test_module_aggregators_hold_only_source_lines_and_comments() {
local offenders=""
local aggregator
for aggregator in $aggregators; do
while IFS= read -r aggregator; do
if grep -qvE '^[[:space:]]*(#|source |$)' "$ROOT_DIR/$aggregator"; then
offenders="$offenders $aggregator"
fi
done
done < <(build_aggregators)

assert_empty "$offenders"
}

# The glob above is only a safety net if it actually finds the modules.
function test_module_aggregator_discovery_finds_every_module() {
local found
found=$(build_aggregators | tr '\n' ' ')

assert_contains "src/runner/index.sh" "$found"
assert_contains "src/coverage/index.sh" "$found"
assert_contains "src/assertions.sh" "$found"
}

function test_build_process_file_embeds_a_file_only_once() {
local dir
dir=$(bashunit::temp_dir)
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' refactor(src): move module aggregators inside their directory as index.sh by Chemaclass · Pull Request #934 · TypedDevs/bashunit · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude/rules/architecture-map.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,7 +48,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| Module | Owns |
|--------|------|
| `bashunit` + `main.sh` | entry, subcommand routing, flag parsing, run lifecycle, exit codes, cleanup calls |
| `runner.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/index.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/context.sh` | workdir restore, test identity/location exports, title interpolation, capability probes |
| `runner/payload.sh` | the `_BASHUNIT_RUNNER_*_OUT` return slots; encode/decode of the per-test result payload |
| `runner/diagnostics.sh` | runtime-error detection, kill-signal classification, profiling, verbose/file headers |
Expand All@@ -68,7 +68,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| `clock.sh` | time impl selection (EPOCHREALTIME > date > perl > …), return-slot reads |
| `str.sh` / `math.sh` / `io.sh` / `globals.sh` | pure-bash utilities; `globals.sh` has `temp_file`/`temp_dir` (public test API) |
| `test_doubles.sh` | spy/mock state via `_BASHUNIT_SPY_*` globals + files |
| `coverage.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/index.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) |
| `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches |
| `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` |
Expand Down
11 changes: 6 additions & 5 deletions .claude/rules/bash-style.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -120,7 +120,7 @@ local thing=$_BASHUNIT_PKG_THING_OUT

Examples in tree: `src/runner/payload.sh` (`_BASHUNIT_RUNNER_FIELD_OUT`,
`_BASHUNIT_RUNNER_TOTAL_OUT`, `_BASHUNIT_RUNNER_TYPE_OUT`, `_BASHUNIT_RUNNER_OUTPUT_OUT`),
`src/coverage.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).
`src/coverage/branches.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).

### When the helper builds dynamic variable names (mock/spy state)

Expand DownExpand Up@@ -158,10 +158,11 @@ function bashunit::pkg::do_thing() {

### Intentional dynamic-scope mutation is a separate pattern

The coverage branch helpers in `src/coverage.sh` (`_branch_push_if` and friends) deliberately
mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). That is
documented inline at `src/coverage.sh:818-821` and is **not** the outvar pattern — the
helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
The coverage branch helpers in `src/coverage/branches.sh` (`_branch_push_if` and friends)
deliberately mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`).
That is documented inline above the helpers in that file and is **not** the outvar pattern —
the helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
It is also why those helpers and `extract_branches` must stay in one file (ADR-010).
Don't introduce new instances of this pattern without an inline justification comment.

## ShellCheck
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,7 +3,7 @@
## Unreleased

### Changed
- Internal: `src/runner.sh` is split into a `src/runner/` module of ten single-responsibility filesbehind a `source`-only aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924)
- Internal: `src/runner.sh` and `src/coverage.sh` are split into `src/runner/` and `src/coverage/` modules of single-responsibility files, each behind a `source`-only `index.sh` aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924, #925)

### Fixed
- `build.sh` dedupes embedded files by repo-relative path. The previous basename key compared the top-level loop's relative paths against the recursion's absolute ones, so a file reached from two places could be bundled twice in the released binary; it also collided for same-named files in different directories (#923)
Expand Down
120 changes: 85 additions & 35 deletions adrs/adr-010-src-module-directories.md
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
# Splitting large `src/` files into module directories

* Status: accepted
* Status: accepted, amended 2026-08-01
* Deciders: Chemaclass
* Date: 2026-07-30

Technical Story: https://github.com/TypedDevs/bashunit/issues/924

> **Amendment, 2026-08-01.** The aggregator moves *inside* the module directory
> as `index.sh`, replacing the original `src/<module>.sh` beside it. The
> reasoning that first chose "beside" was wrong, and one of the two arguments
> that overturn it is a correctness bug rather than a preference — see
> [Aggregator placement](#aggregator-placement-amended-2026-08-01).

## Context and Problem Statement

`src/runner.sh` had grown to 2145 lines and 57 functions covering five unrelated
Expand DownExpand Up@@ -36,24 +42,60 @@ directory without changing what the released single-file binary does?

## Decision Outcome

Chosen option: **`src/runner/` directory behind a thin aggregator beside it**.

`src/runner.sh` keeps its single `source` line in the `bashunit` entrypoint and
becomes ten `source` lines plus comments. `build.sh` needs no per-module
knowledge: it already recurses into `source` lines, so the module children are
discovered through the aggregator.

The aggregator sits **beside** the directory rather than inside it as an
`index.sh`. Both are identical to `build.sh` — see the next section, the build
follows the `source` graph and cannot tell the difference — so the choice is
about the source tree, not the artifact. Beside wins on precedent: it is exactly
how `src/assertions.sh` has aggregated the flat `src/assert_*.sh` files since
long before module directories existed, so there is one aggregator convention in
`src/` rather than two. Inside would make each module a self-contained directory
and keep `ls src/` free of aggregator/directory pairs, which is the real argument
for it; it was rejected as not worth renaming shipped modules and rewriting
entrypoint lines for a cosmetic gain. Revisit only with a deliberate amendment
here — not per module, or `src/` ends up with both conventions.
Chosen option: **`src/runner/` directory with the aggregator inside it as
`index.sh`** (originally "beside it"; amended 2026-08-01, below).

The entrypoint keeps a single `source` line per module —
`src/runner/index.sh` — and that file is ten `source` lines plus comments.
`build.sh` needs no per-module knowledge: it already recurses into `source`
lines, so the module children are discovered through the aggregator.

### Aggregator placement (amended 2026-08-01)

The aggregator lives **inside** the module directory as `index.sh`:

```
src/runner/index.sh <- the module's entry point
src/runner/exec.sh
src/coverage/index.sh
src/coverage/engine.sh
```

Both placements are identical to `build.sh` — it follows the `source` graph and
cannot tell the difference (next section) — so this is a source-tree decision,
not an artifact one.

The original decision was "beside" (`src/runner.sh` + `src/runner/*.sh`), on the
grounds that it matched the `src/assertions.sh` → `src/assert_*.sh` precedent and
kept one aggregator convention in `src/`. **That reasoning was wrong.**
`src/assertions.sh` aggregates *flat* files; there is no `src/assert/` directory.
It is not a module aggregator at all, it is unaffected by this choice, and it
would remain exactly as it is under either placement. There was never a
"two conventions" risk to avoid.

Two arguments settle it for inside:

* **A hand-maintained aggregator list drifts; a glob cannot.** The rule that
aggregators hold only `source` lines is enforced by
`test_module_aggregators_hold_only_source_lines_and_comments`, which named its
aggregators in a string: `"src/assertions.sh src/runner.sh"`. `src/coverage.sh`
was added in #928 and never appended, so the rule silently stopped covering it
within one module of being introduced. With the aggregator at a predictable
`src/*/index.sh`, the test discovers modules by glob and cannot drift. This is
the deciding argument: it is a correctness property, not a preference.
* **The cost only grows.** Converting two shipped modules costs two renames and
two entrypoint lines. #931 adds nine more; converting after it costs eleven.

Alongside those, the ordinary case for treating a directory as the module: `mv`
or `rm -r` moves it as one unit, an orphaned `src/runner.sh` left behind by a
deleted `src/runner/` becomes impossible, and `ls src/` lists modules rather than
aggregator/directory pairs. The convention also matches what most ecosystems do —
`index.ts`, `__init__.py`, `mod.rs`.

`index.sh` over `main.sh` or `init.sh`: both of those collide with existing
concepts here (`src/main.sh`, and `src/init.sh` implementing `bashunit init`).

Apply this to every module. Mixing placements is the outcome worth avoiding.

This required fixing `build.sh` first (#923). Its embed dedupe was keyed on a
file's *basename*, which both hid a genuine double-embed (the top-level loop
Expand DownExpand Up@@ -161,27 +203,35 @@ context · payload · diagnostics → parallel · hooks · result → provider
grouping in a filename prefix rather than in the directory structure.
* Bad, because it does not generalise — `src/coverage.sh` would add nine more.

### `src/runner/` directory behind an aggregator beside it
### `src/runner/` directory behind an aggregator beside it (rejected on amendment)

* Good, because it mirrors the existing `src/assertions.sh` → `src/assert_*.sh`
aggregator precedent, and `src/dev/debug.sh` already proved `src/` can nest.
* Good, because `build.sh` discovers children through recursion, so adding a
module file needs no build change.
* Good, because the entrypoint keeps one `source` line per module, unchanged in
shape from when the module was a single file.
* Good, because the entrypoint line keeps the shape it had when the module was a
single file.
* Bad, because the aggregator's path is unpredictable, so the test enforcing the
"only `source` lines" rule needs a hand-maintained list — which drifted the
first time a second module appeared (#928 added `src/coverage.sh` and never
appended it, silently un-enforcing the rule for that file).
* Bad, because `ls src/` shows an aggregator/directory pair per module, and the
pair can desync: deleting `src/runner/` leaves an orphaned `src/runner.sh`.
* Bad, because it required fixing the build's dedupe key first (#923).
* Bad, because `ls src/` shows an aggregator/directory pair per module.

### `src/runner/index.sh` inside the directory

* Good, because a module becomes one self-contained directory — moving or
deleting it is a single `mv`/`rm -r`, and `ls src/` shows modules, not pairs.
* Good, because it scales more cleanly if `src/` ever reaches a dozen modules.
* ~~Good, because it mirrors the `src/assertions.sh` → `src/assert_*.sh`
precedent~~ — withdrawn. `src/assertions.sh` aggregates flat files and has no
directory, so it is not a module aggregator and is unaffected either way.

### `src/runner/index.sh` inside the directory (chosen on amendment)

* Good, because the aggregator sits at a predictable `src/*/index.sh`, so the
enforcement test discovers modules by glob and cannot drift.
* Good, because a module becomes one self-contained directory — `mv`/`rm -r`
moves it as a unit, no orphan is possible, and `ls src/` lists modules.
* Good, because it matches the convention most ecosystems already use
(`index.ts`, `__init__.py`, `mod.rs`), so the entry point is where a reader
looks for it.
* Neutral on the build: identical to beside, which walks the `source` graph.
* Bad, because it would rename the two shipped modules and rewrite their
entrypoint lines for no behavioural gain.
* Bad, because `src/assertions.sh` would stay a beside-aggregator (it has no
directory), leaving `src/` with two aggregator conventions.
* Bad, because it cost renaming the two shipped modules and their entrypoint
lines — a cost that only grows, hence doing it before #931 adds nine more.

## Links

Expand Down
4 changes: 2 additions & 2 deletions bashunit
Original file line numberDiff line numberDiff line change
Expand Up@@ -73,7 +73,7 @@ source "$BASHUNIT_ROOT_DIR/src/io.sh"
source "$BASHUNIT_ROOT_DIR/src/math.sh"
source "$BASHUNIT_ROOT_DIR/src/parallel.sh"
source "$BASHUNIT_ROOT_DIR/src/env.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage/index.sh"
source "$BASHUNIT_ROOT_DIR/src/clock.sh"
source "$BASHUNIT_ROOT_DIR/src/state.sh"
source "$BASHUNIT_ROOT_DIR/src/colors.sh"
Expand All@@ -87,7 +87,7 @@ source "$BASHUNIT_ROOT_DIR/src/assertions.sh"
source "$BASHUNIT_ROOT_DIR/src/doc.sh"
source "$BASHUNIT_ROOT_DIR/src/reports.sh"
source "$BASHUNIT_ROOT_DIR/src/rerun.sh"
source "$BASHUNIT_ROOT_DIR/src/runner.sh"
source "$BASHUNIT_ROOT_DIR/src/runner/index.sh"
source "$BASHUNIT_ROOT_DIR/src/benchmark.sh"
source "$BASHUNIT_ROOT_DIR/src/bashunit.sh"
source "$BASHUNIT_ROOT_DIR/src/init.sh"
Expand Down
2 changes: 1 addition & 1 deletion src/coverage.sh → src/coverage/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/coverage/ module: only `source` lines and comments
# Entry point for the src/coverage/ module: only `source` lines and comments
# belong here. build.sh emits a file's body before recursing into its `source`
# lines, so any statement here would run before its dependencies in the built
# binary (adrs/adr-010-src-module-directories.md).
Expand Down
2 changes: 1 addition & 1 deletion src/runner.sh → src/runner/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/runner/ module: only `source` lines and comments belong
# Entry point for the src/runner/ module: only `source` lines and comments belong
# here. build.sh emits a file's body before recursing into its `source` lines, so
# any statement here would run before its dependencies in the built binary.
#
Expand Down
34 changes: 28 additions & 6 deletions tests/unit/build_test.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -91,22 +91,44 @@ function test_build_embed_docs_fails_on_missing_markers() {

# build::process_file emits a file's body and *then* recurses into its `source`
# lines, so an aggregator holding anything else at top level would run that code
# before its dependencies in the built binary but after them in dev mode. Add any
# new aggregator here.
function test_module_aggregators_hold_only_source_lines_and_comments() {
local aggregators="src/assertions.sh src/runner.sh"
# before its dependencies in the built binary but after them in dev mode.
#
# Discovered by glob, never by a hand-maintained list: the previous list named
# src/assertions.sh and src/runner.sh, and src/coverage.sh was added in #928
# without being appended, so the rule silently stopped covering it. A module's
# aggregator is src/<module>/index.sh (ADR-010); src/assertions.sh is the one
# flat-file aggregator, which has no directory of its own.
function build_aggregators() {
echo "src/assertions.sh"
local index
for index in "$ROOT_DIR"/src/*/index.sh; do
[ -f "$index" ] || continue
echo "${index#"$ROOT_DIR"/}"
done
}

function test_module_aggregators_hold_only_source_lines_and_comments() {
local offenders=""
local aggregator
for aggregator in $aggregators; do
while IFS= read -r aggregator; do
if grep -qvE '^[[:space:]]*(#|source |$)' "$ROOT_DIR/$aggregator"; then
offenders="$offenders $aggregator"
fi
done
done < <(build_aggregators)

assert_empty "$offenders"
}

# The glob above is only a safety net if it actually finds the modules.
function test_module_aggregator_discovery_finds_every_module() {
local found
found=$(build_aggregators | tr '\n' ' ')

assert_contains "src/runner/index.sh" "$found"
assert_contains "src/coverage/index.sh" "$found"
assert_contains "src/assertions.sh" "$found"
}

function test_build_process_file_embeds_a_file_only_once() {
local dir
dir=$(bashunit::temp_dir)
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' refactor(src): move module aggregators inside their directory as index.sh by Chemaclass · Pull Request #934 · TypedDevs/bashunit · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude/rules/architecture-map.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,7 +48,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| Module | Owns |
|--------|------|
| `bashunit` + `main.sh` | entry, subcommand routing, flag parsing, run lifecycle, exit codes, cleanup calls |
| `runner.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/index.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/context.sh` | workdir restore, test identity/location exports, title interpolation, capability probes |
| `runner/payload.sh` | the `_BASHUNIT_RUNNER_*_OUT` return slots; encode/decode of the per-test result payload |
| `runner/diagnostics.sh` | runtime-error detection, kill-signal classification, profiling, verbose/file headers |
Expand All@@ -68,7 +68,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| `clock.sh` | time impl selection (EPOCHREALTIME > date > perl > …), return-slot reads |
| `str.sh` / `math.sh` / `io.sh` / `globals.sh` | pure-bash utilities; `globals.sh` has `temp_file`/`temp_dir` (public test API) |
| `test_doubles.sh` | spy/mock state via `_BASHUNIT_SPY_*` globals + files |
| `coverage.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/index.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) |
| `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches |
| `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` |
Expand Down
11 changes: 6 additions & 5 deletions .claude/rules/bash-style.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -120,7 +120,7 @@ local thing=$_BASHUNIT_PKG_THING_OUT

Examples in tree: `src/runner/payload.sh` (`_BASHUNIT_RUNNER_FIELD_OUT`,
`_BASHUNIT_RUNNER_TOTAL_OUT`, `_BASHUNIT_RUNNER_TYPE_OUT`, `_BASHUNIT_RUNNER_OUTPUT_OUT`),
`src/coverage.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).
`src/coverage/branches.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).

### When the helper builds dynamic variable names (mock/spy state)

Expand DownExpand Up@@ -158,10 +158,11 @@ function bashunit::pkg::do_thing() {

### Intentional dynamic-scope mutation is a separate pattern

The coverage branch helpers in `src/coverage.sh` (`_branch_push_if` and friends) deliberately
mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). That is
documented inline at `src/coverage.sh:818-821` and is **not** the outvar pattern — the
helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
The coverage branch helpers in `src/coverage/branches.sh` (`_branch_push_if` and friends)
deliberately mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`).
That is documented inline above the helpers in that file and is **not** the outvar pattern —
the helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
It is also why those helpers and `extract_branches` must stay in one file (ADR-010).
Don't introduce new instances of this pattern without an inline justification comment.

## ShellCheck
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,7 +3,7 @@
## Unreleased

### Changed
- Internal: `src/runner.sh` is split into a `src/runner/` module of ten single-responsibility filesbehind a `source`-only aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924)
- Internal: `src/runner.sh` and `src/coverage.sh` are split into `src/runner/` and `src/coverage/` modules of single-responsibility files, each behind a `source`-only `index.sh` aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924, #925)

### Fixed
- `build.sh` dedupes embedded files by repo-relative path. The previous basename key compared the top-level loop's relative paths against the recursion's absolute ones, so a file reached from two places could be bundled twice in the released binary; it also collided for same-named files in different directories (#923)
Expand Down
120 changes: 85 additions & 35 deletions adrs/adr-010-src-module-directories.md
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
# Splitting large `src/` files into module directories

* Status: accepted
* Status: accepted, amended 2026-08-01
* Deciders: Chemaclass
* Date: 2026-07-30

Technical Story: https://github.com/TypedDevs/bashunit/issues/924

> **Amendment, 2026-08-01.** The aggregator moves *inside* the module directory
> as `index.sh`, replacing the original `src/<module>.sh` beside it. The
> reasoning that first chose "beside" was wrong, and one of the two arguments
> that overturn it is a correctness bug rather than a preference — see
> [Aggregator placement](#aggregator-placement-amended-2026-08-01).

## Context and Problem Statement

`src/runner.sh` had grown to 2145 lines and 57 functions covering five unrelated
Expand DownExpand Up@@ -36,24 +42,60 @@ directory without changing what the released single-file binary does?

## Decision Outcome

Chosen option: **`src/runner/` directory behind a thin aggregator beside it**.

`src/runner.sh` keeps its single `source` line in the `bashunit` entrypoint and
becomes ten `source` lines plus comments. `build.sh` needs no per-module
knowledge: it already recurses into `source` lines, so the module children are
discovered through the aggregator.

The aggregator sits **beside** the directory rather than inside it as an
`index.sh`. Both are identical to `build.sh` — see the next section, the build
follows the `source` graph and cannot tell the difference — so the choice is
about the source tree, not the artifact. Beside wins on precedent: it is exactly
how `src/assertions.sh` has aggregated the flat `src/assert_*.sh` files since
long before module directories existed, so there is one aggregator convention in
`src/` rather than two. Inside would make each module a self-contained directory
and keep `ls src/` free of aggregator/directory pairs, which is the real argument
for it; it was rejected as not worth renaming shipped modules and rewriting
entrypoint lines for a cosmetic gain. Revisit only with a deliberate amendment
here — not per module, or `src/` ends up with both conventions.
Chosen option: **`src/runner/` directory with the aggregator inside it as
`index.sh`** (originally "beside it"; amended 2026-08-01, below).

The entrypoint keeps a single `source` line per module —
`src/runner/index.sh` — and that file is ten `source` lines plus comments.
`build.sh` needs no per-module knowledge: it already recurses into `source`
lines, so the module children are discovered through the aggregator.

### Aggregator placement (amended 2026-08-01)

The aggregator lives **inside** the module directory as `index.sh`:

```
src/runner/index.sh <- the module's entry point
src/runner/exec.sh
src/coverage/index.sh
src/coverage/engine.sh
```

Both placements are identical to `build.sh` — it follows the `source` graph and
cannot tell the difference (next section) — so this is a source-tree decision,
not an artifact one.

The original decision was "beside" (`src/runner.sh` + `src/runner/*.sh`), on the
grounds that it matched the `src/assertions.sh` → `src/assert_*.sh` precedent and
kept one aggregator convention in `src/`. **That reasoning was wrong.**
`src/assertions.sh` aggregates *flat* files; there is no `src/assert/` directory.
It is not a module aggregator at all, it is unaffected by this choice, and it
would remain exactly as it is under either placement. There was never a
"two conventions" risk to avoid.

Two arguments settle it for inside:

* **A hand-maintained aggregator list drifts; a glob cannot.** The rule that
aggregators hold only `source` lines is enforced by
`test_module_aggregators_hold_only_source_lines_and_comments`, which named its
aggregators in a string: `"src/assertions.sh src/runner.sh"`. `src/coverage.sh`
was added in #928 and never appended, so the rule silently stopped covering it
within one module of being introduced. With the aggregator at a predictable
`src/*/index.sh`, the test discovers modules by glob and cannot drift. This is
the deciding argument: it is a correctness property, not a preference.
* **The cost only grows.** Converting two shipped modules costs two renames and
two entrypoint lines. #931 adds nine more; converting after it costs eleven.

Alongside those, the ordinary case for treating a directory as the module: `mv`
or `rm -r` moves it as one unit, an orphaned `src/runner.sh` left behind by a
deleted `src/runner/` becomes impossible, and `ls src/` lists modules rather than
aggregator/directory pairs. The convention also matches what most ecosystems do —
`index.ts`, `__init__.py`, `mod.rs`.

`index.sh` over `main.sh` or `init.sh`: both of those collide with existing
concepts here (`src/main.sh`, and `src/init.sh` implementing `bashunit init`).

Apply this to every module. Mixing placements is the outcome worth avoiding.

This required fixing `build.sh` first (#923). Its embed dedupe was keyed on a
file's *basename*, which both hid a genuine double-embed (the top-level loop
Expand DownExpand Up@@ -161,27 +203,35 @@ context · payload · diagnostics → parallel · hooks · result → provider
grouping in a filename prefix rather than in the directory structure.
* Bad, because it does not generalise — `src/coverage.sh` would add nine more.

### `src/runner/` directory behind an aggregator beside it
### `src/runner/` directory behind an aggregator beside it (rejected on amendment)

* Good, because it mirrors the existing `src/assertions.sh` → `src/assert_*.sh`
aggregator precedent, and `src/dev/debug.sh` already proved `src/` can nest.
* Good, because `build.sh` discovers children through recursion, so adding a
module file needs no build change.
* Good, because the entrypoint keeps one `source` line per module, unchanged in
shape from when the module was a single file.
* Good, because the entrypoint line keeps the shape it had when the module was a
single file.
* Bad, because the aggregator's path is unpredictable, so the test enforcing the
"only `source` lines" rule needs a hand-maintained list — which drifted the
first time a second module appeared (#928 added `src/coverage.sh` and never
appended it, silently un-enforcing the rule for that file).
* Bad, because `ls src/` shows an aggregator/directory pair per module, and the
pair can desync: deleting `src/runner/` leaves an orphaned `src/runner.sh`.
* Bad, because it required fixing the build's dedupe key first (#923).
* Bad, because `ls src/` shows an aggregator/directory pair per module.

### `src/runner/index.sh` inside the directory

* Good, because a module becomes one self-contained directory — moving or
deleting it is a single `mv`/`rm -r`, and `ls src/` shows modules, not pairs.
* Good, because it scales more cleanly if `src/` ever reaches a dozen modules.
* ~~Good, because it mirrors the `src/assertions.sh` → `src/assert_*.sh`
precedent~~ — withdrawn. `src/assertions.sh` aggregates flat files and has no
directory, so it is not a module aggregator and is unaffected either way.

### `src/runner/index.sh` inside the directory (chosen on amendment)

* Good, because the aggregator sits at a predictable `src/*/index.sh`, so the
enforcement test discovers modules by glob and cannot drift.
* Good, because a module becomes one self-contained directory — `mv`/`rm -r`
moves it as a unit, no orphan is possible, and `ls src/` lists modules.
* Good, because it matches the convention most ecosystems already use
(`index.ts`, `__init__.py`, `mod.rs`), so the entry point is where a reader
looks for it.
* Neutral on the build: identical to beside, which walks the `source` graph.
* Bad, because it would rename the two shipped modules and rewrite their
entrypoint lines for no behavioural gain.
* Bad, because `src/assertions.sh` would stay a beside-aggregator (it has no
directory), leaving `src/` with two aggregator conventions.
* Bad, because it cost renaming the two shipped modules and their entrypoint
lines — a cost that only grows, hence doing it before #931 adds nine more.

## Links

Expand Down
4 changes: 2 additions & 2 deletions bashunit
Original file line numberDiff line numberDiff line change
Expand Up@@ -73,7 +73,7 @@ source "$BASHUNIT_ROOT_DIR/src/io.sh"
source "$BASHUNIT_ROOT_DIR/src/math.sh"
source "$BASHUNIT_ROOT_DIR/src/parallel.sh"
source "$BASHUNIT_ROOT_DIR/src/env.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage/index.sh"
source "$BASHUNIT_ROOT_DIR/src/clock.sh"
source "$BASHUNIT_ROOT_DIR/src/state.sh"
source "$BASHUNIT_ROOT_DIR/src/colors.sh"
Expand All@@ -87,7 +87,7 @@ source "$BASHUNIT_ROOT_DIR/src/assertions.sh"
source "$BASHUNIT_ROOT_DIR/src/doc.sh"
source "$BASHUNIT_ROOT_DIR/src/reports.sh"
source "$BASHUNIT_ROOT_DIR/src/rerun.sh"
source "$BASHUNIT_ROOT_DIR/src/runner.sh"
source "$BASHUNIT_ROOT_DIR/src/runner/index.sh"
source "$BASHUNIT_ROOT_DIR/src/benchmark.sh"
source "$BASHUNIT_ROOT_DIR/src/bashunit.sh"
source "$BASHUNIT_ROOT_DIR/src/init.sh"
Expand Down
2 changes: 1 addition & 1 deletion src/coverage.sh → src/coverage/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/coverage/ module: only `source` lines and comments
# Entry point for the src/coverage/ module: only `source` lines and comments
# belong here. build.sh emits a file's body before recursing into its `source`
# lines, so any statement here would run before its dependencies in the built
# binary (adrs/adr-010-src-module-directories.md).
Expand Down
2 changes: 1 addition & 1 deletion src/runner.sh → src/runner/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/runner/ module: only `source` lines and comments belong
# Entry point for the src/runner/ module: only `source` lines and comments belong
# here. build.sh emits a file's body before recursing into its `source` lines, so
# any statement here would run before its dependencies in the built binary.
#
Expand Down
34 changes: 28 additions & 6 deletions tests/unit/build_test.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -91,22 +91,44 @@ function test_build_embed_docs_fails_on_missing_markers() {

# build::process_file emits a file's body and *then* recurses into its `source`
# lines, so an aggregator holding anything else at top level would run that code
# before its dependencies in the built binary but after them in dev mode. Add any
# new aggregator here.
function test_module_aggregators_hold_only_source_lines_and_comments() {
local aggregators="src/assertions.sh src/runner.sh"
# before its dependencies in the built binary but after them in dev mode.
#
# Discovered by glob, never by a hand-maintained list: the previous list named
# src/assertions.sh and src/runner.sh, and src/coverage.sh was added in #928
# without being appended, so the rule silently stopped covering it. A module's
# aggregator is src/<module>/index.sh (ADR-010); src/assertions.sh is the one
# flat-file aggregator, which has no directory of its own.
function build_aggregators() {
echo "src/assertions.sh"
local index
for index in "$ROOT_DIR"/src/*/index.sh; do
[ -f "$index" ] || continue
echo "${index#"$ROOT_DIR"/}"
done
}

function test_module_aggregators_hold_only_source_lines_and_comments() {
local offenders=""
local aggregator
for aggregator in $aggregators; do
while IFS= read -r aggregator; do
if grep -qvE '^[[:space:]]*(#|source |$)' "$ROOT_DIR/$aggregator"; then
offenders="$offenders $aggregator"
fi
done
done < <(build_aggregators)

assert_empty "$offenders"
}

# The glob above is only a safety net if it actually finds the modules.
function test_module_aggregator_discovery_finds_every_module() {
local found
found=$(build_aggregators | tr '\n' ' ')

assert_contains "src/runner/index.sh" "$found"
assert_contains "src/coverage/index.sh" "$found"
assert_contains "src/assertions.sh" "$found"
}

function test_build_process_file_embeds_a_file_only_once() {
local dir
dir=$(bashunit::temp_dir)
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' refactor(src): move module aggregators inside their directory as index.sh by Chemaclass · Pull Request #934 · TypedDevs/bashunit · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude/rules/architecture-map.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,7 +48,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| Module | Owns |
|--------|------|
| `bashunit` + `main.sh` | entry, subcommand routing, flag parsing, run lifecycle, exit codes, cleanup calls |
| `runner.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/index.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/context.sh` | workdir restore, test identity/location exports, title interpolation, capability probes |
| `runner/payload.sh` | the `_BASHUNIT_RUNNER_*_OUT` return slots; encode/decode of the per-test result payload |
| `runner/diagnostics.sh` | runtime-error detection, kill-signal classification, profiling, verbose/file headers |
Expand All@@ -68,7 +68,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| `clock.sh` | time impl selection (EPOCHREALTIME > date > perl > …), return-slot reads |
| `str.sh` / `math.sh` / `io.sh` / `globals.sh` | pure-bash utilities; `globals.sh` has `temp_file`/`temp_dir` (public test API) |
| `test_doubles.sh` | spy/mock state via `_BASHUNIT_SPY_*` globals + files |
| `coverage.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/index.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) |
| `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches |
| `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` |
Expand Down
11 changes: 6 additions & 5 deletions .claude/rules/bash-style.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -120,7 +120,7 @@ local thing=$_BASHUNIT_PKG_THING_OUT

Examples in tree: `src/runner/payload.sh` (`_BASHUNIT_RUNNER_FIELD_OUT`,
`_BASHUNIT_RUNNER_TOTAL_OUT`, `_BASHUNIT_RUNNER_TYPE_OUT`, `_BASHUNIT_RUNNER_OUTPUT_OUT`),
`src/coverage.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).
`src/coverage/branches.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).

### When the helper builds dynamic variable names (mock/spy state)

Expand DownExpand Up@@ -158,10 +158,11 @@ function bashunit::pkg::do_thing() {

### Intentional dynamic-scope mutation is a separate pattern

The coverage branch helpers in `src/coverage.sh` (`_branch_push_if` and friends) deliberately
mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). That is
documented inline at `src/coverage.sh:818-821` and is **not** the outvar pattern — the
helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
The coverage branch helpers in `src/coverage/branches.sh` (`_branch_push_if` and friends)
deliberately mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`).
That is documented inline above the helpers in that file and is **not** the outvar pattern —
the helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
It is also why those helpers and `extract_branches` must stay in one file (ADR-010).
Don't introduce new instances of this pattern without an inline justification comment.

## ShellCheck
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,7 +3,7 @@
## Unreleased

### Changed
- Internal: `src/runner.sh` is split into a `src/runner/` module of ten single-responsibility filesbehind a `source`-only aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924)
- Internal: `src/runner.sh` and `src/coverage.sh` are split into `src/runner/` and `src/coverage/` modules of single-responsibility files, each behind a `source`-only `index.sh` aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924, #925)

### Fixed
- `build.sh` dedupes embedded files by repo-relative path. The previous basename key compared the top-level loop's relative paths against the recursion's absolute ones, so a file reached from two places could be bundled twice in the released binary; it also collided for same-named files in different directories (#923)
Expand Down
120 changes: 85 additions & 35 deletions adrs/adr-010-src-module-directories.md
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
# Splitting large `src/` files into module directories

* Status: accepted
* Status: accepted, amended 2026-08-01
* Deciders: Chemaclass
* Date: 2026-07-30

Technical Story: https://github.com/TypedDevs/bashunit/issues/924

> **Amendment, 2026-08-01.** The aggregator moves *inside* the module directory
> as `index.sh`, replacing the original `src/<module>.sh` beside it. The
> reasoning that first chose "beside" was wrong, and one of the two arguments
> that overturn it is a correctness bug rather than a preference — see
> [Aggregator placement](#aggregator-placement-amended-2026-08-01).

## Context and Problem Statement

`src/runner.sh` had grown to 2145 lines and 57 functions covering five unrelated
Expand DownExpand Up@@ -36,24 +42,60 @@ directory without changing what the released single-file binary does?

## Decision Outcome

Chosen option: **`src/runner/` directory behind a thin aggregator beside it**.

`src/runner.sh` keeps its single `source` line in the `bashunit` entrypoint and
becomes ten `source` lines plus comments. `build.sh` needs no per-module
knowledge: it already recurses into `source` lines, so the module children are
discovered through the aggregator.

The aggregator sits **beside** the directory rather than inside it as an
`index.sh`. Both are identical to `build.sh` — see the next section, the build
follows the `source` graph and cannot tell the difference — so the choice is
about the source tree, not the artifact. Beside wins on precedent: it is exactly
how `src/assertions.sh` has aggregated the flat `src/assert_*.sh` files since
long before module directories existed, so there is one aggregator convention in
`src/` rather than two. Inside would make each module a self-contained directory
and keep `ls src/` free of aggregator/directory pairs, which is the real argument
for it; it was rejected as not worth renaming shipped modules and rewriting
entrypoint lines for a cosmetic gain. Revisit only with a deliberate amendment
here — not per module, or `src/` ends up with both conventions.
Chosen option: **`src/runner/` directory with the aggregator inside it as
`index.sh`** (originally "beside it"; amended 2026-08-01, below).

The entrypoint keeps a single `source` line per module —
`src/runner/index.sh` — and that file is ten `source` lines plus comments.
`build.sh` needs no per-module knowledge: it already recurses into `source`
lines, so the module children are discovered through the aggregator.

### Aggregator placement (amended 2026-08-01)

The aggregator lives **inside** the module directory as `index.sh`:

```
src/runner/index.sh <- the module's entry point
src/runner/exec.sh
src/coverage/index.sh
src/coverage/engine.sh
```

Both placements are identical to `build.sh` — it follows the `source` graph and
cannot tell the difference (next section) — so this is a source-tree decision,
not an artifact one.

The original decision was "beside" (`src/runner.sh` + `src/runner/*.sh`), on the
grounds that it matched the `src/assertions.sh` → `src/assert_*.sh` precedent and
kept one aggregator convention in `src/`. **That reasoning was wrong.**
`src/assertions.sh` aggregates *flat* files; there is no `src/assert/` directory.
It is not a module aggregator at all, it is unaffected by this choice, and it
would remain exactly as it is under either placement. There was never a
"two conventions" risk to avoid.

Two arguments settle it for inside:

* **A hand-maintained aggregator list drifts; a glob cannot.** The rule that
aggregators hold only `source` lines is enforced by
`test_module_aggregators_hold_only_source_lines_and_comments`, which named its
aggregators in a string: `"src/assertions.sh src/runner.sh"`. `src/coverage.sh`
was added in #928 and never appended, so the rule silently stopped covering it
within one module of being introduced. With the aggregator at a predictable
`src/*/index.sh`, the test discovers modules by glob and cannot drift. This is
the deciding argument: it is a correctness property, not a preference.
* **The cost only grows.** Converting two shipped modules costs two renames and
two entrypoint lines. #931 adds nine more; converting after it costs eleven.

Alongside those, the ordinary case for treating a directory as the module: `mv`
or `rm -r` moves it as one unit, an orphaned `src/runner.sh` left behind by a
deleted `src/runner/` becomes impossible, and `ls src/` lists modules rather than
aggregator/directory pairs. The convention also matches what most ecosystems do —
`index.ts`, `__init__.py`, `mod.rs`.

`index.sh` over `main.sh` or `init.sh`: both of those collide with existing
concepts here (`src/main.sh`, and `src/init.sh` implementing `bashunit init`).

Apply this to every module. Mixing placements is the outcome worth avoiding.

This required fixing `build.sh` first (#923). Its embed dedupe was keyed on a
file's *basename*, which both hid a genuine double-embed (the top-level loop
Expand DownExpand Up@@ -161,27 +203,35 @@ context · payload · diagnostics → parallel · hooks · result → provider
grouping in a filename prefix rather than in the directory structure.
* Bad, because it does not generalise — `src/coverage.sh` would add nine more.

### `src/runner/` directory behind an aggregator beside it
### `src/runner/` directory behind an aggregator beside it (rejected on amendment)

* Good, because it mirrors the existing `src/assertions.sh` → `src/assert_*.sh`
aggregator precedent, and `src/dev/debug.sh` already proved `src/` can nest.
* Good, because `build.sh` discovers children through recursion, so adding a
module file needs no build change.
* Good, because the entrypoint keeps one `source` line per module, unchanged in
shape from when the module was a single file.
* Good, because the entrypoint line keeps the shape it had when the module was a
single file.
* Bad, because the aggregator's path is unpredictable, so the test enforcing the
"only `source` lines" rule needs a hand-maintained list — which drifted the
first time a second module appeared (#928 added `src/coverage.sh` and never
appended it, silently un-enforcing the rule for that file).
* Bad, because `ls src/` shows an aggregator/directory pair per module, and the
pair can desync: deleting `src/runner/` leaves an orphaned `src/runner.sh`.
* Bad, because it required fixing the build's dedupe key first (#923).
* Bad, because `ls src/` shows an aggregator/directory pair per module.

### `src/runner/index.sh` inside the directory

* Good, because a module becomes one self-contained directory — moving or
deleting it is a single `mv`/`rm -r`, and `ls src/` shows modules, not pairs.
* Good, because it scales more cleanly if `src/` ever reaches a dozen modules.
* ~~Good, because it mirrors the `src/assertions.sh` → `src/assert_*.sh`
precedent~~ — withdrawn. `src/assertions.sh` aggregates flat files and has no
directory, so it is not a module aggregator and is unaffected either way.

### `src/runner/index.sh` inside the directory (chosen on amendment)

* Good, because the aggregator sits at a predictable `src/*/index.sh`, so the
enforcement test discovers modules by glob and cannot drift.
* Good, because a module becomes one self-contained directory — `mv`/`rm -r`
moves it as a unit, no orphan is possible, and `ls src/` lists modules.
* Good, because it matches the convention most ecosystems already use
(`index.ts`, `__init__.py`, `mod.rs`), so the entry point is where a reader
looks for it.
* Neutral on the build: identical to beside, which walks the `source` graph.
* Bad, because it would rename the two shipped modules and rewrite their
entrypoint lines for no behavioural gain.
* Bad, because `src/assertions.sh` would stay a beside-aggregator (it has no
directory), leaving `src/` with two aggregator conventions.
* Bad, because it cost renaming the two shipped modules and their entrypoint
lines — a cost that only grows, hence doing it before #931 adds nine more.

## Links

Expand Down
4 changes: 2 additions & 2 deletions bashunit
Original file line numberDiff line numberDiff line change
Expand Up@@ -73,7 +73,7 @@ source "$BASHUNIT_ROOT_DIR/src/io.sh"
source "$BASHUNIT_ROOT_DIR/src/math.sh"
source "$BASHUNIT_ROOT_DIR/src/parallel.sh"
source "$BASHUNIT_ROOT_DIR/src/env.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage/index.sh"
source "$BASHUNIT_ROOT_DIR/src/clock.sh"
source "$BASHUNIT_ROOT_DIR/src/state.sh"
source "$BASHUNIT_ROOT_DIR/src/colors.sh"
Expand All@@ -87,7 +87,7 @@ source "$BASHUNIT_ROOT_DIR/src/assertions.sh"
source "$BASHUNIT_ROOT_DIR/src/doc.sh"
source "$BASHUNIT_ROOT_DIR/src/reports.sh"
source "$BASHUNIT_ROOT_DIR/src/rerun.sh"
source "$BASHUNIT_ROOT_DIR/src/runner.sh"
source "$BASHUNIT_ROOT_DIR/src/runner/index.sh"
source "$BASHUNIT_ROOT_DIR/src/benchmark.sh"
source "$BASHUNIT_ROOT_DIR/src/bashunit.sh"
source "$BASHUNIT_ROOT_DIR/src/init.sh"
Expand Down
2 changes: 1 addition & 1 deletion src/coverage.sh → src/coverage/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/coverage/ module: only `source` lines and comments
# Entry point for the src/coverage/ module: only `source` lines and comments
# belong here. build.sh emits a file's body before recursing into its `source`
# lines, so any statement here would run before its dependencies in the built
# binary (adrs/adr-010-src-module-directories.md).
Expand Down
2 changes: 1 addition & 1 deletion src/runner.sh → src/runner/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/runner/ module: only `source` lines and comments belong
# Entry point for the src/runner/ module: only `source` lines and comments belong
# here. build.sh emits a file's body before recursing into its `source` lines, so
# any statement here would run before its dependencies in the built binary.
#
Expand Down
34 changes: 28 additions & 6 deletions tests/unit/build_test.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -91,22 +91,44 @@ function test_build_embed_docs_fails_on_missing_markers() {

# build::process_file emits a file's body and *then* recurses into its `source`
# lines, so an aggregator holding anything else at top level would run that code
# before its dependencies in the built binary but after them in dev mode. Add any
# new aggregator here.
function test_module_aggregators_hold_only_source_lines_and_comments() {
local aggregators="src/assertions.sh src/runner.sh"
# before its dependencies in the built binary but after them in dev mode.
#
# Discovered by glob, never by a hand-maintained list: the previous list named
# src/assertions.sh and src/runner.sh, and src/coverage.sh was added in #928
# without being appended, so the rule silently stopped covering it. A module's
# aggregator is src/<module>/index.sh (ADR-010); src/assertions.sh is the one
# flat-file aggregator, which has no directory of its own.
function build_aggregators() {
echo "src/assertions.sh"
local index
for index in "$ROOT_DIR"/src/*/index.sh; do
[ -f "$index" ] || continue
echo "${index#"$ROOT_DIR"/}"
done
}

function test_module_aggregators_hold_only_source_lines_and_comments() {
local offenders=""
local aggregator
for aggregator in $aggregators; do
while IFS= read -r aggregator; do
if grep -qvE '^[[:space:]]*(#|source |$)' "$ROOT_DIR/$aggregator"; then
offenders="$offenders $aggregator"
fi
done
done < <(build_aggregators)

assert_empty "$offenders"
}

# The glob above is only a safety net if it actually finds the modules.
function test_module_aggregator_discovery_finds_every_module() {
local found
found=$(build_aggregators | tr '\n' ' ')

assert_contains "src/runner/index.sh" "$found"
assert_contains "src/coverage/index.sh" "$found"
assert_contains "src/assertions.sh" "$found"
}

function test_build_process_file_embeds_a_file_only_once() {
local dir
dir=$(bashunit::temp_dir)
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' refactor(src): move module aggregators inside their directory as index.sh by Chemaclass · Pull Request #934 · TypedDevs/bashunit · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude/rules/architecture-map.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,7 +48,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| Module | Owns |
|--------|------|
| `bashunit` + `main.sh` | entry, subcommand routing, flag parsing, run lifecycle, exit codes, cleanup calls |
| `runner.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/index.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/context.sh` | workdir restore, test identity/location exports, title interpolation, capability probes |
| `runner/payload.sh` | the `_BASHUNIT_RUNNER_*_OUT` return slots; encode/decode of the per-test result payload |
| `runner/diagnostics.sh` | runtime-error detection, kill-signal classification, profiling, verbose/file headers |
Expand All@@ -68,7 +68,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| `clock.sh` | time impl selection (EPOCHREALTIME > date > perl > …), return-slot reads |
| `str.sh` / `math.sh` / `io.sh` / `globals.sh` | pure-bash utilities; `globals.sh` has `temp_file`/`temp_dir` (public test API) |
| `test_doubles.sh` | spy/mock state via `_BASHUNIT_SPY_*` globals + files |
| `coverage.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/index.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) |
| `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches |
| `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` |
Expand Down
11 changes: 6 additions & 5 deletions .claude/rules/bash-style.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -120,7 +120,7 @@ local thing=$_BASHUNIT_PKG_THING_OUT

Examples in tree: `src/runner/payload.sh` (`_BASHUNIT_RUNNER_FIELD_OUT`,
`_BASHUNIT_RUNNER_TOTAL_OUT`, `_BASHUNIT_RUNNER_TYPE_OUT`, `_BASHUNIT_RUNNER_OUTPUT_OUT`),
`src/coverage.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).
`src/coverage/branches.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).

### When the helper builds dynamic variable names (mock/spy state)

Expand DownExpand Up@@ -158,10 +158,11 @@ function bashunit::pkg::do_thing() {

### Intentional dynamic-scope mutation is a separate pattern

The coverage branch helpers in `src/coverage.sh` (`_branch_push_if` and friends) deliberately
mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). That is
documented inline at `src/coverage.sh:818-821` and is **not** the outvar pattern — the
helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
The coverage branch helpers in `src/coverage/branches.sh` (`_branch_push_if` and friends)
deliberately mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`).
That is documented inline above the helpers in that file and is **not** the outvar pattern —
the helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
It is also why those helpers and `extract_branches` must stay in one file (ADR-010).
Don't introduce new instances of this pattern without an inline justification comment.

## ShellCheck
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,7 +3,7 @@
## Unreleased

### Changed
- Internal: `src/runner.sh` is split into a `src/runner/` module of ten single-responsibility filesbehind a `source`-only aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924)
- Internal: `src/runner.sh` and `src/coverage.sh` are split into `src/runner/` and `src/coverage/` modules of single-responsibility files, each behind a `source`-only `index.sh` aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924, #925)

### Fixed
- `build.sh` dedupes embedded files by repo-relative path. The previous basename key compared the top-level loop's relative paths against the recursion's absolute ones, so a file reached from two places could be bundled twice in the released binary; it also collided for same-named files in different directories (#923)
Expand Down
120 changes: 85 additions & 35 deletions adrs/adr-010-src-module-directories.md
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
# Splitting large `src/` files into module directories

* Status: accepted
* Status: accepted, amended 2026-08-01
* Deciders: Chemaclass
* Date: 2026-07-30

Technical Story: https://github.com/TypedDevs/bashunit/issues/924

> **Amendment, 2026-08-01.** The aggregator moves *inside* the module directory
> as `index.sh`, replacing the original `src/<module>.sh` beside it. The
> reasoning that first chose "beside" was wrong, and one of the two arguments
> that overturn it is a correctness bug rather than a preference — see
> [Aggregator placement](#aggregator-placement-amended-2026-08-01).

## Context and Problem Statement

`src/runner.sh` had grown to 2145 lines and 57 functions covering five unrelated
Expand DownExpand Up@@ -36,24 +42,60 @@ directory without changing what the released single-file binary does?

## Decision Outcome

Chosen option: **`src/runner/` directory behind a thin aggregator beside it**.

`src/runner.sh` keeps its single `source` line in the `bashunit` entrypoint and
becomes ten `source` lines plus comments. `build.sh` needs no per-module
knowledge: it already recurses into `source` lines, so the module children are
discovered through the aggregator.

The aggregator sits **beside** the directory rather than inside it as an
`index.sh`. Both are identical to `build.sh` — see the next section, the build
follows the `source` graph and cannot tell the difference — so the choice is
about the source tree, not the artifact. Beside wins on precedent: it is exactly
how `src/assertions.sh` has aggregated the flat `src/assert_*.sh` files since
long before module directories existed, so there is one aggregator convention in
`src/` rather than two. Inside would make each module a self-contained directory
and keep `ls src/` free of aggregator/directory pairs, which is the real argument
for it; it was rejected as not worth renaming shipped modules and rewriting
entrypoint lines for a cosmetic gain. Revisit only with a deliberate amendment
here — not per module, or `src/` ends up with both conventions.
Chosen option: **`src/runner/` directory with the aggregator inside it as
`index.sh`** (originally "beside it"; amended 2026-08-01, below).

The entrypoint keeps a single `source` line per module —
`src/runner/index.sh` — and that file is ten `source` lines plus comments.
`build.sh` needs no per-module knowledge: it already recurses into `source`
lines, so the module children are discovered through the aggregator.

### Aggregator placement (amended 2026-08-01)

The aggregator lives **inside** the module directory as `index.sh`:

```
src/runner/index.sh <- the module's entry point
src/runner/exec.sh
src/coverage/index.sh
src/coverage/engine.sh
```

Both placements are identical to `build.sh` — it follows the `source` graph and
cannot tell the difference (next section) — so this is a source-tree decision,
not an artifact one.

The original decision was "beside" (`src/runner.sh` + `src/runner/*.sh`), on the
grounds that it matched the `src/assertions.sh` → `src/assert_*.sh` precedent and
kept one aggregator convention in `src/`. **That reasoning was wrong.**
`src/assertions.sh` aggregates *flat* files; there is no `src/assert/` directory.
It is not a module aggregator at all, it is unaffected by this choice, and it
would remain exactly as it is under either placement. There was never a
"two conventions" risk to avoid.

Two arguments settle it for inside:

* **A hand-maintained aggregator list drifts; a glob cannot.** The rule that
aggregators hold only `source` lines is enforced by
`test_module_aggregators_hold_only_source_lines_and_comments`, which named its
aggregators in a string: `"src/assertions.sh src/runner.sh"`. `src/coverage.sh`
was added in #928 and never appended, so the rule silently stopped covering it
within one module of being introduced. With the aggregator at a predictable
`src/*/index.sh`, the test discovers modules by glob and cannot drift. This is
the deciding argument: it is a correctness property, not a preference.
* **The cost only grows.** Converting two shipped modules costs two renames and
two entrypoint lines. #931 adds nine more; converting after it costs eleven.

Alongside those, the ordinary case for treating a directory as the module: `mv`
or `rm -r` moves it as one unit, an orphaned `src/runner.sh` left behind by a
deleted `src/runner/` becomes impossible, and `ls src/` lists modules rather than
aggregator/directory pairs. The convention also matches what most ecosystems do —
`index.ts`, `__init__.py`, `mod.rs`.

`index.sh` over `main.sh` or `init.sh`: both of those collide with existing
concepts here (`src/main.sh`, and `src/init.sh` implementing `bashunit init`).

Apply this to every module. Mixing placements is the outcome worth avoiding.

This required fixing `build.sh` first (#923). Its embed dedupe was keyed on a
file's *basename*, which both hid a genuine double-embed (the top-level loop
Expand DownExpand Up@@ -161,27 +203,35 @@ context · payload · diagnostics → parallel · hooks · result → provider
grouping in a filename prefix rather than in the directory structure.
* Bad, because it does not generalise — `src/coverage.sh` would add nine more.

### `src/runner/` directory behind an aggregator beside it
### `src/runner/` directory behind an aggregator beside it (rejected on amendment)

* Good, because it mirrors the existing `src/assertions.sh` → `src/assert_*.sh`
aggregator precedent, and `src/dev/debug.sh` already proved `src/` can nest.
* Good, because `build.sh` discovers children through recursion, so adding a
module file needs no build change.
* Good, because the entrypoint keeps one `source` line per module, unchanged in
shape from when the module was a single file.
* Good, because the entrypoint line keeps the shape it had when the module was a
single file.
* Bad, because the aggregator's path is unpredictable, so the test enforcing the
"only `source` lines" rule needs a hand-maintained list — which drifted the
first time a second module appeared (#928 added `src/coverage.sh` and never
appended it, silently un-enforcing the rule for that file).
* Bad, because `ls src/` shows an aggregator/directory pair per module, and the
pair can desync: deleting `src/runner/` leaves an orphaned `src/runner.sh`.
* Bad, because it required fixing the build's dedupe key first (#923).
* Bad, because `ls src/` shows an aggregator/directory pair per module.

### `src/runner/index.sh` inside the directory

* Good, because a module becomes one self-contained directory — moving or
deleting it is a single `mv`/`rm -r`, and `ls src/` shows modules, not pairs.
* Good, because it scales more cleanly if `src/` ever reaches a dozen modules.
* ~~Good, because it mirrors the `src/assertions.sh` → `src/assert_*.sh`
precedent~~ — withdrawn. `src/assertions.sh` aggregates flat files and has no
directory, so it is not a module aggregator and is unaffected either way.

### `src/runner/index.sh` inside the directory (chosen on amendment)

* Good, because the aggregator sits at a predictable `src/*/index.sh`, so the
enforcement test discovers modules by glob and cannot drift.
* Good, because a module becomes one self-contained directory — `mv`/`rm -r`
moves it as a unit, no orphan is possible, and `ls src/` lists modules.
* Good, because it matches the convention most ecosystems already use
(`index.ts`, `__init__.py`, `mod.rs`), so the entry point is where a reader
looks for it.
* Neutral on the build: identical to beside, which walks the `source` graph.
* Bad, because it would rename the two shipped modules and rewrite their
entrypoint lines for no behavioural gain.
* Bad, because `src/assertions.sh` would stay a beside-aggregator (it has no
directory), leaving `src/` with two aggregator conventions.
* Bad, because it cost renaming the two shipped modules and their entrypoint
lines — a cost that only grows, hence doing it before #931 adds nine more.

## Links

Expand Down
4 changes: 2 additions & 2 deletions bashunit
Original file line numberDiff line numberDiff line change
Expand Up@@ -73,7 +73,7 @@ source "$BASHUNIT_ROOT_DIR/src/io.sh"
source "$BASHUNIT_ROOT_DIR/src/math.sh"
source "$BASHUNIT_ROOT_DIR/src/parallel.sh"
source "$BASHUNIT_ROOT_DIR/src/env.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage/index.sh"
source "$BASHUNIT_ROOT_DIR/src/clock.sh"
source "$BASHUNIT_ROOT_DIR/src/state.sh"
source "$BASHUNIT_ROOT_DIR/src/colors.sh"
Expand All@@ -87,7 +87,7 @@ source "$BASHUNIT_ROOT_DIR/src/assertions.sh"
source "$BASHUNIT_ROOT_DIR/src/doc.sh"
source "$BASHUNIT_ROOT_DIR/src/reports.sh"
source "$BASHUNIT_ROOT_DIR/src/rerun.sh"
source "$BASHUNIT_ROOT_DIR/src/runner.sh"
source "$BASHUNIT_ROOT_DIR/src/runner/index.sh"
source "$BASHUNIT_ROOT_DIR/src/benchmark.sh"
source "$BASHUNIT_ROOT_DIR/src/bashunit.sh"
source "$BASHUNIT_ROOT_DIR/src/init.sh"
Expand Down
2 changes: 1 addition & 1 deletion src/coverage.sh → src/coverage/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/coverage/ module: only `source` lines and comments
# Entry point for the src/coverage/ module: only `source` lines and comments
# belong here. build.sh emits a file's body before recursing into its `source`
# lines, so any statement here would run before its dependencies in the built
# binary (adrs/adr-010-src-module-directories.md).
Expand Down
2 changes: 1 addition & 1 deletion src/runner.sh → src/runner/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/runner/ module: only `source` lines and comments belong
# Entry point for the src/runner/ module: only `source` lines and comments belong
# here. build.sh emits a file's body before recursing into its `source` lines, so
# any statement here would run before its dependencies in the built binary.
#
Expand Down
34 changes: 28 additions & 6 deletions tests/unit/build_test.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -91,22 +91,44 @@ function test_build_embed_docs_fails_on_missing_markers() {

# build::process_file emits a file's body and *then* recurses into its `source`
# lines, so an aggregator holding anything else at top level would run that code
# before its dependencies in the built binary but after them in dev mode. Add any
# new aggregator here.
function test_module_aggregators_hold_only_source_lines_and_comments() {
local aggregators="src/assertions.sh src/runner.sh"
# before its dependencies in the built binary but after them in dev mode.
#
# Discovered by glob, never by a hand-maintained list: the previous list named
# src/assertions.sh and src/runner.sh, and src/coverage.sh was added in #928
# without being appended, so the rule silently stopped covering it. A module's
# aggregator is src/<module>/index.sh (ADR-010); src/assertions.sh is the one
# flat-file aggregator, which has no directory of its own.
function build_aggregators() {
echo "src/assertions.sh"
local index
for index in "$ROOT_DIR"/src/*/index.sh; do
[ -f "$index" ] || continue
echo "${index#"$ROOT_DIR"/}"
done
}

function test_module_aggregators_hold_only_source_lines_and_comments() {
local offenders=""
local aggregator
for aggregator in $aggregators; do
while IFS= read -r aggregator; do
if grep -qvE '^[[:space:]]*(#|source |$)' "$ROOT_DIR/$aggregator"; then
offenders="$offenders $aggregator"
fi
done
done < <(build_aggregators)

assert_empty "$offenders"
}

# The glob above is only a safety net if it actually finds the modules.
function test_module_aggregator_discovery_finds_every_module() {
local found
found=$(build_aggregators | tr '\n' ' ')

assert_contains "src/runner/index.sh" "$found"
assert_contains "src/coverage/index.sh" "$found"
assert_contains "src/assertions.sh" "$found"
}

function test_build_process_file_embeds_a_file_only_once() {
local dir
dir=$(bashunit::temp_dir)
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); refactor(src): move module aggregators inside their directory as index.sh by Chemaclass · Pull Request #934 · TypedDevs/bashunit · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude/rules/architecture-map.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,7 +48,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| Module | Owns |
|--------|------|
| `bashunit` + `main.sh` | entry, subcommand routing, flag parsing, run lifecycle, exit codes, cleanup calls |
| `runner.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/index.sh` | aggregator only — sources the `src/runner/` module below |
| `runner/context.sh` | workdir restore, test identity/location exports, title interpolation, capability probes |
| `runner/payload.sh` | the `_BASHUNIT_RUNNER_*_OUT` return slots; encode/decode of the per-test result payload |
| `runner/diagnostics.sh` | runtime-error detection, kill-signal classification, profiling, verbose/file headers |
Expand All@@ -68,7 +68,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end).
| `clock.sh` | time impl selection (EPOCHREALTIME > date > perl > …), return-slot reads |
| `str.sh` / `math.sh` / `io.sh` / `globals.sh` | pure-bash utilities; `globals.sh` has `temp_file`/`temp_dir` (public test API) |
| `test_doubles.sh` | spy/mock state via `_BASHUNIT_SPY_*` globals + files |
| `coverage.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/index.sh` | aggregator only — sources the `src/coverage/` modules below |
| `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) |
| `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches |
| `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` |
Expand Down
11 changes: 6 additions & 5 deletions .claude/rules/bash-style.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -120,7 +120,7 @@ local thing=$_BASHUNIT_PKG_THING_OUT

Examples in tree: `src/runner/payload.sh` (`_BASHUNIT_RUNNER_FIELD_OUT`,
`_BASHUNIT_RUNNER_TOTAL_OUT`, `_BASHUNIT_RUNNER_TYPE_OUT`, `_BASHUNIT_RUNNER_OUTPUT_OUT`),
`src/coverage.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).
`src/coverage/branches.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).

### When the helper builds dynamic variable names (mock/spy state)

Expand DownExpand Up@@ -158,10 +158,11 @@ function bashunit::pkg::do_thing() {

### Intentional dynamic-scope mutation is a separate pattern

The coverage branch helpers in `src/coverage.sh` (`_branch_push_if` and friends) deliberately
mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). That is
documented inline at `src/coverage.sh:818-821` and is **not** the outvar pattern — the
helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
The coverage branch helpers in `src/coverage/branches.sh` (`_branch_push_if` and friends)
deliberately mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`).
That is documented inline above the helpers in that file and is **not** the outvar pattern —
the helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
It is also why those helpers and `extract_branches` must stay in one file (ADR-010).
Don't introduce new instances of this pattern without an inline justification comment.

## ShellCheck
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,7 +3,7 @@
## Unreleased

### Changed
- Internal: `src/runner.sh` is split into a `src/runner/` module of ten single-responsibility filesbehind a `source`-only aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924)
- Internal: `src/runner.sh` and `src/coverage.sh` are split into `src/runner/` and `src/coverage/` modules of single-responsibility files, each behind a `source`-only `index.sh` aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924, #925)

### Fixed
- `build.sh` dedupes embedded files by repo-relative path. The previous basename key compared the top-level loop's relative paths against the recursion's absolute ones, so a file reached from two places could be bundled twice in the released binary; it also collided for same-named files in different directories (#923)
Expand Down
120 changes: 85 additions & 35 deletions adrs/adr-010-src-module-directories.md
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
# Splitting large `src/` files into module directories

* Status: accepted
* Status: accepted, amended 2026-08-01
* Deciders: Chemaclass
* Date: 2026-07-30

Technical Story: https://github.com/TypedDevs/bashunit/issues/924

> **Amendment, 2026-08-01.** The aggregator moves *inside* the module directory
> as `index.sh`, replacing the original `src/<module>.sh` beside it. The
> reasoning that first chose "beside" was wrong, and one of the two arguments
> that overturn it is a correctness bug rather than a preference — see
> [Aggregator placement](#aggregator-placement-amended-2026-08-01).

## Context and Problem Statement

`src/runner.sh` had grown to 2145 lines and 57 functions covering five unrelated
Expand DownExpand Up@@ -36,24 +42,60 @@ directory without changing what the released single-file binary does?

## Decision Outcome

Chosen option: **`src/runner/` directory behind a thin aggregator beside it**.

`src/runner.sh` keeps its single `source` line in the `bashunit` entrypoint and
becomes ten `source` lines plus comments. `build.sh` needs no per-module
knowledge: it already recurses into `source` lines, so the module children are
discovered through the aggregator.

The aggregator sits **beside** the directory rather than inside it as an
`index.sh`. Both are identical to `build.sh` — see the next section, the build
follows the `source` graph and cannot tell the difference — so the choice is
about the source tree, not the artifact. Beside wins on precedent: it is exactly
how `src/assertions.sh` has aggregated the flat `src/assert_*.sh` files since
long before module directories existed, so there is one aggregator convention in
`src/` rather than two. Inside would make each module a self-contained directory
and keep `ls src/` free of aggregator/directory pairs, which is the real argument
for it; it was rejected as not worth renaming shipped modules and rewriting
entrypoint lines for a cosmetic gain. Revisit only with a deliberate amendment
here — not per module, or `src/` ends up with both conventions.
Chosen option: **`src/runner/` directory with the aggregator inside it as
`index.sh`** (originally "beside it"; amended 2026-08-01, below).

The entrypoint keeps a single `source` line per module —
`src/runner/index.sh` — and that file is ten `source` lines plus comments.
`build.sh` needs no per-module knowledge: it already recurses into `source`
lines, so the module children are discovered through the aggregator.

### Aggregator placement (amended 2026-08-01)

The aggregator lives **inside** the module directory as `index.sh`:

```
src/runner/index.sh <- the module's entry point
src/runner/exec.sh
src/coverage/index.sh
src/coverage/engine.sh
```

Both placements are identical to `build.sh` — it follows the `source` graph and
cannot tell the difference (next section) — so this is a source-tree decision,
not an artifact one.

The original decision was "beside" (`src/runner.sh` + `src/runner/*.sh`), on the
grounds that it matched the `src/assertions.sh` → `src/assert_*.sh` precedent and
kept one aggregator convention in `src/`. **That reasoning was wrong.**
`src/assertions.sh` aggregates *flat* files; there is no `src/assert/` directory.
It is not a module aggregator at all, it is unaffected by this choice, and it
would remain exactly as it is under either placement. There was never a
"two conventions" risk to avoid.

Two arguments settle it for inside:

* **A hand-maintained aggregator list drifts; a glob cannot.** The rule that
aggregators hold only `source` lines is enforced by
`test_module_aggregators_hold_only_source_lines_and_comments`, which named its
aggregators in a string: `"src/assertions.sh src/runner.sh"`. `src/coverage.sh`
was added in #928 and never appended, so the rule silently stopped covering it
within one module of being introduced. With the aggregator at a predictable
`src/*/index.sh`, the test discovers modules by glob and cannot drift. This is
the deciding argument: it is a correctness property, not a preference.
* **The cost only grows.** Converting two shipped modules costs two renames and
two entrypoint lines. #931 adds nine more; converting after it costs eleven.

Alongside those, the ordinary case for treating a directory as the module: `mv`
or `rm -r` moves it as one unit, an orphaned `src/runner.sh` left behind by a
deleted `src/runner/` becomes impossible, and `ls src/` lists modules rather than
aggregator/directory pairs. The convention also matches what most ecosystems do —
`index.ts`, `__init__.py`, `mod.rs`.

`index.sh` over `main.sh` or `init.sh`: both of those collide with existing
concepts here (`src/main.sh`, and `src/init.sh` implementing `bashunit init`).

Apply this to every module. Mixing placements is the outcome worth avoiding.

This required fixing `build.sh` first (#923). Its embed dedupe was keyed on a
file's *basename*, which both hid a genuine double-embed (the top-level loop
Expand DownExpand Up@@ -161,27 +203,35 @@ context · payload · diagnostics → parallel · hooks · result → provider
grouping in a filename prefix rather than in the directory structure.
* Bad, because it does not generalise — `src/coverage.sh` would add nine more.

### `src/runner/` directory behind an aggregator beside it
### `src/runner/` directory behind an aggregator beside it (rejected on amendment)

* Good, because it mirrors the existing `src/assertions.sh` → `src/assert_*.sh`
aggregator precedent, and `src/dev/debug.sh` already proved `src/` can nest.
* Good, because `build.sh` discovers children through recursion, so adding a
module file needs no build change.
* Good, because the entrypoint keeps one `source` line per module, unchanged in
shape from when the module was a single file.
* Good, because the entrypoint line keeps the shape it had when the module was a
single file.
* Bad, because the aggregator's path is unpredictable, so the test enforcing the
"only `source` lines" rule needs a hand-maintained list — which drifted the
first time a second module appeared (#928 added `src/coverage.sh` and never
appended it, silently un-enforcing the rule for that file).
* Bad, because `ls src/` shows an aggregator/directory pair per module, and the
pair can desync: deleting `src/runner/` leaves an orphaned `src/runner.sh`.
* Bad, because it required fixing the build's dedupe key first (#923).
* Bad, because `ls src/` shows an aggregator/directory pair per module.

### `src/runner/index.sh` inside the directory

* Good, because a module becomes one self-contained directory — moving or
deleting it is a single `mv`/`rm -r`, and `ls src/` shows modules, not pairs.
* Good, because it scales more cleanly if `src/` ever reaches a dozen modules.
* ~~Good, because it mirrors the `src/assertions.sh` → `src/assert_*.sh`
precedent~~ — withdrawn. `src/assertions.sh` aggregates flat files and has no
directory, so it is not a module aggregator and is unaffected either way.

### `src/runner/index.sh` inside the directory (chosen on amendment)

* Good, because the aggregator sits at a predictable `src/*/index.sh`, so the
enforcement test discovers modules by glob and cannot drift.
* Good, because a module becomes one self-contained directory — `mv`/`rm -r`
moves it as a unit, no orphan is possible, and `ls src/` lists modules.
* Good, because it matches the convention most ecosystems already use
(`index.ts`, `__init__.py`, `mod.rs`), so the entry point is where a reader
looks for it.
* Neutral on the build: identical to beside, which walks the `source` graph.
* Bad, because it would rename the two shipped modules and rewrite their
entrypoint lines for no behavioural gain.
* Bad, because `src/assertions.sh` would stay a beside-aggregator (it has no
directory), leaving `src/` with two aggregator conventions.
* Bad, because it cost renaming the two shipped modules and their entrypoint
lines — a cost that only grows, hence doing it before #931 adds nine more.

## Links

Expand Down
4 changes: 2 additions & 2 deletions bashunit
Original file line numberDiff line numberDiff line change
Expand Up@@ -73,7 +73,7 @@ source "$BASHUNIT_ROOT_DIR/src/io.sh"
source "$BASHUNIT_ROOT_DIR/src/math.sh"
source "$BASHUNIT_ROOT_DIR/src/parallel.sh"
source "$BASHUNIT_ROOT_DIR/src/env.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage.sh"
source "$BASHUNIT_ROOT_DIR/src/coverage/index.sh"
source "$BASHUNIT_ROOT_DIR/src/clock.sh"
source "$BASHUNIT_ROOT_DIR/src/state.sh"
source "$BASHUNIT_ROOT_DIR/src/colors.sh"
Expand All@@ -87,7 +87,7 @@ source "$BASHUNIT_ROOT_DIR/src/assertions.sh"
source "$BASHUNIT_ROOT_DIR/src/doc.sh"
source "$BASHUNIT_ROOT_DIR/src/reports.sh"
source "$BASHUNIT_ROOT_DIR/src/rerun.sh"
source "$BASHUNIT_ROOT_DIR/src/runner.sh"
source "$BASHUNIT_ROOT_DIR/src/runner/index.sh"
source "$BASHUNIT_ROOT_DIR/src/benchmark.sh"
source "$BASHUNIT_ROOT_DIR/src/bashunit.sh"
source "$BASHUNIT_ROOT_DIR/src/init.sh"
Expand Down
2 changes: 1 addition & 1 deletion src/coverage.sh → src/coverage/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/coverage/ module: only `source` lines and comments
# Entry point for the src/coverage/ module: only `source` lines and comments
# belong here. build.sh emits a file's body before recursing into its `source`
# lines, so any statement here would run before its dependencies in the built
# binary (adrs/adr-010-src-module-directories.md).
Expand Down
2 changes: 1 addition & 1 deletion src/runner.sh → src/runner/index.sh
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash

# Aggregator for the src/runner/ module: only `source` lines and comments belong
# Entry point for the src/runner/ module: only `source` lines and comments belong
# here. build.sh emits a file's body before recursing into its `source` lines, so
# any statement here would run before its dependencies in the built binary.
#
Expand Down
34 changes: 28 additions & 6 deletions tests/unit/build_test.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -91,22 +91,44 @@ function test_build_embed_docs_fails_on_missing_markers() {

# build::process_file emits a file's body and *then* recurses into its `source`
# lines, so an aggregator holding anything else at top level would run that code
# before its dependencies in the built binary but after them in dev mode. Add any
# new aggregator here.
function test_module_aggregators_hold_only_source_lines_and_comments() {
local aggregators="src/assertions.sh src/runner.sh"
# before its dependencies in the built binary but after them in dev mode.
#
# Discovered by glob, never by a hand-maintained list: the previous list named
# src/assertions.sh and src/runner.sh, and src/coverage.sh was added in #928
# without being appended, so the rule silently stopped covering it. A module's
# aggregator is src/<module>/index.sh (ADR-010); src/assertions.sh is the one
# flat-file aggregator, which has no directory of its own.
function build_aggregators() {
echo "src/assertions.sh"
local index
for index in "$ROOT_DIR"/src/*/index.sh; do
[ -f "$index" ] || continue
echo "${index#"$ROOT_DIR"/}"
done
}

function test_module_aggregators_hold_only_source_lines_and_comments() {
local offenders=""
local aggregator
for aggregator in $aggregators; do
while IFS= read -r aggregator; do
if grep -qvE '^[[:space:]]*(#|source |$)' "$ROOT_DIR/$aggregator"; then
offenders="$offenders $aggregator"
fi
done
done < <(build_aggregators)

assert_empty "$offenders"
}

# The glob above is only a safety net if it actually finds the modules.
function test_module_aggregator_discovery_finds_every_module() {
local found
found=$(build_aggregators | tr '\n' ' ')

assert_contains "src/runner/index.sh" "$found"
assert_contains "src/coverage/index.sh" "$found"
assert_contains "src/assertions.sh" "$found"
}

function test_build_process_file_embeds_a_file_only_once() {
local dir
dir=$(bashunit::temp_dir)
Expand Down
Loading