refactor(coverage): split src/coverage.sh into a src/coverage/ module #925

Activity

1. Chemaclass

   added

   refactoringRefactoring or cleaning relatedRefactoring or cleaning related

   on Jul 30, 2026
2. github-project-automation

   added this to bashuniton Jul 30, 2026
3. Chemaclass

   self-assigned this

   on Jul 30, 2026
4. Chemaclass commented on Jul 30, 2026

   

   Chemaclass

   on Jul 30, 2026

   MemberAuthor

   More actions

   Responsibility map (step 1 — for review before any code moves)

   src/coverage.sh on main (c34bff5): 2548 lines, 59 functions. Data below is derived
   mechanically from the file, not from memory.

   Proposed layout — 12 files

   file	fns	~lines	owns
   coverage.sh	0	16	aggregator: source lines + comments only
   coverage/config.sh	4	104	auto_discover_paths, init, xtrace_is_supported, resolve_engine — path discovery, engine selection
   coverage/paths.sh	4	121	normalize_path, should_track, get_tracked_files, path_to_filename — is-this-file-tracked decisions + caches
   coverage/engine.sh	10	254	enable_trap/disable_trap, _enable_xtrace/_disable_xtrace, record_line, flush_buffer, _resolve_output_files, finalize, cleanup, aggregate_parallel — capture, buffering, teardown
   coverage/lines.sh	8	198	is_executable_line, get_executable_lines, get_hit_lines, compute_file_coverage, _ends_with_continuation, get_all_line_hits, load_hits_by_line, get_all_line_tests
   coverage/stats.sh	8	119	get_coverage_class, get_color_for_class, calculate_percentage, get_file_stats, precompute_file_stats, get_cached_stats, get_percentage, check_threshold
   coverage/functions.sh	1	92	extract_functions
   coverage/branches.sh	14	193	_append_arm, _is_case_pattern_line, the nine _branch_*, extract_branches, _arm_taken, compute_branch_hits
   coverage/report_text.sh	5	232	report_text + _uncovered/_line_hits/_functions, _compress_ranges
   coverage/report_lcov.sh	1	85	report_lcov
   coverage/report_html.sh	2	64	html_escape, report_html (the orchestrator)
   coverage/html_index.sh	1	362	generate_index_html
   coverage/html_file.sh	1	410	generate_file_html

   All 59 functions are assigned; none dropped, none duplicated (verified by set difference).

   Layering is acyclic

   Derived from all 91 internal call edges:

   config · paths · lines · functions          (leaves / near-leaves)
     → engine · stats · branches
       → report_text · report_lcov · report_html → html_index · html_file
   

   No edge runs backwards. Nothing in paths/lines/stats/branches calls into engine,
   so the capture path is a true sink. branches depends on lines only
   (_arm_taken → is_executable_line, compute_branch_hits → load_hits_by_line).

   Globals

   Every _BASHUNIT_COVERAGE_* global stays with the group that writes it:

   group	globals
   config	_DATA_FILE, _TRACKED_FILES, _TRACKED_CACHE_FILE, _TEST_HITS_FILE, _ENGINE_RESOLVED, _IS_PARALLEL
   paths	_TRACK_CACHE, _PATH_CACHE
   engine	_BUFFER, _BUFFER_COUNT, _BUFFER_LIMIT, _HITS_BUFFER, _XTRACE_* (6), _DATA_TARGET_OUT, _HITS_TARGET_OUT, both awk program strings
   lines	_NONEXEC_PATTERN, _HITS_BY_LINE
   stats	_STATS_FILES/EXEC/HIT/PCT/CLASS/COUNT/LOOKUP
   branches	_BASHUNIT_BRANCH_ARMS_OUT, _BASHUNIT_ARM_TAKEN_OUT
   report_text	_BASHUNIT_RANGES_OUT

   Crossing the module boundary — only three, all pre-existing:

   • _BASHUNIT_COVERAGE_CURRENT_TEST_FILE and _BASHUNIT_COVERAGE_CURRENT_TEST_FN are
     written by src/runner/context.sh and read here. Inbound; unchanged by the split.
   • _BASHUNIT_RUNNER_RESULT_ORDINAL is read here and owned by src/runner/payload.sh.
     Stays a plain global — not to be "fixed" in this PR.

   The .editorconfig decision — narrower than the issue assumed

   The issue notes the 370-char line. The real number: 56 lines exceed 120 chars, and they
   are concentrated almost entirely in the two HTML template emitters:

   generate_index_html   26
   generate_file_html    24
   aggregate_parallel     3
   record_line            1
   init                   1
   get_cached_stats       1
   

   So the recommendation is not to widen the glob to the whole directory. Replace
   [src/coverage.sh] max_line_length = unset with the same rule scoped to
   html_index.sh and html_file.sh only, and reflow the remaining 6 lines. Every other
   module then honours the global 120 limit — a net tightening rather than a blanket exemption.

   Constraints carried forward

   • branches.sh must stay one file. extract_branches keeps if_decision_line,
     if_arms, if_depth, if_arm_start as locals that the _branch_* helpers mutate
     through Bash's dynamic scoping. The justification comment (now src/coverage.sh:1111-1112
     and :1200-1201, not :818-821 as the issue says) travels with them.
   • --coverage paths are nightly / non-gating (.github/workflows/coverage.yml), so
     ./bashunit --coverage --parallel tests/ has to be run explicitly before the PR.
   • Tests stay flat. The 9 existing tests/unit/coverage_*_test.sh files already
     approximate this split; they get renamed to match the new modules, never moved into
     tests/unit/coverage/ (make test globs one level).
   • src/coverage.sh:173's reference to runner.sh was already retargeted to
     runner/exec.sh + runner/hooks.sh in refactor(runner): split src/runner.sh into a src/runner/ module #927.

   Verification plan

   Same method that made #924 reviewable: compare the non-blank line multiset before and
   after (must be identical, each line exactly once) and the function count (59), then
   make sa, make lint, sequential, --parallel, --parallel --simple --strict,
   --coverage --parallel, and bash build.sh bin -v.

   ---

   Holding here for review, per acceptance criterion 1 — no code has moved. Two calls I'd
   like confirmed before starting:

   1. 12 files as above, splitting the 836-line HTML block into three, rather than one
      report_html.sh?
   2. The narrow .editorconfig scoping (2 files exempt, 6 lines reflowed) over widening
      the exemption to the whole src/coverage/ directory?
5. Chemaclass commented on Jul 31, 2026

   

   Chemaclass

   on Jul 31, 2026

   MemberAuthor

   More actions

   Responsibility map for src/coverage.sh

   Step 1 deliverable, posted for review before any code moves. Measured against c34bff5
   (post-#927): 2548 lines, 59 functions, 33 module-owned globals, 13 of those functions
   called from outside the file.

   Call graph and globals table below are generated from the file, not eyeballed.

   ---

   Proposed layout

   src/coverage/ behind a thin aggregator, mirroring src/runner/. Namespace unchanged,
   no renames, bashunit:76 keeps its single source.

   File	Functions	~Lines
   coverage/paths.sh	normalize_path should_track auto_discover_paths get_tracked_files	154
   coverage/lines.sh	is_executable_line get_executable_lines get_hit_lines	95
   coverage/hits.sh	get_all_line_hits load_hits_by_line get_all_line_tests _ends_with_continuation	113
   coverage/functions.sh	extract_functions	99
   coverage/branches.sh	_append_arm _is_case_pattern_line _branch_push_if _branch_close_if_arm _branch_emit_if _branch_push_case _branch_close_case_arm _branch_emit_case _branch_open_case_pattern _branch_push_loop _branch_emit_loop extract_branches _arm_taken compute_branch_hits	229
   coverage/engine.sh	init cleanup xtrace_is_supported resolve_engine enable_trap disable_trap _enable_xtrace _disable_xtrace record_line _resolve_output_files flush_buffer finalize	402
   coverage/parallel.sh	aggregate_parallel	68
   coverage/stats.sh	get_coverage_class get_color_for_class calculate_percentage compute_file_coverage get_file_stats precompute_file_stats get_cached_stats get_percentage check_threshold	171
   coverage/report_text.sh	report_text _compress_ranges report_text_uncovered report_text_line_hits report_text_functions	249
   coverage/report_lcov.sh	report_lcov	86
   coverage/report_html.sh	html_escape path_to_filename report_html generate_index_html generate_file_html	849

   Layering (leaves first, acyclic — verified against the generated call graph):

   paths · lines · functions
      └─ hits
           ├─ branches
           ├─ stats ── report_text
           │            report_html
           └─ report_lcov
   engine ── paths            (parallel is a leaf on the data files)
   

   Aggregator source order: paths · lines · hits · functions · branches · engine · parallel · stats · report_text · report_lcov · report_html.

   ---

   Function summaries

   paths.sh — what counts as a tracked file

   • normalize_path — collapse a path to the canonical form used as the data-file key
   • should_track — whether a path is inside the configured coverage roots (memoised, 99 lines, the hottest predicate on the trap path)
   • auto_discover_paths — derive coverage roots when none configured
   • get_tracked_files — list the files that ended up tracked

   lines.sh — static line classification

   • is_executable_line — whether a source line can ever be hit, via _BASHUNIT_COVERAGE_NONEXEC_PATTERN
   • get_executable_lines — count them for a file
   • get_hit_lines — count executable lines with at least one hit

   hits.sh — reading recorded hit data

   • get_all_line_hits — parse the data file into per-line hit counts, folding \-continuations
   • _ends_with_continuation — the continuation test that folding relies on
   • load_hits_by_line — memoising wrapper populating _BASHUNIT_COVERAGE_HITS_BY_LINE
   • get_all_line_tests — per-line attribution of which test hit it

   functions.sh

   • extract_functions — locate function definitions and their line spans (used by lcov, text and html reports)

   branches.sh — branch coverage (ADR-007)

   • extract_branches — single pass building the arm table for if/case/loop
   • _append_arm — push an arm onto _BASHUNIT_BRANCH_ARMS_OUT
   • _is_case_pattern_line — recognise a case pattern line
   • _branch_push_if _branch_close_if_arm _branch_emit_if — if/elif/else arm bookkeeping
   • _branch_push_case _branch_close_case_arm _branch_emit_case _branch_open_case_pattern — case arm bookkeeping
   • _branch_push_loop _branch_emit_loop — loop arm bookkeeping
   • _arm_taken — whether an arm was executed
   • compute_branch_hits — taken/total for a file

   engine.sh — tracing lifecycle (ADR-009)

   • init — one-time setup of data files, caches, engine choice (cross-cutting, see below)
   • cleanup — remove the run's data files
   • xtrace_is_supported — BASH_XTRACEFD probe (4.1+)
   • resolve_engine — resolve auto to xtrace or trap
   • enable_trap / disable_trap — the seam runner/exec.sh and runner/hooks.sh call
   • _enable_xtrace / _disable_xtrace — xtrace engine, PS4 swap and trace drain
   • record_line — the DEBUG-trap handler, buffered
   • flush_buffer — write the buffer out
   • _resolve_output_files — pick the data/hits targets (parallel-aware)
   • finalize — flush and normalise at end of run

   parallel.sh

   • aggregate_parallel — merge per-worker data files after a parallel run

   stats.sh — percentages and the precomputed cache

   • calculate_percentage · get_coverage_class · get_color_for_class — arithmetic and threshold banding
   • compute_file_coverage — executable/hit pair for one file
   • get_file_stats — that pair plus percentage and class
   • precompute_file_stats / get_cached_stats — fill and read the _BASHUNIT_COVERAGE_STATS_* arrays
   • get_percentage — run-wide percentage
   • check_threshold — exit-code gate

   report_text.sh / report_lcov.sh / report_html.sh — the three renderers, plus
   _compress_ranges (uncovered line ranges), html_escape, path_to_filename.

   ---

   Globals

   33 module-owned globals. Full assign/read map generated; the parts that constrain the split:

   Global	Assigned by	Lands in
   _BASHUNIT_COVERAGE_TRACKED_FILES _TRACK_CACHE _PATH_CACHE _TRACKED_CACHE_FILE	init, record_line	paths.sh
   _BASHUNIT_COVERAGE_NONEXEC_PATTERN	file scope, 7 contiguous lines	lines.sh
   _BASHUNIT_COVERAGE_HITS_BY_LINE	load_hits_by_line	hits.sh
   _BASHUNIT_BRANCH_ARMS_OUT _BASHUNIT_ARM_TAKEN_OUT	_append_arm, _arm_taken	branches.sh
   _BASHUNIT_COVERAGE_BUFFER* _HITS_BUFFER _DATA_FILE _TEST_HITS_FILE _ENGINE_RESOLVED _IS_PARALLEL _XTRACE_* _DATA/HITS_TARGET_OUT	init, record_line, flush_buffer, _resolve_output_files, _enable/_disable_xtrace	engine.sh
   _BASHUNIT_COVERAGE_STATS_* (7)	init, precompute_file_stats	stats.sh
   _BASHUNIT_RANGES_OUT	_compress_ranges	report_text.sh

   Crossing the file boundary, both read-only and left as-is:

   • _BASHUNIT_RUNNER_RESULT_ORDINAL (src/coverage.sh:224) — as the issue instructs
   • _BASHUNIT_COVERAGE_CURRENT_TEST_FILE / _CURRENT_TEST_FN — written by src/runner/context.sh
   • _BASHUNIT_COLOR_*, _BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_{LOW,HIGH} — normal shared config

   ---

   Three things the map turned up that need a decision

   1. init is the one genuinely cross-cutting function. It assigns globals owned by
   five proposed modules (paths caches, engine buffers, stats arrays, tracked files, engine
   choice). Splitting it would be a behaviour change, which this issue forbids, so it stays
   whole in engine.sh with a comment naming the modules it resets. This is a deliberate,
   documented cross-module write — the mirror of the _BASHUNIT_RUNNER_RESULT_ORDINAL
   cross-module read already sanctioned above. Flagging it rather than hiding it.

   2. .editorconfig — scope the exception to one file, do not widen to the directory.
   Of the 56 lines over 120 chars, 50 are in generate_index_html (26) and
   generate_file_html (24), both landing in report_html.sh. So:

   [src/coverage/report_html.sh]
   max_line_length = unset

   Every other new file inherits the global 120. That leaves 6 lines to reflow by hand
   (aggregate_parallel ×3, record_line, init, get_cached_stats) — the only non-relocation
   edit in the diff, and I'd call it out explicitly in the PR. Widening the glob to
   src/coverage/** would silently drop the limit for nine files that do not need it.

   3. Two file-scope ordering constraints (ADR-010). The aggregator takes only source
   lines and comments, so every file-scope initializer moves into its module. Two of them are
   order-dependent within their file and must stay contiguous and in order:

   • _BASHUNIT_COVERAGE_XTRACE_PS4 expands $_BASHUNIT_COVERAGE_XTRACE_FS at file scope → both in engine.sh, FS first
   • _BASHUNIT_COVERAGE_NONEXEC_PATTERN is built across 7 successive ="${...}"'|...' lines → all in lines.sh

   Also worth noting: the "name kept from the trap-only era" comment the issue asks about is
   already updated — #927 rewrote it to reference runner/exec.sh and runner/hooks.sh.
   No action needed on that bullet.

   ---

   Tests

   The 9 existing flat tests/unit/coverage_*_test.sh files map onto this split cleanly and
   stay flat, no tests/unit/coverage/:

   core→paths+engine · engine→engine · executable→lines · branches→branches ·
   percent+helpers→stats · reporting→report_* · parallel_aggregation→parallel ·
   subshell→engine

   No renames look justified; every existing file's scope maps to one or two new modules
   without moving.

   ---

   Awaiting review before moving any code.
6. Chemaclass commented on Jul 31, 2026

   

   Chemaclass

   on Jul 31, 2026

   MemberAuthor

   More actions

   Agreed: the 13-file layout from the 30 Jul map, with the narrow .editorconfig scoping (exempt html_index.sh + html_file.sh, reflow the remaining 6 lines).

   The second map posted on 31 Jul was a duplicate — I had read the issue body but not the existing comments. It reached the same conclusions independently (same 56/50 long-line split, same narrow editorconfig recommendation, same single-file branches.sh, same note that #927 already retargeted the trap-only-era comment), so treat it as corroboration and follow the 30 Jul layout. Its only correction worth keeping: the dynamic-scope justification comment is at :1111-1112 / :1200-1201, not :818-821 as the issue body says.

   Starting the split now.
7. Chemaclass commented on Jul 31, 2026

   

   Chemaclass

   on Jul 31, 2026

   MemberAuthor

   More actions

   Done in #928 (merged).
8. Chemaclass

   closed this as completedon Jul 31, 2026
9. github-project-automation

   moved this to Done in bashuniton Jul 31, 2026
10. Chemaclass

    added a commit that references this issue on Jul 31, 2026

    docs(adr): record what the coverage split taught ADR-010

    ...

    ded8d88