Skip to content

validate.ts has no tests, and its structure makes it untestable #645

Description

@jeffreylouden

scripts/validate.ts is 2,079 lines and has zero tests. It is the gate every data change passes:

  • .husky/pre-commitpnpm validate && pnpm exec lint-staged
  • .github/workflows/validate.yml:65pnpm validate
  • .github/workflows/release.yml:41pnpm validate

There are 12 test files in scripts/__tests__/. None of them touch it.

Why it is untestable today, not just untested

$ grep -c '^export' scripts/validate.ts
0
$ grep -n 'import.meta.url\|require.main' scripts/validate.ts
(nothing)

Nothing is exported, and the file runs on import — the tail is bare top-level statements ending in process.exit(isValid ? 0 : 1). A test that so much as imports the module walks the whole data/ tree and then kills the vitest worker. So this is not "someone should sit down and write tests"; the file has to be opened up first.

Suggested shape

scripts/build-catalog-index.ts already solves this in-repo and has a test file:

// build-catalog-index.ts:332
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {

Same treatment here: a main-guard around the tail, and export on the units worth pinning. The pure ones are the cheap win, since they need no fixture tree:

Function Line What it decides
isValidCategory 83 whether a category value passes
getErrorCodeFromZodIssue 124 which E1NN code an author sees
validateMarkdown 224 unclosed fence / inline-code detection
normalizeToMarkdownString 274 string[] → joined markdown
detectSupersedeCycle 1079 lineage cycle detection
collectWarnings 1126 the whole W1NN warning set

validateFile (830) and validate (1415) need a fixture directory rather than the live data/, which is a bigger change — worth splitting out separately if it makes this too large.

Things worth pinning specifically

detectSupersedeCycle and collectWarnings are the two where a silent regression costs the most: a cycle that slips through corrupts lineage in the built database, and a warning that stops firing just quietly stops being emitted — nothing turns red, so nobody finds out.

validateMarkdown's backtick-parity pass (224–270) is the kind of hand-rolled scanner that deserves characterization tests before anyone edits it again. Two behaviours I'd want written down as tests before changing anything, whichever way they're decided:

  • the fence count (content.match(/```/g) || []).length reads six consecutive backticks as two fences
  • a prose line containing one backtick (Use the ` key) is reported as unclosed inline code

Both may well be fine as-is. The point is that right now nothing says which behaviour is intended, so a future edit can't tell a fix from a regression.

Scope note

Characterization first — encode what it does today, do not change behaviour in the same PR. This script gates every contributor's commit, so a test PR that also "fixes" a validation rule is very hard to review and very easy to get wrong.

Related: #644.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementImprovement to existing itemsscriptsChanges to build/validation scripts

    Type

    No type

    Projects

    Status
    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions