test: build mockstdio_server with isolated cache to prevent flaky CI #241
Conversation
WalkthroughThis change updates the Go build process in two test files by removing the Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
client/transport/stdio_test.go (1)
25-27: Good implementation for isolated build cache.The change aligns well with the PR objectives by creating a unique temporary directory for each build's cache a 8000 nd setting the
GOCACHEenvironment variable accordingly. This effectively prevents race conditions between parallel builds.However, there's a minor issue with error handling:
Consider handling the error from
os.MkdirTemprather than discarding it:-tmpCache, _ := os.MkdirTemp("", "gocache") +tmpCache, err := os.MkdirTemp("", "gocache") +if err != nil { + return fmt.Errorf("failed to create temporary cache directory: %v", err) +}Also, consider cleaning up the temporary directory after the build completes to prevent accumulation of unused directories:
cmd.Env = append(os.Environ(), "GOCACHE="+tmpCache) +defer os.RemoveAll(tmpCache)client/stdio_test.go (1)
26-28: Good implementation for isolated build cache.The change aligns well with the PR objectives by creating a unique temporary directory for each build's cache and setting the
GOCACHEenvironment variable accordingly. This effectively prevents race conditions between parallel builds.However, there's a minor issue with error handling:
Consider handling the error from
os.MkdirTemprather than discarding it:-tmpCache, _ := os.MkdirTemp("", "gocache") +tmpCache, err := os.MkdirTemp("", "gocache") +if err != nil { + return fmt.Errorf("failed to create temporary cache directory: %v", err) +}Also, consider cleaning up the temporary directory after the build completes to prevent accumulation of unused directories:
cmd.Env = append(os.Environ(), "GOCACHE="+tmpCache) +defer os.RemoveAll(tmpCache)
CI occasionally failed with the linker error:
/link: cannot open file DO NOT USE - main build pseudo-cache built
This is most likely because several parallel `go build` invocations
shared the same `$GOCACHE`, letting one job evict the object file
another job had promised the linker. The placeholder path then leaked
through and the build aborted.
This gives each compile its own cache by setting `GOCACHE=$(mktemp -d)`
for the helper’s `go build` call.
After these changes `go test ./... -race` passed 100/100 consecutive
runs locally.
dc624ae to
3d3a7a0
Compare
…ark3labs#241) CI occasionally failed with the linker error: /link: cannot open file DO NOT USE - main build pseudo-cache built This is most likely because several parallel `go build` invocations shared the same `$GOCACHE`, letting one job evict the object file another job had promised the linker. The placeholder path then leaked through and the build aborted. This gives each compile its own cache by setting `GOCACHE=$(mktemp -d)` for the helper’s `go build` call. After these changes `go test ./... -race` passed 100/100 consecutive runs locally.
…ark3labs#241) CI occasionally failed with the linker error: /link: cannot open file DO NOT USE - main build pseudo-cache built This is most likely because several parallel `go build` invocations shared the same `$GOCACHE`, letting one job evict the object file another job had promised the linker. The placeholder path then leaked through and the build aborted. This gives each compile its own cache by setting `GOCACHE=$(mktemp -d)` for the helper’s `go build` call. After these changes `go test ./... -race` passed 100/100 consecutive runs locally.
CI occasionally failed with the linker error:
This is most likely because several parallel
go buildinvocations shared the same$GOCACHE, letting one job evict the object file another job had promised the linker. The placeholder path then leaked through and the build aborted.This gives each compile its own cache by setting
GOCACHE=$(mktemp -d)for the helper’sgo buildcall.I hadn't seen #240 when I started working on this and it addresses the issue as well. I think these changes are complimentary though and using isolated
GOCACHEis probably still a useful change.Summary by CodeRabbit