Repository navigation
ci: fix build cache, drop duplicated test run - #1385
Open
marcleblanc2 wants to merge 3 commits into
Open
marcleblanc2 wants to merge 3 commits into
marcleblanc2 wants to merge 3 commits into
Conversation
Every CI run compiled the whole dependency tree from scratch on all three
OSes, because:
- goreleaser-check ran setup-go without ever building anything, so it
finished first and saved a 7 KB cache under the key that go-lint and
go-test on Linux then restored ('cache hit', never re-saved).
- setup-go's built-in cache has no restore-keys, so any go.sum change
(every renovate PR) started cold on macOS and Windows too.
- go-ci ran 'go test -race' and then 'go test' again, recompiling
everything without race instrumentation to rerun the same tests.
Now each job owns its cache key (job, OS, arch, Go version, go.sum hash)
with a prefix fallback, goreleaser-check no longer installs Go, and the
test job runs the race pass only.
Amp-Thread-ID: https://ampcode.com/threads/T-01a18b5c-c5bb-7497-82f1-3e9333c8b7e1
Co-authored-by: Amp <[email protected]>
The upload tests were tagged //go:build !race in #1082 because testify fmt-prints every recorded mock argument, and printing a bare *io.PipeReader reflects into the pipe's internals while the producer goroutine writes to it. The Windows variant never got the tag, so 'go test -race' has been failing on Windows the whole time, hidden because the step runs under pwsh, which does not fail on a non-zero exit in the middle of a multi-line script; the second non-race pass then went green. The mock client now wraps the body in a RequestBody struct, so fmt prints the pointer address instead of walking the pipe. That removes the need for the !race tag and for the second test pass. uploadFile also starts the writer goroutine only after NewHTTPRequest succeeds; before, an error there leaked a goroutine blocked forever on a pipe nobody reads. Amp-Thread-ID: https://ampcode.com/threads/T-01a18b5c-c5bb-7497-82f1-3e9333c8b7e1 Co-authored-by: Amp <[email protected]>
marcleblanc2
changed the title
marcleblanc2
enabled auto-merge (squash)
October 30, 2026 06:33
marcleblanc2
changed the title
marcleblanc2
changed the title
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.
Why
CI compiled the whole dependency tree from scratch on every run, on all three OSes: go-test took 5–9 min, go-lint 3 min. Three causes:
goreleaser-checksaved an empty cache under the same key the test and lint jobs use, so they always "hit" that empty cache and never saved their own.go-ciran the test suite twice: once with-race, then again without, which recompiles everything. The second pass only existed because the upload tests ininternal/batches/servicewere excluded from-race(Turn off race detection for upload test #1082).-race(the Windows upload test was never excluded), but the non-race second pass masked it.What
go.sumchange only downloads the delta.-race, on all three OSes.goreleaser-check; it only parses the config.sourcegraph/actions/go-setupwithactions/setup-gobecause the former doesn't exposecache/cache-dependency-pathinputs, and src-cli stopped using the former's private-repo token in chore: use sourcegraph/actions/go-setup and upgrade mise go to 1.25.6 #1251.go-ci.ymlandgo-lint.ymlcould switch back tosourcegraph/actions/go-setup@mainwithcache: falseWhy application code changed
Running the upload tests under
-raceexposed a real race in the test mock, not in the tests themselves. The mock recorded the request body as a bare*io.PipeReader; when testify printed the recorded arguments to compare them, it walked into the pipe's internals while the upload goroutine was still writing to it.internal/api/mocknow wraps the body in a smallRequestBodystruct so testify prints a pointer instead of walking the pipe. With that fixed, the!racebuild tag and the separate non-race test pass are no longer needed.While there,
uploadFilenow starts its writer goroutine only after the request is built; before, an error building the request leaked a goroutine blocked on a pipe nobody reads.Measured on this branch
workflow_dispatchrerun)Caches on
mainappear after this merges; until then every branch's first push is cold.Amp thread
https://ampcode.com/threads/T-01a18b5c-c5bb-7497-82f1-3e9333c8b7e1