CNTRLPLANE-244: fix(ci): configure Codecov PR comments and add unit tests#8085
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded two unit tests for supported version functions ( ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
openshift-ci
Bot
added
area/control-plane-operator
bryan-cox
force-pushed
the
test-codecov-integration
branch
from
599899f to
bd4c7cc
Compare
openshift-ci
Bot
added
area/cli
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/util/install.go`:
- Line 57: The global assignment installOpts.ExternalDNSInterval = "3m" slows
reconciliation for all providers; make this Azure-specific by applying the 3m
interval only when the test target is Azure (e.g., guard the assignment with a
provider/platform check such as provider == "azure" or platform.IsAzure());
leave installOpts.ExternalDNSInterval unset (or set to the default) for
non-Azure providers so only Azure e2e suites use the throttling mitigation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: ae285ba7-34a6-43c1-83f5-5ed437c86487
📒 Files selected for processing (3)
cmd/install/assets/hypershift_operator.gocmd/install/assets/hypershift_operator_test.gotest/e2e/util/install.go
| installOpts.ExternalDNSCredentials = opts.ExternalDNSCredentials | ||
| installOpts.ExternalDNSDomainFilter = opts.ExternalDNSDomainFilter | ||
| installOpts.ExternalDNSProvider = opts.ExternalDNSProvider | ||
| installOpts.ExternalDNSInterval = "3m" |
There was a problem hiding this comment.
Scope the 3m external-dns interval to Azure instead of all providers.
Line 57 globally slows reconciliation (--interval=3m) for all e2e providers, but the throttling mitigation is Azure-specific. This can add avoidable runtime to non-Azure suites.
Suggested fix
- installOpts.ExternalDNSInterval = "3m"
+ if opts.ExternalDNSProvider == "azure" {
+ installOpts.ExternalDNSInterval = "3m"
+ }As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
📝 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.
| installOpts.ExternalDNSInterval = "3m" | |
| if opts.ExternalDNSProvider == "azure" { | |
| installOpts.ExternalDNSInterval = "3m" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/util/install.go` at line 57, The global assignment
installOpts.ExternalDNSInterval = "3m" slows reconciliation for all providers;
make this Azure-specific by applying the 3m interval only when the test target
is Azure (e.g., guard the assignment with a provider/platform check such as
provider == "azure" or platform.IsAzure()); leave
installOpts.ExternalDNSInterval unset (or set to the default) for non-Azure
providers so only Azure e2e suites use the throttling mitigation.
3f9e5da to
1fe813d
Compare
bryan-cox
changed the title
bryan-cox
force-pushed
the
test-codecov-integration
branch
from
53bbac0 to
df0b7bb
Compare
|
/retest |
bryan-cox
force-pushed
the
test-codecov-integration
branch
from
df0b7bb to
62d34a5
Compare
bryan-cox
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@bryan-cox: This pull request references CNTRLPLANE-244 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
bryan-cox
force-pushed
the
test-codecov-integration
branch
from
c7ec702 to
41e2a7a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8085 +/- ##
==========================================
+ Coverage 26.56% 26.68% +0.12%
==========================================
Files 1087 1090 +3
Lines 105042 105242 +200
==========================================
+ Hits 27902 28087 +185
+ Misses 74731 74730 -1
- Partials 2409 2425 +16 🚀 New features to boost your workflow:
|
bryan-cox
force-pushed
the
test-codecov-integration
branch
2 times, most recently
from
472ee9d to
d775623
Compare
- Set wait_for_ci: false so Codecov posts PR comments immediately instead of waiting for pending Prow e2e statuses that never resolve - Add per-file coverage diffs to PR comment layout - Add unit tests for String, GetRevision, and GetKubeVersionForSupportedVersion Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
bryan-cox
changed the title
openshift-ci-robot
removed
the
jira/valid-reference
|
@bryan-cox: No Jira issue is referenced in the title of this pull request. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
bryan-cox
force-pushed
the
test-codecov-integration
branch
from
d775623 to
f1b582e
Compare
bryan-cox
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@bryan-cox: This pull request references CNTRLPLANE-244 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
|
/lgtm |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
/verified by @bryan-cox |
openshift-ci-robot
added
the
verified
|
@bryan-cox: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/lgtm |
|
@celebdor: Overrode contexts on behalf of celebdor: ci/prow/e2e-aks, ci/prow/e2e-aws, ci/prow/e2e-aws-upgrade-hypershift-operator, ci/prow/e2e-kubevirt-aws-ovn-reduced, ci/prow/e2e-v2-aws DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@bryan-cox: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
What this PR does / why we need it:
Fixes Codecov PR comments not appearing on PRs that have pending Prow e2e commit statuses.
Codecov's
wait_for_cisetting defaults totrue. From the Codecov YAML Reference, undercodecov.notify:Prow creates
pendingcommit statuses for optional e2e tests (e.g.ci/prow/e2e-aws,ci/prow/e2e-aks) that say "Waiting for pipeline condition to trigger this job". These statuses never resolve unless explicitly triggered via/test, so Codecov waits indefinitely and never posts PR comments or status checks.Evidence:
failuretide— no e2e statuses created (config-only PR)tidetideChanges:
wait_for_ci: false— post PR comments immediately after coverage uploadfilesadded to comment layout — show per-file coverage diffsString(),GetRevision(), andGetKubeVersionForSupportedVersion()Which issue(s) this PR fixes:
Fixes Codecov PR comment integration
Special notes for your reviewer:
The
wait_for_cifix won't take effect on this PR since Codecov readscodecov.ymlfrom the default branch (main). After merge, push to any open PR to verify comments appear.Checklist: