Skip to content

Five incompatible private worksheet fixtures and 29 inline DTO literals #103

Description

@Artmann

What's wrong

There is no shared DTO fixture builder, so the tests carry 29 full literals and five mutually incompatible private builders.

grep "sortOrder: " across test files returns 29 hits, spread over ~15 files. WorksheetDto (schemas.ts:195-203) has 7 required fields; DatabaseDto (:149-159) has 6 including a nested connectionInfo, which is copy-pasted verbatim across 7 files purely so the literal type-checks.

Five suites independently grew a private builder, each with a different positional signature over the same DTO:

file signature
worksheet-naming.test.ts:7 worksheet(name)
worksheet-selection.test.ts:11 createWorksheet(id, lastOpenedAt)
worksheet-editor-content.test.ts:6 worksheet(id, content)
WorksheetTabs.test.tsx:20 worksheet(id, name)
database-explorer-search.test.ts:29 makeDatabase(name)

Each encodes exactly the one axis its own suite varies, which is precisely why none could be reused by the next — the classic symptom of a missing overrides-based builder.

They have already drifted: sortOrder is 0 in three files and null in the rest; databaseId is '' in one and null elsewhere.

Proposed change

A new src/app/test-fixtures.ts — sibling to the existing test-utils.tsx / test-fetch.ts, matching that directory's test-* naming — exporting:

makeWorksheet(overrides: Partial<WorksheetDto> = {})
makeDatabase(overrides: Partial<DatabaseDto> = {})

The axis a test varies becomes the only thing it writes, every state the five positional builders express is reachable plus every state they cannot, and a DTO field addition is a one-line change in one file instead of 29 edits across 15.

Scope

One new ~30-line file with no dependency on test-utils.tsx.

Migrate the five files that already have a private builder first — that alone proves the shape fits five genuinely different use patterns — and let the other ten migrate opportunistically.

Leave makeColumn/makeTable/makeSchema (database-explorer-search.test.ts:7,18,39) local; they serve one file.

Explicitly does not touch src/app/test-utils.tsx. createTabsState (:90-102) and renderWithProviders (:137) keep their exact signatures, so other pending changes citing them are unaffected. The only interaction is passing makeWorksheet(...) where a literal used to go into the same RenderOptions.worksheets array.

Risks

Real risk of hiding a dependency — a test passing because sortOrder happened to be 0 would silently take the builder's default.

Mitigate by choosing defaults matching the majority literal and passing the minority values explicitly during migration. Ordering-sensitive suites (worksheet-selection.test.ts sorts on lastOpenedAt) must keep those values explicit, which is the point: after migration the sort key is the only thing visible in the fixture.

Validation

Migration is behaviour-preserving, so the bar is all 15 suites green with no assertion edits.

Split into two commits (builders + the five private-builder files, then the rest) so a failure localises. No test for the builders themselves — a fixture builder with no branching is verified by its consumers.


Found in a codebase-wide simplification audit (F-S30-b). Confidence: medium-high. The duplication and the five divergent signatures are objectively verifiable; the judgement call is whether a shared test builder crosses the "don't over-abstract test code" line. It does clear the bar — five independently-invented incompatible builders is the strongest possible evidence of a missing shared model — but it is test-only ergonomics with no production behaviour at stake, so it ranks low against everything else.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions