Skip to content

storage tests: set a decomposition standard for the oversized system-record test files (next-state is 1695 lines) #2250

Description

@Jurij89

Follow-up from the review of #2243. Test-only decomposition; no behavioural change. Filed rather than taken in that PR, for the reasons below.

What

packages/storage/test/system-record-next-state-v1.test.ts grew from 939 lines at the integration base to 1695 across the D5b stack. It now holds active derivation tests, terminal derivation tests, fork-resolution fixtures, replay helpers and long explanatory comments in one file.

Suggested direction, recorded from the review: split the terminal/quarantine derivation specs into focused test files and move the fork/quarantine construction helpers into packages/storage/test/helpers, keeping the same assertions and fixtures.

Context worth having before anyone acts on this

The growth is real and the numbers above are measured. But the "1k-line boundary" is a heuristic rather than a convention this package currently holds — six of the 70 storage test files exceed 1000 lines, and the largest, system-record-materializer-lifecycle-v1.test.ts at 3031 lines, is nearly double this one and predates this stack:

3031  system-record-materializer-lifecycle-v1.test.ts
1695  system-record-next-state-v1.test.ts
1449  graph-set-index-store.test.ts
1214  system-record-atomic-apply-executor-v1.test.ts
1115  store-scheduler-system-record-admission.test.ts
1106  sparql-http.test.ts

So applying a per-file limit to whichever file a PR happened to touch is arbitrary. If we want the boundary, it should be set once as a package-wide standard with the other five in scope; if we do not, this file is the second-largest of a set where large is currently normal. That decision is the actual work here, and it is larger than one file.

Why it is not in #2243

  • It is test-only, with zero behavioural effect, in a fourteen-commit stack at convergence with a full green CI run. Re-earning that for a file reorganisation with no behaviour change is a poor trade.
  • It pairs naturally with storage: decompose system-record next-state derivation into normalized transition descriptor + shared plan builder #2244, which decomposes the production next-state module the reviewer notes it mirrors. Splitting the tests along one boundary and the module along another, in separate changes, is how the two drift apart. Doing them together, on a quiet base, is both cheaper and more likely to produce a coherent split.

Unlike #2244, the evidence-chain argument is weak here and should not be used to justify the deferral: the source comments that cite tests do so by test name, and names survive a file move. This is a scheduling and standard-setting decision, not a risk one.

Related: #2052 (umbrella), #2243, #2244.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions