[OmarAlJarrah]

Summary

Test files were hard to navigate after #1109's move, for reasons a scan of the whole suite at main (ff3fff4) pinned down:

• 51 source files had two test files, x_test.go outside the package and x_internal_test.go inside it;
• 99 tests sat beside a different file from the one their name and subject belong to;
• eight test files ran past 1,500 lines, schema/allof_test.go to 3,078;
• helpers were written more than once, schema's lowering fixture among them, byte for byte on both sides of the package boundary.

This PR fixes each of them and writes the rules down. Every change is to test code, except the shared helpers added to the two test-helper packages, ir/irtest and compilers/openapi/internal/openapitest.

• Internals through export_test.go. 38 in-package test files in 19 packages become external tests. Each takes its source's x_test.go, merged into that file where one already existed, and reaches what it needs through the package's export_test.go. The tests are the same code, qualified by their package. Where an alias's exported name is taken, its name says what it adds, as irtest.CompareGoldenOn (takes the stub testingT) and atomicfile.ReplaceWithChmod do. A fixture both sides of a package use is declared once there, as nodeview's YAMLDoc and reader's ResolvedDir are. auth's nil-requirement test is rewritten instead: it now goes through LowerSecurityRequirements, the exported function, and still catches the two faults the original caught.

• _internal names. Nine in-package test files that sat beside external tests under plain names take the _internal_test.go spelling. The lint exclusion and three comments naming them follow.

• One schema fixture. The Lowerer fixture is declared once, exported, in schema/export_test.go, and both sides of the package use it. All 459 uses were rewritten from type information, not spelling. operation's external lowerServiceSpec, which compiles a whole spec, becomes compileService, so it no longer shares a name with the in-package helper that drives LowerService.

• Shared helpers. Comparing every test helper's body, whatever its own name and the names of its parameters and locals, found the copies. Each helper with a home now has one:

  • within a package: e2e's compileClean, which replaces two compile helpers; irverify's violationCodes and violationPaths; reader's resolved-directory fixture; and e2e's componentID, which it also wrote as namedID;
  • in openapitest, for the compiler's packages: SchemaFromYAML, ParamBySource, FirstDiag, PointersOf, PreservationClaims, HolderSpec, ClearAnchors and YAMLDocument, which five packages had each written;
  • in irtest, for the IR's: RepoRoot, NestedOverloadDoc, NestedListValue, DocOf, ModelOf, ScalarOf, RefsTo, Encoded, RefusedAsCyclic and DiagCodes.

  Two wrappers of openapitest.AssertHasCode become calls to it, four helpers that took a BigVal's address become new(ir.BigVal(v)), and the CLI tests stop writing out warnSpec a second time. RepoRoot and Encoded take their failures through an internal function, so those are tested too.

• Restated bounds. Six tests in schema and operation spelled a bound's value, or what follows from it, instead of reading the bound, two of them with a comment that the bound was unexported. Each bound is now aliased in its package's export_test.go, as resolve.MaxRefChain already was, and the tests derive their chain lengths, components and expected messages from it, so a changed bound leaves them testing its new boundary.

• Misfiled tests. 99 tests move to the test file of the file declaring their subject, with any helper only they use. Among them: load's 25 hold tests go to external_hold_internal_test.go, value's NumericLiteral tests to scalar_test.go, pass's collectTypeIDs tests to validate_graphql_test.go, and ir's TypeCommon and template tests to typedef_test.go.

• Large files split by topic. The eight files over 1,500 lines split into x_<topic>_test.go files, which sort beside their source. For example, allof becomes allof_nesting, allof_required, allof_residue, allof_conflicts and allof_narrowing. Every test file is now under 1,250 lines.

• t.Helper in conformance assertions. Ten conformance case bodies called t.Helper, so their failures were reported at TestConformance's loop line; they no longer do. Eleven sub-assertions called by other assertions now do.

• CONTRIBUTING gains three rules under "Where a test goes", each saying what fails when it is missed:

  • a test is external unless it can't be: a package's tests stay inside when moving them out would take more than ten aliases, and a cmd program's are all inside it, since a main package cannot be imported;
  • a test is named for what it tests, or for the construct it pins, and a function a table runs as a case's body leaves t.Helper out;
  • a helper is written once.

  The root guidance's testing summary points at the section.

Deliberately left:

• 43 inside/outside pairs remain. Each needs an unexported field or method, a generic or a variable it assigns, none of which an alias carries faithfully. Otherwise it sits in schema, operation, bind, irverify, harness or overlay, whose tests would need 16 to 51 aliases each, enough to make a second API. Two of the 43 are new, in nodeview and ir's typedef: tests moved to the file declaring their subject, beside tests of that file that must stay inside.
• Packages whose tests all stay inside. load, merge, annotation and scan would need 14 to 106 aliases each.
• Messages that spell a bound. Tests that pin a diagnostic's exact text keep the number it names, such as the 64 levels a merge chain is expanded to. A changed bound fails them loudly rather than letting them pass beside it, so they stay as written.
• Bounds whose exact boundary no test pins. Planting an off-by-one at each of the 38 comparisons against a named bound leaves the suite green at 18 of them. That predates this change, does not depend on where any test sits, and is test: no test pins the exact boundary of 18 named bounds #1111.
• Entry-point tests stay filed by subject. Tests named for a package's entry point or for a construct are filed by the subject they exercise through it: the CLI's TestRun_ tests, load's TestResolve_, operation's TestContent_ and TestOperations_, and schema's TestLower_. CONTRIBUTING names that convention.
• Helpers still written in more than one package, as the helper rule now names them:
  • ones that belong in a shared home but cannot live there, each copy saying why: parseFull and compileNullable compile through openapi.New, which openapitest cannot import, and componentID spells an ID the idderivations rule allows only in a test file;
  • ones outside what openapitest and irtest serve: file reads and writes, AST predicates, package loading and stub compilers;
  • one-expression shorthands such as ir.TypeRef{Target: id} or typeByName, which hold no behaviour for a copy to drift from.

Test plan

• Test names, go test -list '.*' ./...: 3,478 on main, 3,501 here. Twenty-three are new, the tests of the shared helpers. One is renamed: TestLowerSecurityRequirement_Nil is now TestLowerSecurityRequirements_DropsANilEntrySilently.
• Full suite under go test -json -race ./...: 12,271 passing tests and subtests on main, fuzz seeds included, against 12,294 here, differing only in those 24 names. Four TestLoad_AFileURIReadsTheFileItNames subtests are named after a random temp directory, so they differ in name only.
• go run ./cmd/morphic-covergate over that run's profile: all 15,233 statements covered. The 64 new ones are the shared helpers'.
• Per package, each move, split and rewrite left the package's tests and subtests identical to main's, compared with go test -json before and after.
• Declaration dumps (every top-level declaration with its comment) are byte-identical before and after for the splits and the moves. The exceptions are the references qualified by a move out of the package, a comment that no longer calls the visibility cases "below", and one that no longer calls the round-trip tests "above".
• Planted faults:
  • the rewritten auth test fails on a nil entry kept as an empty option, and on a nil entry reported;
  • a failure planted in assertSelfURI was reported at conformance_test.go:51 before the t.Helper change and is now reported at the line that fails;
  • planting a restated bound at another value leaves its tests passing, and planting its comparison one off fails them;
  • a fault planted in each shared helper fails that helper's test. Three at first did not, and their tests were fixed: DiagCodes' test used a list that reads the same reversed, no fixture reached SchemaFromYAML's validation check, and nothing counted encoded's and rootFrom's Helper calls.
• make fmt build vet lint lint-planted fuzz bench-smoke exit 0, with golangci-lint at the pinned version and FUZZTIME=2s.
• go test -shuffle=on ./... passes in two runs, so no moved test leans on another's order.
• Each commit's tree vets and passes the tests of the packages it touches on its own.
• A trial merge with the one open PR, chore: add a review agent built from past PR fixes #888, is clean.