Repository navigation
ci: report coverage once every job has uploaded, and tolerate timing noise - #363
alexander-akait wants to merge 2 commits into
Conversation
|
Merging this PR will not alter performance
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe change adds Codecov branch, notification, coverage precision, and status settings. It also adds a workflow job that runs after the test matrix, unless the workflow is cancelled, and invokes Codecov’s Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly summarizes the main changes: deferred coverage reporting after all jobs upload and tolerance for coverage timing variation. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
4b13d213-17e5-4d6b-8161-af8db2ed42c7
📒 Files selected for processing: 2
.github/workflows/test.ymlcodecov.yml
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
|
|
||
| coverage-notify: | ||
| needs: test | ||
| # A failed matrix job still gets a report from what did upload. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Disable matrix fail-fast before reporting coverage as complete.
If one test matrix job fails, fail-fast: true cancels other running and queued jobs. Those jobs cannot reach their Codecov upload step. coverage-notify can then report a partial result after the matrix ends. Set test.strategy.fail-fast to false so the remaining jobs can finish and upload. (docs.github.com)
🧰 Tools
🪛 zizmor (1.30.1)
[warning] 1-130: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 114-130: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
| uses: codecov/codecov-action@303a32d7a59b442fa8d48b6a1cc6825c09c847a5 # v7.1.1 | ||
| with: | ||
| run_command: send-notifications |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '105,140p' .github/workflows/test.yml
cat codecov.ymlRepository: webpack/watchpack
Length of output: 1520
🌐 Web query:
codecov-action v7.1.1 action.yml fail_ci_if_error default send-notifications
💡 Result:
In **`codecov/codecov-action@v7.1.1`**, `fail_ci_if_error` is an optional input with default **`'false'`**—so errors do not fail the CI job by default. The action’s `run_command` is separately defaulted to **`'upload-coverage'`**; `send-notifications` is listed as an available command, not the default. ([raw.githubusercontent.com](https://raw.githubusercontent.com/codecov/codecov-action/v7.1.1/action.yml))
Citations:
- 1: https://raw.githubusercontent.com/codecov/codecov-action/v7.1.1/action.yml
🌐 Web query:
official Codecov documentation or source for codecov.yml notify.manual_trigger and the codecov-action v7.1.1 send-notifications command behavior when notification sending fails
💡 Result:
- **`manual_trigger`**: Codecov’s official Notifications documentation says to add `codecov: notify: manual_trigger: true` to `codecov.yml` and run `codecov-cli send-notifications` after all coverage uploads. Its purpose is to prevent automatic notifications until that command is run. ([docs.codecov.com](https://docs.codecov.com/docs/notifications?utm_source=openai))
- **Action command**: The Action supports `run_command: send-notifications`; its docs describe that command as an available CLI command. ([github.com](https://github.com/codecov/codecov-action?utm_source=openai))
- **If sending fails**: The Action exposes `fail_ci_if_error`, documented as “On error, exit with non-zero code,” with a default of `false`. Its action definition passes that setting to the wrapper as `CC_FAIL_ON_ERROR`. So the documented behavior is that setting `fail_ci_if_error: true` makes an error fail the step; with the default `false`, the docs do not promise a non-zero exit. ([github.com](https://github.com/codecov/codecov-action/blob/main/action.yml?utm_source=openai))
**Version caveat:** v7.1.1 was released as commit `303a32d`, but the cited Action input documentation is for `main`, not that tag. The sources found don’t establish the exact failure path of `send-notifications` in the v7.1.1 implementation, so whether a notification-send failure is treated as an Action error remains unverified here. ([github.com](https://github.com/codecov/codecov-action/releases?utm_source=openai))
Citations:
- 1: https://docs.codecov.com/docs/notifications?utm_source=openai
- 2: https://github.com/codecov/codecov-action?utm_source=openai
- 3: https://github.com/codecov/codecov-action/blob/main/action.yml?utm_source=openai
- 4: https://github.com/codecov/codecov-action/releases?utm_source=openai
🌐 Web query:
codecov/codecov-action v7.1.1 303a32d7a59b442fa8d48b6a1cc6825c09c847a5 action.yml CC_FAIL_ON_ERROR send-notifications implementation
💡 Result:
For **`codecov/codecov-action@v7.1.1`** at commit `303a32d7a59b442fa8d48b6a1cc6825c09c847a5`:
- **`action.yml`** defines `fail_ci_if_error` (default `false`) and maps it to `CC_FAIL_ON_ERROR`. It also exposes `run_command`, with `send-notifications` as an option; the default command is `upload-coverage`. ([raw.githubusercontent.com](https://raw.githubusercontent.com/codecov/codecov-action/303a32d7a59b442fa8d48b6a1cc6825c09c847a5/action.yml))
- **`CC_FAIL_ON_ERROR` behavior:** In the bundled wrapper, `send-notifications` passes the flag to the CLI as `--fail-on-error` when true. If the CLI command fails, the wrapper exits nonzero only when `CC_FAIL_ON_ERROR=true`; otherwise it logs the error and continues. ([raw.githubusercontent.com](https://raw.githubusercontent.com/codecov/codecov-action/303a32d7a59b442fa8d48b6a1cc6825c09c847a5/dist/codecov.sh))
- **Implementation boundary:** The wrapper invokes the selected CLI command with the token and relevant arguments. The CLI’s notification command calls `upload_completion_logic`, but the CLI source found here is from its current `main` branch—not a CLI version pinned by the action commit—so it does not establish the exact internal notification behavior for the CLI version used in a particular run. ([raw.githubusercontent.com](https://raw.githubusercontent.com/codecov/codecov-action/303a32d7a59b442fa8d48b6a1cc6825c09c847a5/dist/codecov.sh))
The action commit itself identifies as version **7.1.1**. ([raw.githubusercontent.com](https://raw.githubusercontent.com/codecov/codecov-action/303a32d7a59b442fa8d48b6a1cc6825c09c847a5/src/version))
Citations:
- 1: https://raw.githubusercontent.com/codecov/codecov-action/303a32d7a59b442fa8d48b6a1cc6825c09c847a5/action.yml
- 2: https://raw.githubusercontent.com/codecov/codecov-action/303a32d7a59b442fa8d48b6a1cc6825c09c847a5/dist/codecov.sh
- 3: https://raw.githubusercontent.com/codecov/codecov-action/303a32d7a59b442fa8d48b6a1cc6825c09c847a5/dist/codecov.sh
- 4: https://raw.githubusercontent.com/codecov/codecov-action/303a32d7a59b442fa8d48b6a1cc6825c09c847a5/src/version
Fail the notification job when send-notifications fails.
fail_ci_if_error defaults to false. If notification sending fails, manual_trigger: true can leave the coverage status or comment missing or stale while this job remains successful. Add fail_ci_if_error: true to expose the reporting failure.
Suggested change
with:
run_command: send-notifications
+ fail_ci_if_error: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uses: codecov/codecov-action@303a32d7a59b442fa8d48b6a1cc6825c09c847a5 # v7.1.1 | |
| with: | |
| run_command: send-notifications | |
| uses: codecov/codecov-action@303a32d7a59b442fa8d48b6a1cc6825c09c847a5 # v7.1.1 | |
| with: | |
| run_command: send-notifications | |
| fail_ci_if_error: true |
🧰 Tools
🪛 zizmor (1.30.1)
[warning] 1-130: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 114-130: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #363 +/- ##
=======================================
Coverage 92.76% 92.76%
=======================================
Files 8 8
Lines 1409 1409
Branches 423 423
=======================================
Hits 1307 1307
Misses 96 96
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Summary
Coverage comes from 42 matrix jobs (3 OSes × 8 Node versions × native/polling, less 6 macOS legs). Codecov posts its status and comment as uploads arrive, so an early report is built from part of the matrix. On #361
codecov/projectshowed −0.12%. Comparing coverage locally, that came from timing: which branch of a race a watcher test takes depends on the runner, so the same code covers a few lines more or less from run to run.This:
codecov.ymlwithnotify.manual_trigger. The status and comment wait for a newcoverage-notifyjob, which runs once everytestjob has finished (also when one failed, so a report still comes from what did upload). It works the same way as webpack'scoverage-notify.0.5%thresholdon the project status, so timing noise doesn't fail it. A real drop is still reported.What kind of change does this PR introduce?
ci
Did you add tests for your changes?
No, CI configuration only.
npm run lintis clean. This PR's own run shows whether the deferred notification works.Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a
Use of AI
Claude Code traced the coverage dip on #361 to timing noise and wrote the config, following webpack's setup. I reviewed it.
Generated by Claude Code
Summary by CodeRabbit
mainbranch, with manual notification triggering, two-decimal precision, and project and patch status targets based on reported coverage.