ci: limit coverage to canonical matrix legs#5917
Closed
killagu wants to merge 1 commit into
Closed
Conversation
Contributor
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
Contributor
📝 WalkthroughWalkthroughThe CI workflow now conditions test execution on OS and Node version. Coverage tests ( ChangesTest Execution Conditional Structure
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 6/8 reviews remaining, refill in 13 minutes and 19 seconds.Comment |
Deploying egg-v3 with
|
| Latest commit: |
ac0599f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fa7ae673.egg-v3.pages.dev |
| Branch Preview URL: | https://agent-egg-dev-f5385c80.egg-v3.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5917 +/- ##
==========================================
- Coverage 85.03% 80.61% -4.43%
==========================================
Files 667 667
Lines 19123 19123
Branches 3723 3723
==========================================
- Hits 16262 15416 -846
- Misses 2468 3179 +711
- Partials 393 528 +135 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
213-223: ⚡ Quick winDeduplicate repeated build commands in conditional branches.
ut run build ...is duplicated in both coverage and non-coverage branches in two jobs. Moving build into a single prior step reduces drift and keeps branch logic focused on test mode only.Proposed refactor
- - name: Run tests with coverage - if: ${{ matrix.os == 'ubuntu-latest' }} - run: | - ut run build -- --workspace ./tools/egg-bin - ut run ci --workspace `@eggjs/bin` + - name: Build egg-bin workspace + run: ut run build -- --workspace ./tools/egg-bin + + - name: Run tests with coverage + if: ${{ matrix.os == 'ubuntu-latest' }} + run: ut run ci --workspace `@eggjs/bin` - name: Run tests without coverage if: ${{ matrix.os != 'ubuntu-latest' }} - run: | - ut run build -- --workspace ./tools/egg-bin - ut run test --workspace `@eggjs/bin` + run: ut run test --workspace `@eggjs/bin`- - name: Run tests with coverage - if: ${{ matrix.node == '24' }} - run: | - ut run build -- --workspace ./tools/scripts - ut run ci --workspace tools/scripts + - name: Build scripts workspace + run: ut run build -- --workspace ./tools/scripts + + - name: Run tests with coverage + if: ${{ matrix.node == '24' }} + run: ut run ci --workspace tools/scripts - name: Run tests without coverage if: ${{ matrix.node != '24' }} - run: | - ut run build -- --workspace ./tools/scripts - ut run test --workspace tools/scripts + run: ut run test --workspace tools/scriptsAlso applies to: 260-270
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 213 - 223, The workflow duplicates the build step in both conditional test branches; extract the shared "ut run build -- --workspace ./tools/egg-bin" invocation into a single step run before the per-OS conditional steps (the steps named "Run tests with coverage" and "Run tests without coverage" and their counterparts at lines 260-270), then have those conditional steps only run the test commands ("ut run ci --workspace `@eggjs/bin`" or "ut run test --workspace `@eggjs/bin`") so the build is executed once for all matrix.os values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 213-223: The workflow duplicates the build step in both
conditional test branches; extract the shared "ut run build -- --workspace
./tools/egg-bin" invocation into a single step run before the per-OS conditional
steps (the steps named "Run tests with coverage" and "Run tests without
coverage" and their counterparts at lines 260-270), then have those conditional
steps only run the test commands ("ut run ci --workspace `@eggjs/bin`" or "ut run
test --workspace `@eggjs/bin`") so the build is executed once for all matrix.os
values.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts the CI workflow to keep the OS/Node matrix check names stable while collecting and uploading code coverage only from canonical matrix legs, running non-coverage test correctness on the remaining legs.
Changes:
- Split test execution into “with coverage” (canonical legs) vs “without coverage” (non-canonical legs) across the CI matrix.
- Restrict Codecov uploads to canonical legs to avoid redundant coverage collection while keeping the existing
donegate and other quality checks intact.
Deploying egg with
|
| Latest commit: |
ac0599f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c94f41d5.egg-cci.pages.dev |
| Branch Preview URL: | https://agent-egg-dev-f5385c80.egg-cci.pages.dev |
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.
Summary
Validation
Refs EGG-44.
Summary by CodeRabbit