Check that prose names identifiers that exist - #10
Merged
Merged
Conversation
A throwaway repository inherits the developer's global core.hooksPath, so a hook installed for real work also runs against fixtures. One that refuses a commit leaves a fixture half-built: the file staged but unrecorded, and a later checkout then carries it onto the branch under test. That is how a passing suite turned red locally while CI stayed green — the hook does not exist there. Repo now points core.hooksPath at a path with no hooks in it, for the same reason it already sets the identity per repository rather than reading it from the machine. A fixture must not inherit the machine. The symptom was TestDiffMeasuresFromTheMergeBase crediting another branch's file to the session, because diffIncludingNewFiles counts staged and untracked files by design.
It asserted zero collisions in 2000 draws over a space of 50 adjectives, 50 animals and 32^4 suffixes. That is 2.6 billion names, so the birthday chance of one collision is about 1 in 1300 — and a test that fails that often teaches a reader to re-run a red suite rather than read it. One is now tolerated, two is not. The chance of a second is about 1 in 3 million, while a generator that had lost its suffix would draw from 2500 names and collide about 620 times here. Measured, not estimated: breaking suffix produces 615. The arithmetic is in the comment, because the next person to see one collision should be able to tell luck from a broken generator without redoing it.
An identifier spelled wrong in code fails the build. The same mistake in a doc comment, a README table or an architecture note passes every gate and is read as fact by the next person. Prose is the one place here with no compiler, which is where the defects were collecting. scanProse walks the tree, reads names out of the syntax tree rather than the file text, and reports any camelCase word in a comment or a markdown code span that is not an identifier anywhere in the module. Reading the AST is what stops a misspelling written twice from vouching for itself. Three rules keep it quiet enough to survive. Every capital must be followed by lowercase, so an acronym is not mistaken for a name. A word that prefixes a real identifier is naming a family. A word a file only writes into a string literal is accepted in that file alone. It starts green on 178 names and takes a fixture rather than a hand check: TestProseScanReportsAPlantedMisspelling runs the real scanner over a temporary package and document, so the discrimination is proved on every run instead of once by somebody who remembered to try it. The fixture is written to a temp directory on purpose. Committing one would put the misspelling into this module's own sources, where it would resolve. scripts/commit-msg does the same for commit messages, which no test can read. It leaves quietly in a repository with no Go files, so it is safe in a shared core.hooksPath, and it warns rather than blocks: a message may name a symbol the commit deletes. What it cannot judge is whether a claim about a real symbol is true. That half stays a reading job.
It asserted the job was still running immediately after Analyse returned, while the stub reviewer returned instantly and runAnalysis ran in a goroutine. Which of the two got there first was up to the scheduler, so the test failed under -race on a loaded runner and passed everywhere else. The reviewer is now held open until the assertions are done, through analysableWith, the same shape TestSidebarShowsWhatAnalysesCost already uses. "Still running" becomes a fact the test arranges rather than one it hopes for. The timing assertion above it is untouched, because that is the real subject: Analyse must hand back a handle rather than block for the length of a review. Not reproduced locally — this machine schedules it the winning way every time across ten runs. The fix removes the race by construction, and CI is what demonstrates it.
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
1 participant
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.
Three commits, each green on its own.
core.hooksPath, so a hook that refuses a commit left a fixture half-built andTestDiffMeasuresFromTheMergeBasefailed locally while CI stayed green. Fixtures already isolate identity from the machine for the same reason; hooks were missed. This lands first so every later commit is green everywhere.TestSessionIsUniquetolerate one collision — it asserted zero over a 2.6 billion name space, which fails by chance about 1 in 1300 runs. One is tolerated, two is not; a generator that lost its suffix collides about 620 times, measured at 615.scanProsereports any camelCase word in a Go comment or a markdown code span that is not an identifier anywhere in the module. Starts green on 178 names.scripts/commit-msgdoes the same for commit messages, which no test can read.Why: an identifier spelled wrong in code fails the build, and the same mistake in prose passes every gate. This session produced several, including a commit message naming a function that does not exist.
Discrimination is proved by a fixture, not by hand —
TestProseScanReportsAPlantedMisspellingruns the real scanner over a temporary package and document, so breaking either half fails the suite.What it cannot judge is whether a claim about a real symbol is true. That stays a reading job.