test/extend: enable test-extension cases for CI#984
test/extend: enable test-extension cases for CI#984huangmingxia wants to merge 1 commit intoopenshift:masterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdded a new "cco/slow" test suite mapping and adjusted suite qualifiers: parallel now matches Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 inconclusive)
✅ Passed checks (3 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: huangmingxia The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/cc @jstuever |
![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 `@cmd/cloud-credential-tests-ext/main.go`:
- Around line 74-79: The cco/slow suite (Name: "cco/slow", Parents:
[]string{"openshift/optional/slow"}, Qualifiers containing
`name.contains("[Slow]")`) is including tests that are also disruptive (e.g.,
test/extend/cloudcredential.go:63); update the Qualifiers for the "cco/slow"
suite to add a negative qualifier that excludes disruptive tests by combining
the existing matcher with a negative match such as
`!name.contains("[Disruptive]")` (i.e., change the qualifier expression to
require "[Slow]" and not "[Disruptive]") so disruptive tests are routed only to
the disruptive suite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 36d06def-2b6f-4739-939c-497fba25a780
📒 Files selected for processing (2)
cmd/cloud-credential-tests-ext/main.gotest/extend/cloudcredential.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #984 +/- ##
==========================================
- Coverage 46.20% 46.18% -0.03%
==========================================
Files 98 98
Lines 12253 12259 +6
==========================================
Hits 5662 5662
- Misses 5941 5947 +6
Partials 650 650
🚀 New features to boost your workflow:
|
huangmingxia
force-pushed
the
enable-ci-run
branch
from
cf78932 to
21b8c21
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cmd/cloud-credential-tests-ext/main.go (1)
74-80:⚠️ Potential issue | 🟠 MajorExclude disruptive tests from the slow suite.
The
cco/slowsuite currently matches all[Slow]tests without filtering out disruptive ones. The test attest/extend/cloudcredential.go:63is tagged with both[Disruptive][Slow], causing it to be routed to bothopenshift/disruptiveandopenshift/optional/slow. This could cause optional/slow jobs to unexpectedly run destructive operations.Suggested fix
{ Name: "cco/slow", Parents: []string{"openshift/optional/slow"}, Qualifiers: []string{ - `name.contains("[Slow]")`, + `name.contains("[Slow]") && !name.contains("[Disruptive]")`, }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cloud-credential-tests-ext/main.go` around lines 74 - 80, The cco/slow suite currently selects tests by `name.contains("[Slow]")` and therefore also picks up tests that are disruptive; update the Qualifiers for the suite named "cco/slow" so it matches Slow tests but excludes Disruptive ones (e.g., change the qualifier expression used in the Qualifiers slice for the suite to require name.contains("[Slow]") AND not name.contains("[Disruptive]") so tests tagged with both are excluded from cco/slow).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cmd/cloud-credential-tests-ext/main.go`:
- Around line 74-80: The cco/slow suite currently selects tests by
`name.contains("[Slow]")` and therefore also picks up tests that are disruptive;
update the Qualifiers for the suite named "cco/slow" so it matches Slow tests
but excludes Disruptive ones (e.g., change the qualifier expression used in the
Qualifiers slice for the suite to require name.contains("[Slow]") AND not
name.contains("[Disruptive]") so tests tagged with both are excluded from
cco/slow).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 49decdce-79f6-454d-87ec-b3b41bf1cabd
📒 Files selected for processing (2)
cmd/cloud-credential-tests-ext/main.gotest/extend/cloudcredential.go
🚧 Files skipped from review as they are similar to previous changes (1)
- test/extend/cloudcredential.go
huangmingxia
force-pushed
the
enable-ci-run
branch
from
21b8c21 to
6dcf2ca
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cmd/cloud-credential-tests-ext/main.go (1)
67-79:⚠️ Potential issue | 🟠 MajorDestructive test
[Disruptive][Slow]excluded from disruptive suite but included in slow suite.The test at
test/extend/cloudcredential.go:63carries both[Disruptive][Slow]tags and performs destructive operations (removes root credentials). With the current qualifiers:
cco/disruptiveexcludes it (due to!name.contains("[Slow]"))cco/slowincludes it (matches[Slow])This routes a destructive test exclusively to
openshift/optional/slow, bypassing the disruptive job context entirely. This is related to the concern raised previously—applying only half the fix leaves destructive tests in the wrong suite.Both qualifiers need adjustment:
Proposed fix
{ Name: "cco/disruptive", Parents: []string{"openshift/disruptive"}, Qualifiers: []string{ - `name.contains("[Disruptive]") && !name.contains("[Slow]")`, + `name.contains("[Disruptive]")`, }, }, { Name: "cco/slow", Parents: []string{"openshift/optional/slow"}, Qualifiers: []string{ - `name.contains("[Slow]")`, + `name.contains("[Slow]") && !name.contains("[Disruptive]")`, }, },This ensures:
[Disruptive]tests (with or without[Slow]) →cco/disruptive[Slow]tests (without[Disruptive]) →cco/slow🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cloud-credential-tests-ext/main.go` around lines 67 - 79, The current qualifiers cause tests tagged "[Disruptive][Slow]" to be excluded from cco/disruptive and included in cco/slow; update the Qualifiers so cco/disruptive accepts any test with "[Disruptive]" (remove the !name.contains("[Slow]") clause from the cco/disruptive Qualifiers) and make cco/slow only pick up slow tests that are NOT disruptive (add !name.contains("[Disruptive]") to the cco/slow Qualifiers); adjust the Qualifiers in the objects named "cco/disruptive" and "cco/slow" accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cmd/cloud-credential-tests-ext/main.go`:
- Around line 67-79: The current qualifiers cause tests tagged
"[Disruptive][Slow]" to be excluded from cco/disruptive and included in
cco/slow; update the Qualifiers so cco/disruptive accepts any test with
"[Disruptive]" (remove the !name.contains("[Slow]") clause from the
cco/disruptive Qualifiers) and make cco/slow only pick up slow tests that are
NOT disruptive (add !name.contains("[Disruptive]") to the cco/slow Qualifiers);
adjust the Qualifiers in the objects named "cco/disruptive" and "cco/slow"
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ea5068b4-7843-495c-9f8e-ee702906d45d
📒 Files selected for processing (2)
cmd/cloud-credential-tests-ext/main.gotest/extend/cloudcredential.go
🚧 Files skipped from review as they are similar to previous changes (1)
- test/extend/cloudcredential.go
huangmingxia
force-pushed
the
enable-ci-run
branch
from
6dcf2ca to
1903f35
Compare
|
/retest |
|
@huangmingxia: The following test failed, say
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. |
|
/test e2e-hypershift |
1 participant
After the CCO test cases were migrated to the test extension, only Level0 cases were enabled to run in CI. This PR enables the remaining CCO test-extension cases so they are also selected and run in CI.
Summary by CodeRabbit