Skip to content

refactor(runner): split src/runner.sh into a src/runner/ module #924

Description

@Chemaclass

Summary

src/runner.sh is 2145 lines and 57 functions covering five distinct
responsibilities (.claude/rules/architecture-map.md:51 already names them: "file
loop, per-test execution, retry/timeout, result parsing, failure context"). Split it
into a src/runner/ module behind a thin aggregator, following the existing
src/assertions.shsrc/assert_*.sh pattern.

Blocked by #923 — path-keyed embed markers must land first, or
src/runner/parallel.sh collides with src/parallel.sh and breaks
test_built_binary_embeds_each_src_file_exactly_once.

This is a pure move

The namespace stays bashunit::runner::*. No function is renamed, no call site
changes.
Every hunk should be a verbatim relocation. If a behavior fix looks
necessary along the way, open a separate issue for it — do not fold it in, or the
diff stops being reviewable.

bashunit:90 keeps its single source "$BASHUNIT_ROOT_DIR/src/runner.sh".
src/runner.sh becomes an aggregator holding only source lines and comments
(required — see the aggregator rule in #923).

Target layout

file ~lines functions
src/runner.sh 14 aggregator only
src/runner/context.sh 145 restore_workdir, sync_coverage_flag, export_test_identity, resolve_test_location, apply_interpolated_title, source_login_shell_profiles, needs_test_duration, _supports_reliable_pipefail
src/runner/payload.sh 215 the 13 _BASHUNIT_RUNNER_*_OUT slot globals + extract_encoded_field, compute_total_assertions, extract_subshell_type, format_subshell_output, decode_subshell_output, extract_result_counts, is_simple_progress_output, line_exists_in_output, extract_assertion_runtime_output
src/runner/diagnostics.sh 135 detect_runtime_error, classify_kill_signal, halt_if_stop_on_failure, record_profile, print_verbose_test_summary, render_running_file_header
src/runner/result.sh 215 parse_result{,_sync,_parallel}, get_failure_source_context, write_{failure,skipped,incomplete,risky}_result_output
src/runner/parallel.sh 90 _supports_wait_n, _count_running_jobs, wait_for_job_slot, spinner
src/runner/hooks.sh 380 run_set_up{,_before_script}, run_tear_down{,_after_script}, execute_{file,test}_hook, record_{file,test}_hook_failure, clear_mocks, clean_script_test_functions, clean_set_up_and_tear_down_after_script, cleanup_on_exit
src/runner/provider.sh 120 parse_data_provider_args
src/runner/exec.sh 450 run_test, execute_test_body, run_with_timeout, build_timeout_result, call_test_functions
src/runner/discovery.sh 280 load_test_files, functions_for_script
src/runner/bench.sh 100 load_bench_files, call_bench_functions

The dependency layering is acyclic — 53 of 57 functions are leaves; only
load_test_files, load_bench_files, run_test and parse_result have callees:

context · payload · diagnostics  →  parallel · hooks · result  →  provider · exec  →  discovery · bench

src/runner/parallel.sh deliberately shares a basename with src/parallel.sh; that
is the case #923 makes safe.

Tests: keep them flat

Do not create tests/unit/runner/. make test globs
$(wildcard $(TEST_SCRIPTS_DIR)/*/*[tT]est.sh) — one level only (152 files vs 156
recursive). Anything at tests/unit/runner/payload_test.sh is silently skipped
by make test, which CI runs (.github/workflows/tests.yml:29, :90). Deepening
the glob is not an option either: the 4 extra recursive matches are deliberately
excluded fixtures (tests/unit/fixtures/tests/example1_test.sh and friends).

So split tests/unit/runner_test.sh (430 lines) into flat
tests/unit/runner_<area>_test.sh files mirroring the src modules — the same
convention coverage_*_test.sh already uses (9 files).

Watch out for

  • File-scoped ShellCheck directive. src/runner.sh:2 is
    # shellcheck disable=SC2155, which currently covers the whole file. Carry it
    only to the new files that actually need it, or narrow it to specific lines.
    Do not blanket-apply it to all ten.
  • Return-slot globals now cross files. src/runner/payload.sh declares the 13
    _BASHUNIT_RUNNER_*_OUT slots; src/runner/exec.sh reads them. Expect SC2034 /
    SC2154 noise and resolve it the way src/state.sh:16-17 already does (a scoped
    disable with a comment naming the reader), not with a blanket file-level disable.
  • First line must be the shebang. build.sh's tail -n +2 strips line 1 of
    every embedded file.
  • jobs -pr must stay in the dispatcher shell. wait_for_job_slot and
    _count_running_jobs depend on running in the shell that spawned the workers.
    Moving them to another file is fine; wrapping them in a subshell is not.
  • _BASHUNIT_RUNNER_RESULT_ORDINAL is read outside the runner
    src/coverage.sh:224. It stays a plain global; do not scope it.
  • No new forks. .claude/rules/perf-fork-budget.md budgets are enforced by
    tests/acceptance/bashunit_{coldstart,run}_forks_test.sh on three platforms.
    A file split adds no forks (parse time dominates cold start, file opens don't) —
    keep it that way.

Docs to update

  • .claude/rules/architecture-map.md:51 — the single runner.sh row becomes the
    module table
  • .claude/rules/architecture-map.md:17 — the call-flow annotation (runner.sh: the per-file loop)src/runner/discovery.sh
  • .claude/rules/bash-style.md:121 — the return-slot example points at
    src/runner.sh; retarget to src/runner/payload.sh
  • New adrs/adr-010-*.md using adrs/TEMPLATE.md: why the module split, why the
    aggregator pattern, why tests stay flat

Acceptance criteria

  • git diff shows only relocations — no renamed functions, no changed logic
  • bashunit:90 still sources exactly one runner file
  • src/runner.sh contains only source lines and comments
  • Every new file's first line is #!/usr/bin/env bash
  • ./bashunit tests/ green
  • ./bashunit --parallel tests/ green
  • make sa && make lint green
  • bash build.sh bin -v prints ✅ Build verified ✅
  • Fork-budget acceptance tests green
  • tests/unit/runner_test.sh split into flat per-area files, all picked up by
    make test/list
  • Bash 3.0+ compatible
  • No CHANGELOG entry — internal, no user-visible behavior change
  • ADR added

Do not

  • Do not rename any function or change any behavior
  • Do not create tests/unit/runner/ (see above)
  • Do not touch src/coverage.sh — separate issue
  • Do not run shfmt -w; make lint is the format gate

Metadata

Metadata

Assignees

Labels

refactoringRefactoring or cleaning related

Type

No type

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions