perf: consolidate workflows & cache build artifacts#18
Conversation
- Rename test.yml to squad-test.yml and convert to reusable workflow - Add workflow_call trigger to enable reuse by squad-ci.yml - Refactor squad-ci.yml from 179 lines to 71 lines (60% reduction) - Consolidate versioning and test execution into thin orchestrator - Keep all 6 test jobs (unit, architecture, blazor, integration, aspire, coverage/report) - Remove duplicate build/restore steps from squad-ci.yml - Maintain single source of truth for test execution Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Eliminates redundant dotnet build in 5 parallel test jobs: - build job caches bin/ and obj/ directories - test jobs use --no-build flag (skip rebuild) - Saves 10-15 minutes per workflow run Cache invalidates on .csproj or Directory.Packages.props changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Documents build artifact caching strategy, trade-offs, and performance impact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #18 +/- ##
=======================================
Coverage 46.22% 46.22%
=======================================
Files 25 25
Lines 411 411
Branches 17 17
=======================================
Hits 190 190
Misses 216 216
Partials 5 5 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR optimizes the CI/CD pipeline by consolidating test workflows and implementing build artifact caching. The changes transform squad-ci.yml into a thin orchestrator that calls squad-test.yml as a reusable workflow, reducing duplication and line count by 60%. Build artifact caching is introduced to eliminate redundant compilation across parallel test jobs, with all 5 test jobs (unit, architecture, bunit, integration, aspire) now sharing binaries from a single build job.
Changes:
- Added
workflow_calltrigger to squad-test.yml, making it reusable by other workflows - Refactored squad-ci.yml to orchestrate versioning and notifications, delegating tests to squad-test.yml
- Implemented build artifact caching strategy: build job caches
bin/Release/andobj/directories, test jobs restore and use--no-buildflag - Removed redundant build steps from all 5 test jobs, reducing overall build time
- Added comprehensive decision documentation explaining the caching strategy, trade-offs, and failure modes
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .github/workflows/squad-test.yml | Added workflow_call trigger; implemented build artifact caching in build job; added cache restore steps to all 5 test jobs; removed redundant build steps from test jobs |
| .github/workflows/squad-ci.yml | Refactored to thin orchestrator pattern; separated versioning into dedicated job; added test-suite job calling squad-test.yml; simplified notification job |
| .ai-team/decisions/inbox/legolas-build-caching.md | New decision document detailing build caching strategy, rationale, performance impact, safety considerations, and monitoring guidance |
| .ai-team/agents/legolas/history.md | Added learnings about reusable workflow patterns, CI/CD consolidation benefits, and build artifact caching implementation details |
Comments suppressed due to low confidence (8)
.github/workflows/squad-test.yml:73
- The
restore-keyspattern${{ runner.os }}-build-could match stale caches from different dependency versions, potentially causing test failures with mismatched binaries. While this provides graceful degradation, it may lead to confusing failures when dependencies change but the partial cache key matches. Consider either: (1) removing the restore-keys to ensure only exact matches are used (fail fast on cache miss), or (2) documenting that developers should manually clear caches after major dependency changes if they encounter unexpected test failures.
.github/workflows/squad-test.yml:95 - Same issue as in the build job: caching
**/obj/directory may cause portability issues due to absolute paths and intermediate build state. Consider caching only**/bin/Release/for better reliability.
.github/workflows/squad-test.yml:224 - Same issue as in the build job: caching
**/obj/directory may cause portability issues due to absolute paths and intermediate build state. Consider caching only**/bin/Release/for better reliability.
.github/workflows/squad-test.yml:302 - Same issue as in the build job: caching
**/obj/directory may cause portability issues due to absolute paths and intermediate build state. Consider caching only**/bin/Release/for better reliability.
.github/workflows/squad-test.yml:367 - Same issue as in the build job: caching
**/obj/directory may cause portability issues due to absolute paths and intermediate build state. Consider caching only**/bin/Release/for better reliability.
.github/workflows/squad-test.yml:10 - When this workflow is called as a reusable workflow (using the new workflow_call trigger), the
github.workflowcontext variable will reference the caller's workflow name, not "Test Suite". This affects the concurrency group behavior defined at lines 25-27. Consider testing that concurrency control works as expected when squad-ci.yml calls this workflow, or document this behavior difference.
.github/workflows/squad-test.yml:70 - Caching the
**/obj/directory may cause issues because it contains absolute paths and intermediate build state that might not be portable across different workflow runs or checkout states. While cachingbin/Release/is safe, consider excluding**/obj/from the cache and only caching**/bin/Release/. If build performance is significantly impacted, test thoroughly to ensure obj artifacts don't cause intermittent failures.
.github/workflows/squad-test.yml:160 - Same issue as in the build job: caching
**/obj/directory may cause portability issues due to absolute paths and intermediate build state. Consider caching only**/bin/Release/for better reliability.
| test-suite: | ||
| name: Run Test Suite | ||
| needs: versioning | ||
| uses: ./.github/workflows/squad-test.yml@main |
There was a problem hiding this comment.
The test-suite job declares a dependency on the versioning job with needs: versioning, but the reusable workflow doesn't receive or use the version output. If the versioning is intended only for the notification step, consider removing this dependency to allow test-suite and versioning to run in parallel, reducing overall workflow execution time. If the version is needed by the test suite, it should be passed as an input parameter.
| test-suite: | ||
| name: Run Test Suite | ||
| needs: versioning | ||
| uses: ./.github/workflows/squad-test.yml@main |
There was a problem hiding this comment.
The reusable workflow reference uses @main as the ref, which could cause issues if this PR is merged to a different branch or if the workflow needs to be tested in the PR itself before merging. Consider using the dynamic GitHub context instead: uses: ./.github/workflows/squad-test.yml (without the @main suffix) to reference the workflow file from the current branch/commit being executed.
| uses: ./.github/workflows/squad-test.yml@main | |
| uses: ./.github/workflows/squad-test.yml |
| #### Design Decisions Made | ||
|
|
||
| **1. Job Parallelism Retained:** | ||
| - All 6 test jobs (unit, architecture, blazor, integration, aspire, coverage/report) remain parallel |
There was a problem hiding this comment.
Minor documentation inconsistency: The comment states "All 6 test jobs (unit, architecture, blazor, integration, aspire, coverage/report) remain parallel" but coverage and report are aggregation jobs, not test jobs. The workflow actually has 5 test jobs (unit, architecture, bunit, integration, aspire). Consider clarifying this to say "All 5 test jobs plus 2 aggregation jobs (coverage, report)" or simply "All 7 jobs".
| - All 6 test jobs (unit, architecture, blazor, integration, aspire, coverage/report) remain parallel | |
| - All 7 jobs (5 test: unit, architecture, bunit, integration, aspire; 2 aggregation: coverage, report) remain parallel |
Summary