os-sdlc: reorganize tests/ to mirror lib with an explicit layout convention #296

New issue

Open

opened 2026-08-06 20:30:43 +00:00 by jared · 2 comments

jared commented 2026-08-06 20:30:43 +00:00

Owner

Copy link

os-sdlc: reorganize tests/ to mirror lib with an explicit layout convention

Problem Statement

The os-sdlc test suite only partially follows the accepted Ruby/minitest convention that test files mirror the production files they test. tests/cops/ mirrors lib/os_sdlc/cops/ correctly, but the rest of tests/ is flat while lib/os_sdlc/ has grown subdirectories (transition_table/, brief/). Several test files are multi-class grab-bags: artifact_test.rb (647 lines, 8 test classes), cli_test.rb (1,064 lines, 15 test classes), transition_table_wave2_test.rb (680 lines, 51 tests, named after a migration wave rather than a subject), and two hardening_* files named after nothing in lib. Given a class, you cannot reliably find its tests; given a failing test file, you cannot tell which unit regressed. No recorded convention exists to measure new test files against.

Solution

Adopt and record one explicit layout rule, then mechanically reorganize the existing suite to satisfy it — pure moves, splits, and renames, with zero change to test behavior, verified by an identical before/after test inventory.

The rule: every file tests/<subpath>/<base>[_<aspect>]_test.rb must correspond to lib/os_sdlc/<subpath>/<base>.rb. Files under tests/integration/ are exempt and are named after the process boundary they drive (cli_*, bin_*). The rule keys off public entry points, not off every .rb file: private collaborators behind a facade (e.g. the Artifact internals, transition-table row modules) are legitimately tested through their facade and do not each require a mirrored test file.

User Stories

1. As a developer, I want each test file's name to identify the lib file it tests, so that I can navigate from a class to its tests without searching.
2. As a developer, I want a failing test file's name to identify the regressed unit, so that diagnosis starts in the right place.
3. As a developer, I want the tests/ tree to mirror lib subdirectories (transition_table/, matching the existing cops/ precedent), so that the two trees stay structurally symmetric.
4. As a developer, I want multi-class grab-bag files (artifact_test.rb, cli_test.rb, transition_table_wave2_test.rb) split on their existing class seams, so that each file has one subject.
5. As a developer, I want process-boundary tests (CLI, bin scripts) in a tests/integration/ directory, so that they are recognizable as boundary tests and a fast unit-only glob is a one-line change later.
6. As a developer, I want aspect-suffixed sibling files (e.g. artifact_verdict_test.rb) allowed under the rule, so that a facade with a lot of behavior can spread across focused files without inventing fake units.
7. As a developer, I want the hardening_* grab-bag tests redistributed into the unit files of the classes they exercise, so that no test file is named after a hardening campaign instead of a subject.
8. As a developer, I want the "wave2" helper and test names replaced with subject-based names, so that file names describe content, not migration history.
9. As a developer, I want the reorganization proven behavior-preserving by an identical before/after inventory of ClassName#test_name pairs and minitest run counts, so that no test is silently dropped or lost in a rename.
10. As a developer, I want the layout rule recorded as an ADR, so that future test files are measured against a recorded decision rather than folklore.
11. As a developer, I want the duplicate top-level Widget fixture class resolved before any split, so that the flat test-class namespace cannot produce a constant collision.
12. As a maintainer, I want inline helper/fixture classes that cross a split boundary moved into the flat Zeitwerk-loaded support directory, so that splits do not duplicate helpers.
13. As a maintainer, I want the reorganization to stay within ADR-0101's constraints (flat support directory, filename-matches-constant, single require entrypoint), so that no accepted decision is silently reversed.
14. As a maintainer, I want files that already satisfy the rule (all of tests/cops/, the lint_worklist_* and fixture_ac_coverage_lint_* aspect families, ~40 mirrored flat files) left untouched, so that the diff stays reviewable.

Implementation Decisions

• The layout rule (stated in Solution) is the deliverable convention; it must be recorded via the ADR system as part of this work, since no existing ADR covers test layout.
• artifact_test.rb splits into four files on existing class seams: artifact basics/ticket-text/metadata stay; verdict recording/raw/integrity classes move to an artifact_verdict aspect file; corruption classes to an artifact_corruption aspect file; ArtifactStepperStateTest to its own file mirroring its lib counterpart. The shared ArtifactFixtures helper already lives in the support directory — no helper movement needed; this split goes first as the cleanest.
• cli_test.rb moves to tests/integration/ split by command: gate, ac-lint, lint, verdict, misc (disclose/retired-verbs/project-config).
• transition_table_wave2_test.rb splits into per-row-module files under tests/transition_table/ (green_assert_rows, lint_gate_rows, ac_lint_rows, review_rows, shard_plan_rows); the whole-table walk classes move to tests/integration/ as a transition-table walk test. The wave2 support helper is renamed to a subject-based name, constant renamed to match (Zeitwerk).
• The existing transition_table_integrity test stays flat, renamed to mirror the lib transition_table entry file it tests.
• The two hardening_* files are dissolved: each test class moves into the unit file for the production class it exercises (verdict → artifact_verdict; lint root config → lint; project execute-then-persist → project; preflight → workspace; review packet → review_packet_diff; disclose multi-note → integration cli misc).
• Mixed-mode files keep their unit classes flat and move only their CLI test classes to tests/integration/ (agent_budget, autocorrect_prepass, shard_plan, lint_worklist_residual_dispatch, workspace_housekeeping).
• cli_intake, cli_step, workspace-bin, and target-threading tests move to tests/integration/ as renames only.
• The duplicate top-level Widget fixture class (defined in two test files) is extracted to the support directory before the cli split.
• Inline helper/base classes needed across a split boundary move to the flat support directory with Zeitwerk-conformant names; support subdirectories are forbidden (ADR-0101).
• The test runner needs no change: it already globs test files recursively (proven by tests/cops/). No exclusion list may be introduced (ADR-0053/0075 lineage). Relative requires of the test helper are adjusted for moved files; a missed one fails loudly at load.
• A rejected alternative, recorded for the ADR: a top-level unit/ vs integration/ split of the whole tree — churns ~100 files for one bit of information and breaks lib↔test path symmetry.

Testing Decisions

• This change is itself test-only; the "tests" are the invariants proving behavior preservation.
• Good verification checks external behavior only: the suite's full run via the single existing entrypoint, not any implementation detail of the reorganization.
• Invariants, checked before and after: (1) total test-method count is identical (baseline 781); (2) the sorted inventory of ClassName#test_name pairs is identical modulo the explicitly planned class renames; (3) minitest's own reported run count from the full-suite entrypoint matches, catching a file dropped from the glob; (4) no duplicate top-level class/module names across test files; (5) the suite passes under more than one fixed seed, guarding against sort-order-dependent state leakage (the glob is sorted, and file renames change execution order).
• Prior art: tests/cops/ demonstrates the mirrored-subdirectory pattern; the lint_worklist_* and fixture_ac_coverage_lint_* families demonstrate the aspect-suffix pattern.
• No new tests are written in this change; coverage gaps discovered during the audit are follow-up tickets, not scope.

Out of Scope

• New unit tests for currently facade-covered or untested classes (escalation_guard, test_runner, assignment_file, stepper_status_classifier, rubocop_delta_report gap list) — follow-up.
• A fast unit-only entrypoint excluding tests/integration/ — follow-up once the directory exists.
• A cop enforcing the mirror rule mechanically — follow-up.
• Optional splits of project_test and workspace_test — deferred until the artifact/cli splits prove the process.
• Any change to production code under lib/.

Further Notes

• Recommended execution order: resolve Widget duplication → artifact split → cli split → transition_table split → hardening redistribution → integration moves/renames → ADR.
• The original premise ("artifact_test.rb tests every Artifact* class") was investigated and corrected: it tests only the Artifact facade plus one stowaway stepper-state class; the Artifact internals are private collaborators, and the convention deliberately does not force per-collaborator test files.

# os-sdlc: reorganize tests/ to mirror lib with an explicit layout convention ## Problem Statement The os-sdlc test suite only partially follows the accepted Ruby/minitest convention that test files mirror the production files they test. `tests/cops/` mirrors `lib/os_sdlc/cops/` correctly, but the rest of `tests/` is flat while `lib/os_sdlc/` has grown subdirectories (`transition_table/`, `brief/`). Several test files are multi-class grab-bags: `artifact_test.rb` (647 lines, 8 test classes), `cli_test.rb` (1,064 lines, 15 test classes), `transition_table_wave2_test.rb` (680 lines, 51 tests, named after a migration wave rather than a subject), and two `hardening_*` files named after nothing in lib. Given a class, you cannot reliably find its tests; given a failing test file, you cannot tell which unit regressed. No recorded convention exists to measure new test files against. ## Solution Adopt and record one explicit layout rule, then mechanically reorganize the existing suite to satisfy it — pure moves, splits, and renames, with zero change to test behavior, verified by an identical before/after test inventory. The rule: every file `tests/<subpath>/<base>[_<aspect>]_test.rb` must correspond to `lib/os_sdlc/<subpath>/<base>.rb`. Files under `tests/integration/` are exempt and are named after the process boundary they drive (`cli_*`, `bin_*`). The rule keys off public entry points, not off every `.rb` file: private collaborators behind a facade (e.g. the Artifact internals, transition-table row modules) are legitimately tested through their facade and do not each require a mirrored test file. ## User Stories 1. As a developer, I want each test file's name to identify the lib file it tests, so that I can navigate from a class to its tests without searching. 2. As a developer, I want a failing test file's name to identify the regressed unit, so that diagnosis starts in the right place. 3. As a developer, I want the tests/ tree to mirror lib subdirectories (transition_table/, matching the existing cops/ precedent), so that the two trees stay structurally symmetric. 4. As a developer, I want multi-class grab-bag files (artifact_test.rb, cli_test.rb, transition_table_wave2_test.rb) split on their existing class seams, so that each file has one subject. 5. As a developer, I want process-boundary tests (CLI, bin scripts) in a tests/integration/ directory, so that they are recognizable as boundary tests and a fast unit-only glob is a one-line change later. 6. As a developer, I want aspect-suffixed sibling files (e.g. artifact_verdict_test.rb) allowed under the rule, so that a facade with a lot of behavior can spread across focused files without inventing fake units. 7. As a developer, I want the hardening_* grab-bag tests redistributed into the unit files of the classes they exercise, so that no test file is named after a hardening campaign instead of a subject. 8. As a developer, I want the "wave2" helper and test names replaced with subject-based names, so that file names describe content, not migration history. 9. As a developer, I want the reorganization proven behavior-preserving by an identical before/after inventory of ClassName#test_name pairs and minitest run counts, so that no test is silently dropped or lost in a rename. 10. As a developer, I want the layout rule recorded as an ADR, so that future test files are measured against a recorded decision rather than folklore. 11. As a developer, I want the duplicate top-level Widget fixture class resolved before any split, so that the flat test-class namespace cannot produce a constant collision. 12. As a maintainer, I want inline helper/fixture classes that cross a split boundary moved into the flat Zeitwerk-loaded support directory, so that splits do not duplicate helpers. 13. As a maintainer, I want the reorganization to stay within ADR-0101's constraints (flat support directory, filename-matches-constant, single require entrypoint), so that no accepted decision is silently reversed. 14. As a maintainer, I want files that already satisfy the rule (all of tests/cops/, the lint_worklist_* and fixture_ac_coverage_lint_* aspect families, ~40 mirrored flat files) left untouched, so that the diff stays reviewable. ## Implementation Decisions - The layout rule (stated in Solution) is the deliverable convention; it must be recorded via the ADR system as part of this work, since no existing ADR covers test layout. - artifact_test.rb splits into four files on existing class seams: artifact basics/ticket-text/metadata stay; verdict recording/raw/integrity classes move to an artifact_verdict aspect file; corruption classes to an artifact_corruption aspect file; ArtifactStepperStateTest to its own file mirroring its lib counterpart. The shared ArtifactFixtures helper already lives in the support directory — no helper movement needed; this split goes first as the cleanest. - cli_test.rb moves to tests/integration/ split by command: gate, ac-lint, lint, verdict, misc (disclose/retired-verbs/project-config). - transition_table_wave2_test.rb splits into per-row-module files under tests/transition_table/ (green_assert_rows, lint_gate_rows, ac_lint_rows, review_rows, shard_plan_rows); the whole-table walk classes move to tests/integration/ as a transition-table walk test. The wave2 support helper is renamed to a subject-based name, constant renamed to match (Zeitwerk). - The existing transition_table_integrity test stays flat, renamed to mirror the lib transition_table entry file it tests. - The two hardening_* files are dissolved: each test class moves into the unit file for the production class it exercises (verdict → artifact_verdict; lint root config → lint; project execute-then-persist → project; preflight → workspace; review packet → review_packet_diff; disclose multi-note → integration cli misc). - Mixed-mode files keep their unit classes flat and move only their CLI test classes to tests/integration/ (agent_budget, autocorrect_prepass, shard_plan, lint_worklist_residual_dispatch, workspace_housekeeping). - cli_intake, cli_step, workspace-bin, and target-threading tests move to tests/integration/ as renames only. - The duplicate top-level Widget fixture class (defined in two test files) is extracted to the support directory before the cli split. - Inline helper/base classes needed across a split boundary move to the flat support directory with Zeitwerk-conformant names; support subdirectories are forbidden (ADR-0101). - The test runner needs no change: it already globs test files recursively (proven by tests/cops/). No exclusion list may be introduced (ADR-0053/0075 lineage). Relative requires of the test helper are adjusted for moved files; a missed one fails loudly at load. - A rejected alternative, recorded for the ADR: a top-level unit/ vs integration/ split of the whole tree — churns ~100 files for one bit of information and breaks lib↔test path symmetry. ## Testing Decisions - This change is itself test-only; the "tests" are the invariants proving behavior preservation. - Good verification checks external behavior only: the suite's full run via the single existing entrypoint, not any implementation detail of the reorganization. - Invariants, checked before and after: (1) total test-method count is identical (baseline 781); (2) the sorted inventory of ClassName#test_name pairs is identical modulo the explicitly planned class renames; (3) minitest's own reported run count from the full-suite entrypoint matches, catching a file dropped from the glob; (4) no duplicate top-level class/module names across test files; (5) the suite passes under more than one fixed seed, guarding against sort-order-dependent state leakage (the glob is sorted, and file renames change execution order). - Prior art: tests/cops/ demonstrates the mirrored-subdirectory pattern; the lint_worklist_* and fixture_ac_coverage_lint_* families demonstrate the aspect-suffix pattern. - No new tests are written in this change; coverage gaps discovered during the audit are follow-up tickets, not scope. ## Out of Scope - New unit tests for currently facade-covered or untested classes (escalation_guard, test_runner, assignment_file, stepper_status_classifier, rubocop_delta_report gap list) — follow-up. - A fast unit-only entrypoint excluding tests/integration/ — follow-up once the directory exists. - A cop enforcing the mirror rule mechanically — follow-up. - Optional splits of project_test and workspace_test — deferred until the artifact/cli splits prove the process. - Any change to production code under lib/. ## Further Notes - Recommended execution order: resolve Widget duplication → artifact split → cli split → transition_table split → hardening redistribution → integration moves/renames → ADR. - The original premise ("artifact_test.rb tests every Artifact* class") was investigated and corrected: it tests only the Artifact facade plus one stowaway stepper-state class; the Artifact internals are private collaborators, and the convention deliberately does not force per-collaborator test files.

jared added the

ready-for-agent

label 2026-08-06 20:30:46 +00:00

jared added the

plugin/os-sdlc

label 2026-08-13 18:11:33 +00:00

jared commented 2026-08-13 18:11:43 +00:00

Author

Owner

Copy link

Still valid: plugins/os-sdlc/tests/ still flat with hardening_preflight_review_packet_test.rb and hardening_verdict_lint_project_test.rb present; no ADR for test layout found.

Still valid: plugins/os-sdlc/tests/ still flat with hardening_preflight_review_packet_test.rb and hardening_verdict_lint_project_test.rb present; no ADR for test layout found.

jared added the

frozen

label 2026-08-16 14:25:03 +00:00

jared commented 2026-08-16 14:26:01 +00:00

Author

Owner

Copy link

Frozen in the 2026-08-16 backlog reset — see #419 for the expiry procedure. Do not work unless a live run rediscovers this issue.

Frozen in the 2026-08-16 backlog reset — see #419 for the expiry procedure. Do not work unless a live run rediscovers this issue.

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

main

No results found.

Labels

Clear labels   

P0

Priority 0 — drop everything

  

P1

Priority 1 — next up

  

P2

Priority 2 — normal

  

P3

Priority 3 — someday

  

bug

Something is broken

  

create

Ticket creates something new

  

delete

Ticket removes something

  

enhancement

New feature or improvement

  

filed-by/agent

  

filed-by/user

  

frozen

Pre-reset backlog freeze 2026-08-16; do not work unless rediscovered by a live run

  

lint-rule

Ticket proposes/modifies a lint rule or cop

  

needs-info

Waiting on reporter for more information

  

needs-triage

Maintainer needs to evaluate this issue

  

next

Human-curated next up — AI never applies or removes

  

plugin/cc-architect

Scope: the cc-architect plugin

  

plugin/os

Scope: the os plugin

  

plugin/os-adr

Scope: the os-adr plugin

  

plugin/os-aidd-lint

Scope: the os-aidd-lint plugin

  

plugin/os-backlog

Scope: the os-backlog plugin

  

plugin/os-context

Scope: the os-context plugin

  

plugin/os-doc-hygiene

Scope: the os-doc-hygiene plugin

  

plugin/os-sdlc

Scope: the os-sdlc plugin

  

plugin/os-vault

Scope: the os-vault plugin

  

project/cc-os

Parent-scope: cc-os repo-wide, not specific to one plugin

  

ready-for-agent

Triage complete; an agent can pick this up

  

ready-for-human

Human-owned; never picked up autonomously

  

recurring

Recurring flavor: occurrence outcomes commented, never closed by AI

  

review

Shipped semi work awaiting human sign-off

  

update

Ticket updates something existing

  

waiting

Blocked; blocker named in a comment

  

wayfinder:grilling

  

wayfinder:map

  

wayfinder:map

wayfinder map ticket

  

wayfinder:research

  

wayfinder:task

  

wayfinder:task

wayfinder map child: task

  

wontfix

Will not be actioned

  

worklist/deviations

Deviation incidents from skill/pipeline runs — evidence pile for correcting the owning artifact

  

worklist/lint-rule

Proposed os-sdlc or os-aidd-lint rule

  

worklist/new-implement-build

Updated implementation harness (ast-grep + stepper)

No labels

P0

P1

P2

P3

bug

create

delete

enhancement

filed-by/agent

filed-by/user

frozen

lint-rule

needs-info

needs-triage

next

plugin/cc-architect

plugin/os

plugin/os-adr

plugin/os-aidd-lint

plugin/os-backlog

plugin/os-context

plugin/os-doc-hygiene

plugin/os-sdlc

plugin/os-vault

project/cc-os

ready-for-agent

ready-for-human

recurring

review

update

waiting

wayfinder:grilling

wayfinder:map

wayfinder:map

wayfinder:research

wayfinder:task

wayfinder:task

wontfix

worklist/deviations

worklist/lint-rule

worklist/new-implement-build

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

jared/cc-os#296

No description provided.