Close the mutation and condition gates on sixteen audit commands - #884
Merged
Merged
Conversation
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. Warning Review limit reachedNext included review available in 17 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: jmrplens/gitlab-mcp-server/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: jmrplens/gitlab-mcp-server/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (22)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe pull request expands regression coverage across repository auditors, tightens action adjudication, separates command execution seams, improves report validation, and tests filesystem, parsing, ordering, stream, and exit-code edge cases. ChangesAudit report contracts
Audit surface regressions
Priority: ➖ Normal Estimated code review effort: 5 (Critical) | ~120 minutes Change: Other 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description gives a clear and relevant summary, but it does not follow the required template. It omits the Related Issue, Type of Change, Changes Made, How to Test, Breaking Changes / Migration Notes, and Checklist sections. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
security
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @jmrplens, your pull request is larger than the review limit of 150,000 diff characters
jmrplens
added this pull request to stack #882
September 21, 2026 15:40
jmrplens
changed the title
jmrplens
force-pushed
the
sweep-cmd-audits
branch
from
September 21, 2026 15:52
8c8a2e4 to
7e016a2
Compare
…ion flag to the fetcher it builds The command's own package had three straight-line crossings no gate can see and one case that claimed a report nothing read back. The merged backlog reaches one function as four byte slices of one type, and each of them decodes into a report that ignores every key it does not name, so a crossed pair produces a backlog with both streams absent rather than a parse failure. The only assertion over that path checked that the summary carried each key, which it does whatever the counts are. Three seamed streams now arrive with figures no sibling shares, and each is read back under the key that names its own stream. -refresh and -offline are two booleans of one options struct, and the two places that build a fetcher from them were told apart by nothing: every case asserted a fetcher was present and none asserted what it was built to do. The endpoint comparison's fetcher is now asked for an area nothing cached, where offline must refuse rather than reach for the network, and the validation mode is driven over a planted root with no cache at all so the reason its report gives is the offline one. The gate that writes its report before refusing is held to that order too: the two gating scopes have their report read from disk after the refusal, and the analyzer failures beside them are held to producing none. Two corrections came with them. The case named for writing its report asserted nothing but the absence of a fatal message, and now reads the report back; and the enum scope's closing line asserted that enums.Report is enums.Report, which is true of every type and was the only reason that package was imported here.
… from the mode refresh and offline are crossable twice on the way to the fetcher the validation mode builds: in the argument list main passes and in the options literal the mode builds from it. A test that calls the mode directly holds the second and says nothing about the first, and either crossing leaves the citation reported stale all the same, for a different reason and after six attempts at gitlab.com. The case therefore starts at the flag set, and asserts the reason rather than the staleness: over a planted root whose one citation is cached nowhere, the report must say the doc is not cached and offline.
…alk to its own edge The R-ENUM rule's summary reports eight counters, and every case here left five of them reading 1: the packages, the fields with a gap, the unsurfaced output fields, the missing values and the extra values. Under that arrangement the condition deciding which output fields surface no value set could be filled from the ones that surface one and still read right, which is why it survived every mutation run the package has had. One case now arranges an offer in which no two counters agree (1, 2, 4, 3, 0, 5, 6 and 7) and asserts the summary whole, so a field tallied under its neighbor fails rather than coincides. The seven exemptions it leaves stale are stale for seven different reasons, from a package the fixture does not have to a field-level key on a field with no gap. The package list was never held to anything either, because every report built here has one package in it: an entry named by the import path it was found under and a list sorted backwards both read the same. A second tool package in the fixture pins the short name and the order. The rest is the edges the fixture could not reach. A constant client-go declares whose type another package owns has an integer kind like any value set, so only the owning-package test keeps it out; the fixture now declares one. An untagged embed of a named string has no fields to flatten, so the walk falls through to the tag test instead of recursing, which no embed in the tree does. And findingLess is asked about two findings equal in every key, which is the only tie sort.Slice can hand it: the two keys above the field are each guarded by a difference. One fixture correction came with them: ListWidgetsOptions is sent as a query string, so its url tag is the name GitLab receives, and Sizes spelled both tags the same way, which made the precedence unobservable. It spells them differently now, so preferring the json tag drops the field from the rule instead of passing.
…s-only flag The metadata scope had two conditions nothing evaluated both ways and four values no fixture could tell apart. A spec whose alias list holds only blank strings is as bare as one holding only its own two names, and no fixture had one, so the branch that skips an empty alias was never taken. A package whose findings already arrive in action order is the case the sort has to leave alone, and every fixture arrived reversed, so the comparator never answered false. Both are planted now, and the condition gate reads 40/40. The counters were the invisible half. Three of the four flag tallies read 1 and the empty_related tally equalled the package count, so any pair of them could have traded places with every assertion still holding; each now arrives with a number no sibling shares. The report's own header is held to the packages it summarizes, which is 164 against 1082 on the real catalog. The grouping fixture claimed three tiers of owner precedence and reached two: both specs of the group that declares an owner overrode it, so the base domain would have served them equally and a swap of those two tiers changed nothing. grouped.a is the spec that reaches the middle tier. The per-package assertions became a table of whole records, which also keeps the helper under the cyclomatic bound the new case pushed it over. Last, gaps-only is one boolean forwarded from the command through buildReport into the grouping, which no gate reads: the full report now has to keep packages the gaps-only one drops, so a forward that inverted or dropped the flag fails. The schema version is compared against shared.SchemaVersion, as the four sibling scopes already do, rather than against a literal 1. Two mutants survive, both the `<` of a sort comparator turning into `<=`. The package sort reads keys taken from a map, so two entries can never tie; a tie in the findings sort would be two findings naming one action, which the canonical catalog cannot produce, and both orders satisfy the only promise an unstable sort makes. Killing either would mean asserting the standard library's behaviour for equal elements rather than a property of this code.
The adjudication table makes two claims and only one had a test. The first, that an entry answers a method no handler calls, was covered; the second, that an entry whose method a handler does call is itself a finding, was computed and printed and asserted nowhere. Neither gate can see the difference: reading the interface's declared methods instead of the ones a handler calls leaves every synthetic case green, because a synthetic table names methods that are both, and turns all 141 live adjudications on the real table into findings. Two tests now hold it, one over a table the test writes and one over the committed table, and the second is the one that fails under that substitution. That second test also names what the report had been saying unread: Integrations.ListActiveGroupIntegrations was adjudicated as reached through the generic slug dispatcher while internal/tools/integrations calls the wrapper directly, so the entry claimed a route that is not the one taken. Removing it changes no coverage figure, since the direct call already counted the method as covered, and empties stale_acceptances. Beside those: the -gaps-only filter is held to both halves and to summarizing what each report carries rather than what it filtered away; the summary table gives every counter a value no sibling shares, so no two accumulators can be crossed unnoticed; the stale key is pinned to the bare service name, which is not the spelling the report's own service field carries; and Run's layout is pinned to two spaces per level with no line prefix, since swapping MarshalIndent's two arguments still yields valid JSON ending in a closing brace and every assertion that only parses the result is blind to it. One survivor is recorded rather than killed: the boundary of the sort comparator at analyze.go:81. CollectServiceUsage keys its map on the same interface name the Service field carries, so two entries can never tie and < is never asked about equals. The strictly increasing assertion states that uniqueness, which is the property behind the branch. Mutants 8 killed, 1 lived, 7 timed out before; 15 killed, 1 lived, 0 timed out after. The timeouts were the 30s default budget rather than a hang: the suite loads the 180 tool packages through go/packages, which costs about 19s cold, so the measurement was taken at MUTANT_BUDGET=240. Conditions were 20/20 before and after.
…lishes
The mutation and condition sweep of this command. Both gates were red on the
same thing: nothing drove the command at all, so the four stages that can fail
between the flags and the written backlog, the -check gate's two answers and
the -gaps-only filter were reachable only from a process.
main is now the one line that spends an exit code runMain returns, the shape
the other twenty commands under cmd/ already use, with the flag set moved off
the package-level ExitOnError one so a bad flag is a code rather than an
os.Exit no test can see. loadActionCatalog is the second seam: the catalog is
built from specs compiled into this binary and cannot fail from anything the
command accepts, so the branch reporting that failure is only reachable from a
stub.
What the tests now hold, beyond reaching those lines:
- check() interpolates six numbers positionally, so the populated case gives
each a value no other carries and asserts the whole sentence. With the
totals all equal, two of them exchanged in the format call read the same.
- parseDomainsTable assigns four capture groups of one regex, and every row
in the real README writes its document link's text and target as the same
string, so nothing in the tree could tell those two groups apart. The
fixture now carries a row whose link text is not its target and compares
the whole row slice.
- doc-ownership.json's two routings, prefix-over-group and the group claims
the README's Meta-tool column truncates away, had no synthetic fixture at
all: only the live baseline exercised them, and it asserts nothing that a
collapsed routing would change. Four tools sharing one naming stem, three
of them sharing one owning group, now pin which file each belongs to.
- the backlog's own object keys are read as plain JSON as well as parsed
back into the type that wrote it, which agrees with itself whatever its
json tags spell, and the Domains table lists its rows out of alphabetical
order so the recorded order is the README's.
Three non-test edits, each deleting something no test could observe rather
than rearranging code. The catalog walk's collision guard checked a stored
group name for emptiness that AddGroup refuses to store; relativeDocPath
checked filepath.Base for an empty string it never returns; and the comment
over the first of those described an overwriting rule the code has never had.
mutants Killed 95, Lived 0, Not covered 22 -> Killed 100, Lived 0,
Not covered 16
conditions 197/222 -> 224/226
The sixteen that remain are the two documented tool artifacts, each verified
by hand: seven mutate a + between string literals in a package-level var into
a -, which does not compile, and nine sit in switch case expressions, which
carry no statement counter; applying each of those nine fails the suite. The
two conditions are the error arm of a call that cannot fail for the value in
hand, json.MarshalIndent of a report and the catalog build, both of whose
inversions the mutation run kills.
…he walk's own failures The documentation gate's header prints three counts of the same kind on one line, and nothing read them apart: transposing the registered-name count and the catalog's left every assertion green. The summary is now pinned to a fixture where no two of the three can take the same value. The stale-declaration half of the allowedIDs table was reachable only through a run over every documentation root, so `run` was never seen asking for it: an entry that excuses nothing now has to suppress the all-clear, reach the error line and set the exit code. The report's nearest-ID suggestion was never rendered at all, and it is the row a reader acts on differently from an alias's, which must carry no suggestion because the link it names already works. Both sorted reports are held to printing the same bytes whichever order the map hands the findings over in, which is also what reaches each comparator from both sides. The walk gets the two arms it never had: a directory pruned because it is a dot-directory or another checkout, and a subtree the walk cannot list, which arrives as the walk's own error rather than through scanFile. The second skips where the process may read a closed directory, as it does elsewhere here. And every declared documentation root is held to existing, since scanRoot passes over one that is not there and the run still prints an all-clear.
…r-flag tally and the filters no live catalog reaches
The command line main assembles was driven by nothing, so the exit code
this audit answers a caller with was the one thing about it no test read.
It is driven now on three arguments: the writing path leaves a parsable
report where -output names it, -check leaves none and passes while
nothing reaches the threshold, and -check -severity=info exits 1 naming
the count. The passing half skips rather than fails if the catalog ever
carries an error-severity finding, because demanding one would make this
a gate on the whole catalog that nothing asked it to be.
The summary's eleven per-flag counters were asserted with one finding
each, so every counter read 1 and a counter wired to its neighbor's case
was invisible; each flag is now carried by a different number of
findings and the whole summary is compared at once. The -check
thresholds are asked with three counts that differ for the same reason:
a sum that read one of them twice used to reach the same total.
Five non-test edits, four of them branches nothing can observe and one a
seam:
- isEmptyOrBoilerplateDescription's `lc == "id"` case is gone. A bare
"id" is two characters, so the length test above it has already
answered, and no input could reach the case.
- isNormalizedEnumParam's `deploy_access_levels` disjunct is gone: the
substring test for access_level already holds that name, so the
disjunct could never be the one that answered.
- inferOwnerFromName's empty-slice guard is gone: strings.SplitN
returns nil only when asked for zero pieces.
- summarize counts info as the switch default rather than as a third
case, since severityFor answers with one of three names and an
unrecognized one ranks as info everywhere else here.
- os.Exit in the -check gate is behind the osExit variable the sibling
audits keep, which is what lets the exit code be read at all.
collectAllClusters and buildPackageReports now take the collected spec
groups instead of the client that would fetch them: one catalog read is
shared between them rather than each asking the same cache for the same
slice, and a planted group can reach the two filters the live catalog
cannot, since no action of it is flag-free and no package of it is
finding-free.
One crossing the suite could not see is closed with it. The cluster a
finding belongs to is looked up by owner package and action name, both
strings taken off the same spec, and handing them over the other way
round finds nothing: every finding in a built report then carries no
cluster and a severity the escalation never reached, and the suite stayed
green, because nothing read a cluster off a report it had built. Twelve
such pairs were applied by hand, one at a time, and that was the one that
survived.
Mutants 124 killed / 0 lived / 11 not covered before, 125 / 0 / 6 after;
the six left are the case expressions of one switch, which Go's coverage
counts in no block, and each was applied by hand and seen to fail the
suite. Conditions 338 of 364 before, 353 of 356 after; the three left are
the branches cmdutil.Fatalf ends the process from, which this package
cannot observe without a seam in a helper it does not own.
…stics to stderr Both gates arrived clean on this package: gremlins reported `Killed 10, Lived 0, Not covered 0` and gobco `20/20` before a line changed, so the sweep here is entirely the class neither of them models, a pair of values no fixture can tell apart. Ten crossings were applied by hand, the suite run under each, and the file reverted. Nine died: the catalog options `run` asks for (dropping either `Enterprise` or `IncludeMCP` leaves eleven default aliases pointing at actions the narrower catalog does not hold, so the audit fails), auditing the pre-standalone catalog instead of the augmented one, the six TSV columns against each other, `errorCount` against `len(findings)` in the two summary lines, the two step names on the failure path, `os.Args` against `os.Args[1:]`, the flag set's own output stream, and the writer pair handed to `writeFindings`. One survived: `main` hands `os.Stdout` and `os.Stderr` to `runMain`, and the only test that reaches `main` aimed both at `os.DevNull`, where the two are one file. Swapped, this command writes the TSV a pipeline reads to stderr and the usage error to stdout, exits with the same codes, and every test stays green. It is now driven with the two streams pointed at separate files and each asserted for what it did and did not receive. No non-test file changed.
…to scan, at the widest tier, under the label each string came from The audit's own wiring was stated nowhere, and seven crossings of it left the whole suite green. Exchanging a tool's description and title, exchanging its input and output schemas, exchanging the resource and template listings, and naming a prompt argument where the prompt belongs each mislabel every row a maintainer would grep the source with. Listing the meta and individual surfaces at Free instead of the widest tier, and dropping either half of the scan from run, are worse: the gate keeps reporting the all-clear over a catalog it has stopped reading, and the door it stands in for rejects on the first offending character wherever that character sits. Two comparisons now state the pairing. Each builds the rows it expects from the listings directly, pairing each label with the field it names, and holds the scan to that multiset; the policy list is pinned to an ordinary letter for the duration, so the rows a crossing would move are produced without putting an offending character into anything the server serves. listSurface is asked for an Ultimate-only name on each tiered surface, which is what holds those two listings at the widest tier. A third test drives run itself and requires a row from all five listings, because the two scans are also driven directly and a run that stopped calling one of them would leave both of those tests green. The run without -apply was undriven as well, which is the gate as CI runs it and the only state in which the audit vouches for the text this build ships rather than for a rewritten one. It is now driven with an environment that would introduce a semicolon everywhere, and must still report the all-clear with nothing installed. That was also the one condition gobco reported never evaluated both ways.
…als and each side of a disagreement The condition gate reported six operands this package only ever evaluated one way, and probing the report for pairs no fixture could tell apart found three more properties nothing held. The ordering tests only exercised the pair sort.Slice has to move, so a comparator that reversed everything handed to it satisfied them; README.md is walked before docs and sorts before it too, which is the pair that says the order is the files' own. A page carrying a dozen buttons is past the size at which sort.Slice stops walking the slice once and starts comparing against a pivot, so the same two buttons are asked about in the other direction and have to answer the same way. A directory that is not this tree — a worktree the agent tooling checks out, or a dot-directory — was never planted below a scanned root, so the rule sourcewalk applies here was held by nothing. Nor was a payload whose percent escapes are malformed: url.QueryUnescape answers an empty string with its error, and an empty string is valid base64 for nothing at all, so taking it as a candidate would report a mangled link as a configuration that is not a client entry and send a reader looking for a retired flag. A subtree the walk cannot list was covered only through permissions, which a root process ignores, so that test skips wherever CI and this sweep run. A path longer than open(2) accepts is refused for everyone, and os.Root resolves one component at a time, so such a tree can be created although it cannot be walked. The three the pair probe found: the -v listing's own doc comment says it is the only way to see what a payload decodes to, and nothing asserted the column carried the decoded entry rather than the encoded one; and the disagreement report was asked only that the retired flag appear somewhere in it, which a report naming the button the others agree with, or putting the majority's arguments under "this button", would also satisfy. Condition coverage 52/58 to 57/58; the mutation gate was already at Lived 0, Not covered 0 and stays there. The one operand left is decodePayload's "lastErr == nil" fallback, which cannot be reached: the candidate list always holds the payload and every encoding either returns or records its error.
…and the tier it judges The mutation gate was already at Lived 0 and Not covered 0, so this closes what neither gate can see plus the five conditions gobco reported never evaluated both ways (137/142). Four crossings survived the whole suite and now fail it. A finding row could print what it found where the tool name belongs, and an -uncovered row could print the description line where the tool belongs, because every assertion about the report looked for words rather than for columns. The "enum value" and "doc value" labels could be exchanged, which is the one thing a row says about where a reader has to go to check the value, and every kind was compared against its own constant rather than against the word. And the surface the audit reads could be narrowed from Ultimate to Free: the clean-run gate stays green on it while the seventeen Premium and Ultimate group descriptions leave the comparison entirely. The conditions were the comparator's other leg and a nested opening remark. sort.SliceStable only ever asks about an adjacent pair, so each tie-break was only ever answered "swap these"; the three sort tests now drive both listing orders, and a line that reports a rejected value beside an unknown parameter is what asks the kind tie-break to leave a pair alone. The last one is a leading remark that brackets something of its own, where the remark ends at the bracket that opened it rather than at the first ")".
… own message The condition gate left fourteen operands one-sided, all of them in arms an audit tool reaches only on a tree it was not given: an entry under .github/workflows that is a directory or that cannot be read, a document node carrying nothing, a scripts/ path that is not a regular file or that the operating system refuses to read, a jobs value that is not a mapping, a document order naming a job the mapping lost or naming one twice, the SemVer rule put to docker-compose, a workflow whose document is not a mapping, and a working directory outside any module. Each now has a fixture, and documentMapping, orderedKeys and semVerCooldownKeys are driven directly, since their callers cannot produce those shapes. Three findings were counted rather than read, and each hides a swap of two arguments of the same type. checkInstallers carries the same two messages for both installers and differed only in the name inside them, so the bodies could be handed over the other way round and the count would still be right; the job rules take the workflow's label and the repository root as two plain strings, and a crossing there would name the root as the file and look for scripts under the workflow's name; and each run-time-code rule prints a pattern beside a reason out of one table of literals. All three are now asserted whole. The unreadable-script arm is exercised through /proc/self/mem rather than a mode-0 file: the gate runs privileged, where a mode-0 file reads fine and the fixture would skip exactly where the arm needs exercising. The test states that precondition and declines rather than passing vacuously where it does not hold.
… walks
The auditor crashed on a file it could parse. The MCP registrations take
their handler as the last argument, so boundaryCall answers len(args)-1,
and a call written with no arguments at all makes that -1; the bounds
check that followed only looked upward, so node.Args[-1] panicked the
walk. scanFile now checks the lower bound too, and a fixture holding
server.AddTool(), AddResource() and AddPrompt() is what fails without it.
The nil half of the audit closure's guard is deleted: all three call
sites reach it through a checked type assertion to *ast.FuncLit, so the
literal is never nil and only the seen set decides. A copy nothing can
observe is redundant code.
The tests close the six conditions gobco reported never evaluated both
ways and the pairs no fixture could tell apart:
- the library walk's own error return, reachable only from a tree
whose tests all parse, so the unparseable-source test is now two
cases and the second plants a broken .go file under the harness tree
- an import spec carrying no path, which a parser never produces and
importsTesting still has to survive, driven from a built AST
- a go statement, a registration argument and an assertion receiver
that are not the node the auditor is looking for
- four files whose tallies differ from one another and whose five
summary counters are five different numbers, so a permutation of the
report's format arguments or of the per-file columns now reads
differently from the correct report; on every earlier fixture those
counts were all 0 or all 1
- a tree of advisory sites and no abort, which the gate must pass
while naming how many it did not gate
- findings from both walks ordered together, since test files are
walked first and a library file whose path sorts before them is
collected second
Mutants 57 killed, 0 lived, 0 not covered, before and after (58 after
the added bound). Conditions 128/134 to 134/134.
Two more crossings the earlier fixtures could not tell apart, both found by applying them and watching the suite stay green. The default root list was only ever driven through cmd: the tree the test planted under internal held nothing, and the one under test held a clean fixture, so dropping either name from the list, or replacing all three with the working directory, changed no output anybody asserted. Each of the three now holds a finding of its own and a fourth directory outside the list holds one too, which is what makes the list a list. underHarnessTree cleans the path before it looks for the tree, and the spellings the case table held were all clean already, so removing the Clean went unnoticed. The table now carries one path with a "." element in it, joined by hand because filepath.Join would fold it away: the predicate is handed whatever spelling its caller holds and answers about the tree that spelling names. Mutants 58 killed, 0 lived, 0 not covered. Conditions 134/134.
…n and the list it files under A labeled break or continue names a loop the rewritten closure would no longer be inside, so it blocks the rewrite exactly as a goto does. Nothing drove that: reading the clause as a conjunction instead classified such a branch as an ordinary break and rewrote the site anyway, and the whole suite stayed green. Two cases now put a labeled branch of each kind through the control-flow walk. Two fixtures fill in the shapes neither earlier one reached: a declaration with no body, a Run call whose receiver is not a package identifier, a pointer element type, a right-hand side longer than its left, a body whose three calls are the ones the assertion rule has to tell apart, a map loop with no key variable, a slice loop with a blank element variable, a struct whose name field is not a string, a name outranking the desc beside it, and a loop that follows a synctest bubble rather than sitting inside it. Six control-flow cases reach the nested break, the classic for, the type switch and the select. The counts that could not tell two records apart now differ. A declared sequential loop and a bubbled one were both one of a kind in every fixture, so filing each under the other list, and swapping the two counters that report them, produced the report a correct run produces; the new fixture holds two of the first against one of the second, and the -check pass line is read for the number it prints. The six summary counters were printed positionally with two pairs sharing a value. The work list's two sort keys were only ever asked in an order a walk already sorted, so neither the file key nor the line key nor their precedence was held by anything. Conditions 225/244 to 244/244. Mutation stays Killed 99, Lived 0; the eight reported Not covered are the case expressions of one tagless switch, which sit in no counted block because Go's coverage blocks begin after the colon, and every one of them was applied by hand and dies.
jmrplens
force-pushed
the
sweep-cmd-audits
branch
from
September 21, 2026 15:53
7e016a2 to
af366cf
Compare
… under The merged backlog's counters were filled with numbers that read the same, and every case decoded the output back through this package's own types, which left the key names the artifact publishes unasserted. The action stream's api_methods and covered_methods reached no assertion at all and the four metadata flags were each tallied once everywhere, so any pair of them could have traded places and every test would have read the same. One fixture now names one package from every stream with no two of its numbers alike, and the whole summary is compared as a record rather than a field at a time. Proving it caught a weakness of its own making: the first fixture's api_methods minus covered_methods came to the six missing methods beside them, so a summary that counted the shortfall instead of the list read correct; the covered count moved to 22. Reading the output back through metaFinding, structGaps and the rest makes their json tags invisible, since a pair traded between two adjacent fields is written one way and read back the other and the round trip agrees with itself. The new case reads the document as a map and names each published path, which is what fails when structGaps, backlogSummary, actionGap or enumGaps trade tags. A trade inside metaFinding stays invisible there and always will: that type decodes the incoming report and encodes the outgoing one, so the two swaps cancel. Last is the one property of the four merge functions no branch flip reaches: the struct stream carries a package whatever its counts, since its report is a full census when -gaps-only is off, while the action, metadata and enum streams each leave a clean entry out. Every other case drives gapped packages, so adding a skip to the first or dropping one from the others changed nothing any test could see.
|
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.
The mutation and condition sweep, applied to sixteen of the audit commands under
cmd/. Each package was measured with gremlins and gobco, every survivor and every undecided condition was read by hand, and what the two tools cannot model was probed by crossing the assignments they cannot tell apart.The packages:
audit_1to1and itsactions,enumsandmetadataanalyzers,audit_doc_coverage,audit_doc_tool_names,audit_discovery_completeness,audit_dynamic_aliases,audit_gateway_chars,audit_install_buttons,audit_meta_descriptions,audit_supply_chain,audit_test_goroutinesandaudit_test_subtests.Three things are worth knowing about this batch, because they are what the sweep is for rather than what it produced.
The class neither tool models is a pair of assignments no fixture can tell apart: two fields of the same type filled from same-valued fixtures, or a result nothing reads back. Crossing them changes no test, so gremlins reports the line killed and gobco reports the condition decided, and the surface is wrong in a way nobody would see. Closing one means distinct fixture values and a whole-object comparison, and each is proved by applying the crossing, watching the suite fail, and reverting.
audit_supply_chaincarried one in its own output: the two installers are read into two strings of the same type and passed positionally, so a swap changed which file a finding accused and not how many findings came back. The fixture makes them differ now and the assertion names the file.One production message changes with the tests: the installer finding's em dash is a semicolon, so the sentence the audit prints and the sentence the test spells are both what the rule for this repository asks for.