Repository navigation
Conversation
Since gobco resolves types, it type-checks a black box test in 'package x_test' with the source importer, which loads the package 'x' from its non-test files only. A black box test that uses a name from export_test.go, the usual way to hand an unexported name to such a test, therefore made gobco panic with "undefined", although 'go test' accepts the package. In 'go test', the package under test includes its own test files, and each package that the black box test imports and that imports 'x' is compiled again against this 'x'. Do the same when resolving the types: the black box test gets the 'x' that gobco has already type-checked from all of its files, and each imported package that depends on 'x' is type-checked again, once, against that 'x'. Replacing only the import of 'x' is not enough, as a package that passes types of 'x' to the black box test would still see the other 'x'. A package that is type-checked again takes its imports from the package that the source importer built, since asking the source importer again would run the go command once more for each import outside the standard library. A package that has no test files of its own gets the same types as before, as its black box test is still checked with the source importer alone.
jmrplens
added a commit
to jmrplens/gitlab-mcp-server
that referenced
this pull request
Oct 28, 2026
…d date the linter count (#1558) Closes #1542 Refs #1269 Three answers of the OpenSSF Best Practices form rested on evidence weaker than they claimed. dynamic_analysis had the race detector and the e2e suite but no tool that varies its inputs: the module carries eleven fuzz targets and `go test` only ever ran their seed corpora. test_branch_coverage80 asks for the share of the code's branches the suite covers, measured with a FLOSS tool, and the repository had gobco figures for three packages, none for the tree, and none in the mode that counts branches. warnings cites the linters the configuration enables, and `.golangci.yml` and the static analysis page said 55 where it enables 52. This pull request gives each of the three the evidence it lacked; whether each answer becomes Met is the maintainer's call, under Before this merges. `.github/workflows/fuzz.yml` fuzzes every target with `go test -run '^$' -fuzz '^<target>$'`, one job per target: for ten minutes on Wednesdays at 05:43 UTC, a day no other weekly workflow takes, and when dispatched, and for one minute on a pull request that changes the workflow or its plan. GitHub offers a workflow for dispatch only once its file is on the default branch, so this pull request can run `fuzz.yml` through that trigger alone; a later change could be dispatched from its branch, and the trigger runs it without anybody having to. No other pull request runs it, and it stays out of `ci.yml`, so it gates nothing. When a target fails, `go test` writes the input under the package's `testdata/fuzz/<target>/`, and the job uploads that directory as `fuzz-<package>-<target>`. The static analysis page gains a Fuzzing section beside the race detector, with how to reproduce such an input and keep it as a regression seed. The targets are not written into the workflow. `cmd/fuzz_targets` lists every package with the toolchain's own go, parses the internal and external test files the go command builds for each, and prints a matrix cell per top-level function the go command reads as a fuzz target (`Fuzz`, or `Fuzz` followed by anything but a lower-case letter, so `Fuzzy` is not one, and a method is not either). A target added anywhere is fuzzed from the next run on, and a plan that finds none fails rather than passing with nothing fuzzed. Its tests hold the workflow to the matrix: the triggers, the read-only token, the anchored `-fuzz`, the fuzz time per trigger and the path the upload takes. `cmd/measure_conditions` takes the whole tree. `plan -tree` lists every package of `./cmd/...` and `./internal/...` and deals them round twenty shards, the first package to the first shard, the second to the second and so on round again, so the slow packages of one directory land in different jobs; each shard is a job on each runner. The shell that wrote one record per job is now the `measure` subcommand, which runs `scripts/coverage-conditions.sh` over each package of its job one after another, streams the output to the log and leaves each package's `gobco.txt` and `run.env` in a directory of its own, so the writer of a record sits beside its reader and their tests hold each to the other. `summarize` adds each runner's total over the packages it measured, with the share rounded down to a tenth of a percent so it never reads higher than it is (799 of 1000 is 79.9%); a package that was not measured is in no total, and the summary quotes the lines of its output that say why: the script's own, and the panic, the failing test, the test package that did not build or set up, or the compiler position gobco's run printed before them. `tree` runs a whole `-tree` dispatch on the machine at hand and writes the same summary, and `make coverage-conditions-tree` runs it, so the figures can be taken again without a dispatch. `conditions.yml` gains the `tree` input, names each job after its package or shard, and gives a shard's job two hours while a job of a named package keeps the hour it had. gobco counts in one of two modes, and every figure here had come from its default, which counts every atomic condition: each operand of `&&` and `||` and each comparison wherever it is written, a `return` statement as much as an `if`. That is not what the criterion names. gobco 1.3.4 also has `-branch`, which counts the whole controlling condition of each `if` and `for` and each case of a `switch`, and nothing else. `scripts/coverage-conditions.sh` now reads `GOBCO_MODE`, `condition` by default or `branch`, refuses any other value before gobco runs, and refuses a report without the figure of its mode as it refuses a `0/0`. `measure_conditions` takes `-mode` on `plan`, `summarize` and `tree`, reads each record's figure in the words of its mode and heads the summary with it; `conditions.yml` gains the `mode` input and passes it to the plan, the measuring step and the summary, and `make coverage-conditions-tree` takes `CONDITIONS_MODE`. I took both figures with that command on 26 October 2026, on linux/amd64, on this branch once its Go code was final, when it sat on `main` at `ed18d7d12`, four packages at a time on ten cores of a machine other work was loading too. The stack has since been rebased onto a later `main`, which brought packages of its own, so the page says which tree was measured and that those packages are in neither total; the only Go change on the branch since is the fuzz target name below. Branch coverage: 265 of the 296 packages below `cmd/` and `internal/` measured, 44936 of their 44954 outcomes taken, 99.9% rounded down, in an hour and twelve minutes. Condition coverage: the same 265, 53412 of 53432 outcomes evaluated, 99.9%, in an hour and twenty-four minutes. `docs/development/testing/README.md` records both with the date and both ways to take them again, and says what each mode counts and why the branch figure can leave out no more outcomes than the condition figure over the same packages: Go stops evaluating `&&` and `||` once the outcome is known, so a decision whose every condition went both ways has gone both ways itself. The eighteen branch outcomes left are eighteen decisions taken one way in nine packages. Thirteen are the error branch of a call no test makes fail (three on a random read, two on a request build, a stat, a write, a schema build, a catalog build, a marshal, an encode, an unmarshal and a marshal back), two are per-action metadata decorations of the kind `make check-spec-conditions` exists to find, and three are cases no test reaches: a decoding with no candidate to try, a URL elicitation in a session negotiated at 2026-07-28 or later, and an input schema without its `value` property. The twenty condition outcomes left are the same places, the elicitation one at its second operand, and two operands in `cmd/audit_edition_tier` that no decision holds whole. That run could not measure 31 packages, and none of them fails under `go test`: twenty-seven hand symbols from an `export_test.go` to their external test package, which gobco type-checks against the non-test files alone (rillig/gobco#42); three have an external test package that does not import the package, for which gobco writes its bridge with an empty import path (rillig/gobco#33); and two have tests that read, in gobco's copy, what gobco rewrote there: `internal/completions` the case labels of its switches, and `internal/testutil/serialtypecheck` the imports of every file of its package, gobco's own among them. gobco v1.3.4 is still its latest release and fixes neither, so `scripts/coverage-conditions.sh` now stages both shapes, and the two tests read the same thing in the copy as in the source (After the first CI round, below). Measured that way on 27 October, on the rebased branch, the 31 take 3773 of their 3778 branch outcomes and evaluate 4113 of their 4120 condition outcomes, so all 296 packages are measured, 48709 of 48732 branch outcomes and 57525 of 57552 condition outcomes over both runs, 99.9% each, from two trees a day apart; the page says so. The counts beside it are corrected too: twenty-two of the 149 sub-packages of `internal/tools` that carry an `action_specs.go` were in the `export_test.go` state, where the page and `scripts/check-spec-conditions.sh` said twenty-five of 169. `.golangci.yml` and the static analysis page now say what the sentence behind the concurrency cap measured and when: July 2026, when the module had 226 packages and the configuration enabled 52 linters and 3 formatters, which the 55 had counted together. Today it enables the same 52 and 3 (`golangci-lint linters` on v2.15.0), and `git grep -n "55 linters"` finds nothing. ## How I checked it - `cmd/fuzz_targets`: its eleven tests failed against stubs before the command existed, and pass with 100.0% of statements; `GOBCO_GATE=all bash scripts/coverage-conditions.sh ./cmd/fuzz_targets` reads 40/40, and `make coverage-mutants PKG=./cmd/fuzz_targets` killed 24 mutants with none lived, uncovered or timed out. `go run ./cmd/fuzz_targets` on this tree plans the eleven targets in five packages. - `cmd/measure_conditions`, the tree subcommands: their tests were written first, but what I saw fail before the code existed was the build (`undefined: treePackages`, `runMeasure`, `measurePackage`, `runScript` and the rest), not an assertion, so the proof that those tests decide the behavior is the mutation run below rather than a red run. - `cmd/measure_conditions`, the reasons a summary gives, the job bound and the mode: those tests were written first and run against the code before them, with stubs for the new signatures where the mode needed them, and failed on their assertions (every new case of the mode, the four new shapes of a reason, and the measure job's bound read as 120 where the test wants the expression). The package passes with 100.0% of statements; gobco reads 270/270 in condition mode and 226/226 in branch mode; gremlins killed 166 mutants with none lived, uncovered or timed out. - `scripts/coverage-conditions.sh`: four new cases in `scripts/coverage_conditions_sh_test.py` (the mode reaching gobco as `-branch` before the tags and the test flags, staged or not; a value it cannot read refused before anything runs; a report without the figure of its mode refused; the gate reading a branch report) failed thirteen subtests against the script before the mode, and the whole suite, thirty tests, passes, as do `scripts/check_spec_conditions_sh_test.py` and shellcheck on both scripts. - Each of the eleven fuzz targets ran clean under `go test -fuzz` for one minute, as the pull request trigger runs them (`cmd/server`'s in 143 s with the build of its instrumented binary, the others in 60 to 72 s). - The two whole-tree runs above measured the same 31 packages as not measured, each with its cause quoted in the summary, and the condition run found the same twenty conditions as an earlier run over the tree before the mode existed. - `go test ./... -count=1` passed at the head before the rebase with the freshness comparisons on and with them deferred, and passes at the rebased head with them deferred, as CI runs a layer below the top. After the rebase it first failed in `cmd/fuzz_targets`: #1498 renamed `internal/toolutil`'s target to `FuzzScanLogMarkup_RawOffset`, and the discovery test named it the old way, so the test names it as the tree spells it now. golangci-lint is clean on both commands, actionlint v1.7.7 accepts `conditions.yml` and `fuzz.yml` (the measure job's bound is the expression `${{ inputs.tree && 120 || 60 }}`), and `make check-supply-chain` (the new workflow's actions are pinned to the commits the others use), `check-test-file-names`, `check-test-subtests`, `check-test-goroutines`, `check-godocs`, `check-dead-consts`, `check-doc-tool-names`, `check-tenancy`, `check-md-escaping`, `check-sdk-context`, `check-action-ids` and `check-llms` pass. - `go run ./cmd/gen_testing_docs/ --check` and `check-site-stats` pass, and so do `node scripts/check-doc-links.mjs`, markdownlint on every changed Markdown file, the table formatter's `--check`, `make check-em-dash` over this layer and `make check-pr-description` over this title and description. ## Before this merges These are the maintainer's, and nothing above claims them: - **The first runs.** I could run neither workflow on GitHub. This pull request runs `fuzz.yml` through its paths trigger, eleven jobs of one minute each, which should pass; the first ten-minute run is the first Wednesday after the merge, or a dispatch then. `conditions.yml` is already on `main`, so `gh workflow run conditions.yml --ref bp-analysis -f tree=true -f systems=ubuntu-latest -f gate=all -f mode=branch`, and the same with `-f mode=condition`, should run this branch's version. Its first dispatch, before the fixes below (runs 40467477742 and 40467482058), failed sixteen shards each on the 31 packages gobco could not read; the next should measure all 296, and a shard fails only where a package's figure leaves outcomes untaken under `gate=all`, which is a measurement, not a failure to measure. - **The Best Practices form.** dynamic_analysis can cite `fuzz.yml` and its first scheduled run, and warnings the 52 linters of `.golangci.yml`. For test_branch_coverage80 the evidence is the branch coverage of 99.9% over all 296 packages, from the two runs above; whether that answers the criterion as Met is the maintainer's judgment. - **Issues to open.** One or more for the twenty-three decisions taken one way: the eighteen in nine packages of the first run, with the two `cmd/audit_edition_tier` conditions beside them, and the five in `internal/tools/resourceevents` and `internal/tools/workitems` the second run found, with the two operands of the `workitems` clear guard beside them, each a test case or a declaration. - **The testing reference.** It is regenerated once, at the top of the stack, which gives the `cmd/fuzz_targets` row the 100.0% `go test -cover` reports. ## On the rebased stack I rebased the stack onto `main` at `8f339f4e8` on 27 October 2026. The checks above ran on this layer before that rebase, except where they say otherwise. On the rebased head, with the two commits the rebase called for (the fuzz target's new name and the dated measurement), `go build ./...`, `go vet ./...` and `go test ./... -count=1` pass with the freshness comparisons deferred, as CI runs every layer below the top, and every commit carries a `Signed-off-by` trailer. The committed generated artifacts are regenerated once at the top of the stack, which runs every gate over the combined tree. ## After the first CI round - **Sonar.** `runScript` ran `bash` by its name (S4036), and now resolves it with `exec.LookPath` first and runs the absolute path, as `cmd/gen_orbit_record` and `cmd/gen_model_results` already run git; a PATH holding no bash is reported as the lookup's failure, which a new test holds. `measurePackage` and `measureAll` took eight parameters each (S107); the runner, the module root, the system, the directory of records and the environment are now one `measurer` value. Both are folded into the commit that added the subcommands. `cmd/measure_conditions` stays at 100.0% of statements, 272 of 272 conditions and 228 of 228 branches, and gremlins kills the 38 mutants of `measure.go` and `tree.go` with none lived or uncovered. - **The 31 packages the dispatch could not measure** (runs 40467477742 and 40467482058, the same 31 with the same causes in both modes). I reproduced each kind on the remote runner with the pinned gobco, v1.3.4, which is still its latest release. `scripts/coverage-conditions.sh` now stages a package with an external test package that gobco cannot read: each of the package's own test files that declares no test, benchmark, fuzz test or example becomes a file of the package in the copy, renamed without its `_test` suffix and opened with `//go:build gc`, which the go command satisfies and gobco's narrower build context does not, so it is compiled into the test binary as before, its names reach the external tests, and gobco leaves it uninstrumented, as it leaves a test file; and an external test package that imports nothing of the package is given a test file that imports it, beside the one name it reads in a file of the package behind the same constraint. No position gobco reports moves, and a package with neither shape is still measured where it is. `internal/completions`'s tests now look for a case label inside its expression, which reads the same labels from gobco's `GobcoCover(n, gobco0 == "label")`, and `internal/testutil/serialtypecheck`'s test leaves gobco's own `gobco_*.go` files out of the imports it holds the package to. Of four new cases in `scripts/coverage_conditions_sh_test.py`, the two staging cases fail against the previous script, on a stand-in gobco that panics on the undefined name and fails setup on the bridge as gobco does, a third shows the stand-in fails that way on the package in place, and a fourth holds that an external test package gobco can read is still measured where it is; its 34 tests pass. On 27 October 2026 I decided to keep this staging, which supersedes my decision of 6 October, recorded in upstream-bugs entry 97 and issue 1099, to leave the `export_test.go` limit to gobco with no workaround in the staging script. The staging is temporary: it goes when gobco publishes a release with the fixes of rillig/gobco#43 (for rillig/gobco#42) and rillig/gobco#39 (for rillig/gobco#33). Entry 97 records that decision and the workaround, entry 140 is new, for the bridge, and the script, the Makefile and the testing page say the staging is temporary. --------- Signed-off-by: José M. Requena Plens <mail@jmrp.io>
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.
Fixes #42.
Since 1.3.4, gobco resolves types, and it type-checks a black box test (
package x_test) through the source importer ofimporter.ForCompiler, which buildsxfrom theGoFilesandCgoFilesthatgo/buildlists, so without the test files ofx. Anexport_test.gois the usual way to hand an unexported name to a black box test, and as soon as the black box test uses one, gobco panics before it instruments anything. The issue has a minimal reproduction; on the test data of this pull request,gobco -cover-test testdata/exporttestwith gobco built from master says:go testaccepts the same package, because it builds the package under test together with its own test files, and compiles again, against thatx, every package on an import path from the black box test tox. For the new test data,go list -test -deps ./testdata/exporttestlists, besides the black box test itself, these three packages as built for the test binary:resolveTypesnow type-checks the packages of the directory in name order, soxbeforex_test, and whenxhas test files of its own, it checks the black box test through an importer that:xthat gobco has just type-checked from all of its files, test files included,x, directly or indirectly, again against thatx, once per package and without function bodies, like the source importer does,Test_gobcoMain__exportTestfails without each of the following, which I checked by taking them out one at a time:xreplaced,filterstill sees the otherexporttest:cannot use exporttest.IsEven (value of type func(n "exporttest".Number) bool) as func("github.com/rillig/gobco/testdata/exporttest".Number) bool value in argument to filter.Keep.numbers.Listfrom the test and fromfilterare two different types:cannot use numbers.Of(1, 2, 3, 4) (value of slice type numbers.List) as numbers.List value in argument to filter.Keep.xbefore the black box test is checked, because the test may import a package that depends onxbefore it importsxitself. The test data imports them in that order, and finding the path only whenxis imported fails like the first case.That lookup asks
go/build, inFindOnlymode, where the imports of the black box test live, until it finds the one in the package directory, so it does not need #39's reading ofgo.mod. For an import outside the standard library,go/buildasks the go command in module mode, as it does for every import the source importer resolves.A package that is type-checked again takes its imports from the package that the source importer built for it, instead of asking the source importer again. The source importer asks
go/buildwhere a package lives on every call, even for a package it has already loaded, so asking again would run the go command once more for each import outside the standard library. I measured what this costs oninternal/tools/deploykeysof gitlab-mcp-server, whose black box test importsinternal/tools, a package that importsdeploykeysand is therefore type-checked again: from the start until gobco reports the package as instrumented, it took 57.9 to 60.6 seconds with this branch and 59.1 to 60.4 seconds with master at 7a09995, in three alternating runs of each with Go 1.27.1 on linux/amd64.A package that has no test files of its own gets the same types as before, as its black box test is still checked with the source importer alone.
I found this on gitlab-mcp-server, where 26 packages keep an
export_test.gonext to a black box test, andgo testpasses on each of them. v1.3.4 and master at 7a09995 stop on every one of them before they instrument anything. On 25 of them, they panic withundefinedon a name that the package'sexport_test.godeclares, and with this branch, gobco instruments all 25, reports their condition coverage, and their tests pass. The remaining one,internal/testutil/serialtypecheck, also meets #40, as it declaresraceEnabledinrace.goand innorace.gounder opposite build constraints. With this branch, gobco stops there on every run withraceEnabled redeclared in this block, since it now checks the package under test first; v1.3.4 and master check the packages of the directory in map order and stop there either with that or withundefined, in most of my runs with the redeclaration. With this branch on top of the branch of #41, gobco instruments that package too, and only its own test fails then, because it reads the imports of every file of its package and objects to thetestingimport of thegobco_no_testmain_test.gothat gobco adds.This is independent of #41, my other open pull request, which fixes #40: that one changes which files of the directory gobco reads and with which build tags, this one how the imports of the black box test are resolved. Both touch
instrumenter.go,instrumenter_test.goandmain_test.go, and git combines the two branches without a conflict. With both branches in, the packages that are type-checked again also get the build tags that #41 sets inbuild.Default, since this importer reads it like the source importer does.sh ./test.shpasses with Go 1.16.15, 1.20.14 and 1.27.1 on linux/amd64, both on this branch on top of master at 7a09995 and on this branch on top of the branch of #41. No dependency is added.Left out on purpose: a package that depends on
xand uses cgo is type-checked again withFakeImportC, so whatever its declarations take fromCstays unresolved in that copy, where the source importer runs cgo for it.