Repository navigation
test: move fuzz targets to packages that link no test tooling - #1381
Open
fuad-daoud wants to merge 19 commits into
Open
fuad-daoud wants to merge 19 commits into
fuad-daoud wants to merge 19 commits into
Conversation
go test -fuzz instruments everything a test binary links, and every execution clears, copies and scans the coverage counters of all of it. A binary links every test file's imports, so ir's fuzz targets were paying for golang.org/x/tools through internal/decls, and FuzzCompile for the end-to-end suite's test tooling. Coverage bookkeeping took 52% of a FuzzCanonicalWords worker's CPU and 33% of a FuzzCompile worker's. Move ir's five targets to ir/fuzz and the three that compile whole documents to compilers/openapi/e2e/fuzz, test-only packages that import no test tooling. The tables ir's targets share with table tests move to internal/ircases, which imports only the standard library, and the ref-shaped specs to internal/testspec, so a row added for one still reaches the other. FuzzCycleDetector now seeds from the whole fixture directory, which also stops a read error costing it its seeds in silence. CONTRIBUTING's rule that a test sits beside its code gains the exception and when it applies. Coverage counts a package's own tests, so nothing moved stands in for them: ir's statements stay covered with these seeds gone.
The conformance table moved to internal/ircases as Canonical, so the comment above describeNames named a symbol that no longer exists. This comment's change carries Omar's resolution in his merge b9cfbaa of main into his branch.
CLAUDE.md's testing rule asks for testify, which the fuzz packages exist to leave out, so it now names the exception. CONTRIBUTING names internal/testspec beside internal/ircases, since the end-to-end targets read their table from it. A narrower edit to docs/ir/naming.md and a different place for ircases' archtest entry keep clear of lines open pull requests also change.
docs/architecture.md's package tree had no entry for ir/fuzz, the end-to-end fuzz package or internal/ircases, and CONTRIBUTING still listed fuzzing among what goes in compilers/openapi/e2e.
internal/ircases and internal/testspec exported their tables as package-level slices, so any test could change a row every other test in its binary reads. CLAUDE.md bars mutable globals, and the styleguide bars handing out a shared slice (5.29) or mutable package state (5.30). No test-support package exported a variable before them. Each table is now a function returning a fresh slice, and a test per function holds that: it edits the rows it was handed and reads them again. A function is a statement coverage counts, so testspec gains its first test, and archtest's testSupport its entry, since only tests reach RefShapedData.
The checks it made through testify's assert became t.Fatalf when it moved, which the styleguide reserves for a failure that makes going on meaningless (6.14). Both checks end the input anyway, so t.Errorf reports the same.
ir/fuzz and compilers/openapi/e2e/fuzz sit apart from their code, assert without testify and read tables from a package not named for ir, against rules 6.20, 6.5 and 2.9, because a fuzz run pays for everything its binary links. Recording it keeps a style review from reporting it, and names #1315, the check that would hold the packages to linking no test tooling.
As a function returning a literal, RefShapedData ran to 78 lines, past the 70-line cap funlen holds production code to, and Canonical would cross it once its table grew. Each table is now an unexported variable, and the exported function returns slices.Clone of it: a fresh slice for every caller, as before, in a function that stays one line however many rows the table holds.
A fuzz run instruments everything its test binary links and pays for all of it on every execution, so a fuzz package that imports testify, go-cmp or internal/decls (which links golang.org/x/tools) searches at a fraction of its speed. Nothing held ir/fuzz and compilers/openapi/e2e/fuzz to the rule CONTRIBUTING states. TestFuzzPackages_LinkNoTestTooling refuses, in each directory named fuzz outside testdata that holds test files, an import that is not the standard library, a module package outside the top-level internal/ and cmd/ and not named in testSupport, or one of internal/ircases and internal/testspec. It also requires the walk to find both real fuzz packages, so a walk matching nothing cannot pass. Direct imports suffice: the pipeline packages the rule admits are held by their own rules entries to what they may link, and the two shared tables import next to nothing. CONTRIBUTING and the fuzz-packages-link-nothing departure now name the test, and the departure's probe runs it.
The fuzz-package rule's tests planted one import in one file in one package, so a walk that stopped after the first package, file or import stayed green. The new test plants the refused import late at every level: second in its file, in the second file of its package, in the second package. A new row witnesses the refusal of the top-level internal/ with a package that is not in testSupport, because the internal/decls row is refused earlier, by the testSupport check.
FuzzCycleDetector is seeded from every .yaml fixture in testdata/openapi and from the ref-shaped specs, not from a "cycle corpus", which stopped naming a set of files when the directory was widened. Its doc now says so. The two ir fuzz targets said the search runs nightly. On main that holds only once the scheduled workflow lands, so until then the sentence was false. Each doc now names scripts/fuzz.sh as what runs the search and claims no schedule, which is true whichever of the two changes lands first.
addYAMLDir reads only the .yaml files directly under testdata/openapi, so the three fixtures under testdata/openapi/nested_refs are not seeds. The previous commit's doc and body said the target was seeded from every fixture in testdata/openapi. This commit corrects the doc to say every .yaml file directly under it. No seed changes: the target still runs 79 seeds.
This was referenced Nov 2, 2026
The fuzz-package walk skipped only testdata, so a fuzz directory under a gitignored checkout such as .claude/refs or under a directory named with a leading underscore was judged although `go list ./...` never reaches it. Skip what every other module walk skips, through ignoredByGoTool, and plant a dot and an underscore directory beside the testdata row. Related to #846, whose walks this gives one more caller of ignoredByGoTool; the other walks are not changed here.
The check that a Go error comes back with no document was reached by no seed, although the doc says an empty source raises one: flipping it to `doc == nil` left every seed green. Seed the empty source.
The 2,213 go-cmp counters are the measurement of #1315, and both that figure and the 14,366 left are darwin/arm64's. Say so beside them.
The package comment of the OpenAPI fuzz targets said they import the compiler and the standard library, but they also import ir/irverify and internal/testspec. State only the negative the archtest rule holds. The two other comments counted the lists they sit beside; drop the count so they cannot go stale.
fuzzPackageViolations skips a directory the go tool ignores unless it is the walk's root. Deleting that exception left the package green, because every planted test's root was a plain temporary directory. The new test roots the walk at a directory named _root and requires that it still find the fuzz package beneath it. This matches what TestWalkGoFiles_RootIsNeverSkipped holds for walkGoFiles.
The departure's Why names darwin/arm64 for its counter figures, and neither #1301 nor #1315 records a platform. The check is on #1381: cross-built for darwin/arm64, ir's binary at 608a5a4 counts 64,256 and ir/fuzz's at 113c9e9 counts 14,366, as cited; linux/amd64 counts 64,701 and 15,112 at the same revisions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Moves the eight fuzz targets to packages that link no test tooling, so each execution of a search spends
its time on Morphic and not on coverage counters for test libraries, and holds those packages to it with
a check.
ir's five targets go toir/fuzz, and the three that compile whole documents go tocompilers/openapi/e2e/fuzz. The tables they share with table tests move tointernal/ircasesandinternal/testspec, each an unexported variable behind a function that hands out a copy.go test -fuzzinstruments everything a test binary links, and every execution clears, copies and scans allof those counters. Coverage bookkeeping took 52% of a
FuzzCanonicalWordsworker's CPU and 33% of aFuzzCompileworker's.This carries the target move from #1301 (Omar's), as its review on that pull request proposed: #1301's nightly search and its
-fuzzminimizetimechange follow as pull requests of their own.Measurements. CPU per execution, one worker, fixed executions, old and new binaries alternating, five
runs each from
608a5a46under heavy machine load (Omar's measurement):FuzzCanonicalWords_PropertiesFuzzHasCanonicalWords_AgreesWithCanonicalWordsFuzzNewBigVal_AcceptedFormsAreJSONValidFuzzParseDecimal_ReadsWhatNewBigValKeepsFuzzDecimal_OrdersAsRationalsDoFuzzLowerSchemaFuzzCompileFuzzCycleDetectorA re-measure of
FuzzNewBigVal_AcceptedFormsAreJSONValidon the carried branch gave 233 µs to 86 µs, 63%saved.
FuzzCycleDetector's two sides start from different seeds, so its row is the least exact.Seeds.
FuzzCycleDetectornow reads every.yamlfile directly undertestdata/openapi(66) and the ref-shapedspecs, plus the empty source: 80 seeds against 44 on
main, and a read error no longer costs it its seeds in silence. Its doc andthe two moved
irtargets' docs say what seeds them and thatscripts/fuzz.shruns the search. They claimno schedule, which holds whether or not #1273 has landed.
Plants and checks.
TestFuzzPackages_LinkNoTestToolingrefuses, in eachfuzzdirectory outsidetestdata, an import that is not the standard library, a module package outside the top-levelinternal/and
cmd/and not named intestSupport, or one ofinternal/ircasesandinternal/testspec. It requiresthe walk to find both real fuzz packages. Its plants place a refused import second in its file, in the second
file of its package and in the second package, so a walk that stops early stays red. A row witnesses the
refusal of a top-level
internal/package outsidetestSupport.Authorship. Omar's commits are carried with him as author.
git range-diffagainst the originals gives=for74db1783,dbb090dc,7981fd70ande982961f. It gives!for113c9e94,345c31ccandf7fe3210, which differ only wheremainmoved: theIRVersionsentence indocs/ir/naming.mdand onehunk of
docs/architecture.md. His mergeb9cfbaaeis split. Itsir/irversion_names_test.goresolutionis its own commit with him as author (
git diff e982961f75 HEAD -- ir/irversion_names_test.gois empty),and its
docs/ir/naming.mdresolution is redone inside the conflict of113c9e94.Crossings.
changes only the example path. Whichever lands second keeps the other's text with this path.
irtargets' doc paragraphs, with the same ci: move the fuzz search from pull requests to a nightly run #1273 pull request: both write the same lines, so thesecond merge resolves to the same text.
CLAUDE.md'sinternal/list: test: give every covered child its own coverage directory #1233 addsexectestto the line this PR rewraps forircases, andwhichever lands second keeps both.
Closes #1315
Related: #1301, #1273, #1268
Before and after
flowchart LR subgraph before["Before"] direction TB b1["fuzz targets in ir and compilers/openapi/e2e"] --> b2["their test binaries link testify, go-cmp, decls, x/tools"] b2 --> b3["every execution clears, copies and scans<br/>64,256 coverage counters in ir's binary at 608a5a46, darwin/arm64"] end subgraph after["After"] direction TB a1["fuzz targets in ir/fuzz and e2e/fuzz"] --> a2["they link the code under test,<br/>internal/ircases and internal/testspec only"] a2 --> a3["14,366 counters in ir/fuzz's binary at 608a5a46, darwin/arm64<br/>25–73% less CPU per execution"] a4["TestFuzzPackages_LinkNoTestTooling"] -. "refuses test tooling in a fuzz package" .-> a2 end before ~~~ afterWhat it brings: a fuzz search spends its executions on Morphic's code rather than on coverage counters for test libraries, about 25–73% less CPU per execution, and a check keeps it that way.
Test plan
make gatepasses on the branch beforemainwas merged (182ae4ca), with the coverage gate at all 17,377 statements andfuzz.shsearching all 15 targets, the moved ones in their new packages included. On this head, withmainat3188471dmerged in,go build ./...,go vetover the touched packages, andgo testoverinternal/archtest,ir,ir/fuzz,compilers/openapi/e2e,compilers/openapi/e2e/fuzz,internal/ircasesandinternal/testspecpass. CI on this head runs the rest.func Fuzz…declarations onmainand on this branch is identical (15).ir.CanonicalWordsreddensFuzzCanonicalWords_Properties; anirverifythat reports on every document reddensFuzzCompileandFuzzLowerSchema; aCompilereturning no document and no reason reddensFuzzCycleDetector. Each from the seeds alone.irstays at 100% without its fuzz seeds:Coverage gate passed: all 1611 statements covered.ircases/testspecfunction made to return its variable without the clone fails its…_IsFreshOnEachCalltest; every row matches the table onmainit came from, the ref-shaped specs byte for byte.testifyimported inir/fuzzandir/irtestimported incompilers/openapi/e2e/fuzzeach reddenTestFuzzPackages_LinkNoTestTooling. Its planted-tree cases refusetestify,ir/irtest,internal/declsandinternal/census, and admitinternal/ircasesandir. A walk that judges only a file's first import, or only a package's first file, failsTestFuzzPackages_JudgeEveryImportOfEveryFileOfEveryPackage; dropping the top-levelinternal/refusal fails theinternal/censusrow. The walk skips what the go tool skips (ignoredByGoTool), as archtest's other module walks do: afuzzdirectory under.claude/or_old/is not judged (TestFuzzPackages_OnlyFuzzDirectoriesTheGoToolReadsAreFound), and a root whose own name the go tool skips is still read (TestFuzzPackages_RootIsNeverSkipped).FuzzCycleDetector's doc names what seeds it, 80 seeds against 44 onmain, the empty source among them, which is the one seed that reaches its Go-error check (flipping that check reddens it); a sweep finds no old table or file name and no "nightly" claim;TestDocCommentspasses.go tool nm(internal/fuzz._ecounters−_counters) ongo test -c -fuzz=.gives 64,256 forirat608a5a46and 14,366 forir/fuzzat113c9e94, as cited; linux/amd64 gives 64,701 and 15,112 at the same revisions. The 2,213 go-cmp adds reproduces on both.