Improve pipelines #177
Improve pipelines #177
Conversation
WalkthroughTwo GitHub Actions workflows are modified to optimize CI/CD pipelines. The e2e.yaml workflow adds a new caching step that caches Go build artifacts and module dependencies, using go.mod file hash as part of the cache key. The test.yml workflow modifies the test coverage command to exclude additional files (store.go and mutable_tree.go) from coverage reporting, appending to existing exclusion filters. Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/test.yml (1)
38-46: Avoid double-caching Go modules/build cache (setup-go cache + actions/cache).You already have
actions/setup-go@v5withcache: true(keyed bygo.sum), and also an explicitactions/cachefor~/go/pkg/modand~/.cache/go-build. This is likely redundant and can cause confusing cache behavior; consider keeping just one approach (prefersetup-go’s built-in cache unless you need custom paths/keys)..github/workflows/e2e.yaml (1)
36-44: Includego.sumin the cache key (go.mod-only can serve stale deps).Keying solely on
**/go.modcan miss dependency changes captured ingo.sum. Suggest:- key: ${{ runner.os }}-go-${{ hashFiles('**/go.mod') }} + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum', '**/go.mod') }}(or just
**/go.sumif you prefer one source of truth).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/e2e.yaml(2 hunks).github/workflows/test.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/e2e.yaml (1)
45-63: Parallel warm-up viago test -run=^$looks fine; ensure it doesn’t race the module cache.Given this runs alongside docker builds, caching should help, but if you see occasional flakiness due to concurrent writes into
~/go/pkg/mod/~/.cache/go-build, consider making only one step populate the Go cache (e.g., rungo mod downloadsingle-threaded before the parallel section).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/e2e.yaml (2)
36-44: Cache key should includego.sum(and ideally Go version) to avoid stale restores.
Right now the key only hashes**/go.mod, so dependency graph changes that only touchgo.sum(or different Go versions) can restore an incompatible cache and lead to flakiness.Suggested tweak:
- uses: actions/[email protected] with: path: | ~/.cache/go-build ~/go/pkg/mod - key: ${{ runner.os }}-go-${{ hashFiles('**/go.mod') }} + key: ${{ runner.os }}-go-${{ steps.setup-go.outputs.go-version }}-${{ hashFiles('**/go.mod', '**/go.sum') }} restore-keys: | - ${{ runner.os }}-go- + ${{ runner.os }}-go-${{ steps.setup-go.outputs.go-version }}- + ${{ runner.os }}-go-(You’d need to add
id: setup-goto theactions/setup-go@v5step.)
45-65: Parallel step: consider adding a trap to kill background jobs on failure/timeout.
Withset -e, a failure after starting background jobs can leave other jobs running until the runner tears down; a simpletrap 'kill $(jobs -p) || true' EXITmakes this more robust.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/e2e.yaml(2 hunks)
🔇 Additional comments (1)
.github/workflows/e2e.yaml (1)
66-70: No concerns with the E2E invocation gating / env wiring.
SKIP_IBC_TESTSis still evaluated only whenenv.GIT_DIFFis set, andmake test-e2eremains unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/e2e.yaml(2 hunks)app/app.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: tests
- GitHub Check: test-e2e
- GitHub Check: golangci-lint
- GitHub Check: Analyze
🔇 Additional comments (2)
.github/workflows/e2e.yaml (2)
36-43: LGTM! Caching will speed up CI builds.The Go module and build cache configuration follows GitHub Actions best practices and should reduce build times as noted in the PR description.
46-46: Verify that E2E steps should always run unconditionally.The
if: env.GIT_DIFFconditions are now commented out, which means the build and test steps will execute on every workflow run regardless of whether Go-related files changed. While this ensures comprehensive testing, it increases CI resource usage.Confirm this change is intentional, especially since the
technote-space/get-diff-actionstep still runs but its output (GIT_DIFF) is no longer used.If the intention is to remove these conditions permanently, consider also removing the unused
get-diff-actionstep (lines 24-34) to simplify the workflow.Also applies to: 67-67
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/e2e.yaml (1)
66-70:SKIP_IBC_TESTSenv assignment looks fine; consider quoting for clarity/consistency.Not required, but you may prefer always-quoted strings for env values to avoid any YAML ambiguity:
- SKIP_IBC_TESTS: ${{ github.event_name == 'pull_request' && 'true' || 'false' }} + SKIP_IBC_TESTS: "${{ github.event_name == 'pull_request' && 'true' || 'false' }}"
Description
Improve test pipelines by:
store.goandmutable_tree.goType of change
Test
Pipeline changes: