POC - Adding stable suite to build tests#31390
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci
Bot
added
the
do-not-merge/hold
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughAdds built-in ChangesStandard stability suites
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: miyadav 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
openshift-ci
Bot
added
the
ready-for-human-review
|
Scheduling required tests: |
|
Scheduling required tests: |
xueqzhan
left a comment
xueqzhan
left a comment
There was a problem hiding this comment.
We might as well define an empty active suite as well.
| { | ||
| Name: "openshift/stable", | ||
| Description: templates.LongDesc(` | ||
| Umbrella suite for component suites that run under stable cluster conditions. |
There was a problem hiding this comment.
Stable suite here really means all tests in this suite are stable and highly reliable. Of course, the cluster is also stable.
There was a problem hiding this comment.
yes , I will add for it to not have any flaky tests as well.
| Umbrella suite for component suites that run under stable cluster conditions. | ||
| `), | ||
| Qualifiers: []string{ | ||
| "name.contains('[Feature:Builds]')", |
There was a problem hiding this comment.
Do you intent to use qualifiers for all tests in origin (vs having child suites)?
There was a problem hiding this comment.
The problem I was facing is the way openshift/build suite was filtering , so it was having many more tests if I use parent/child way.
There was a problem hiding this comment.
I wonder if we can use OTE labels. Then we need to use another way to label tests. One way is to use AI to add labels to all tests across all repos. Another way is to dynamically label tests based on certain criteria. Method one will not work for kube tests I think.
There was a problem hiding this comment.
right , dynamically adding labels to tests based on data that they are actually stable would be ideal ( But without a test id that won't be easy as names keeps changing )
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/testsuites/standard_suites.go`:
- Around line 117-122: Add an explicit qualifier to the stable suite
configuration near the existing name qualifier so tests marked [Flaky] are
excluded, while preserving the current build-test inclusion and
MaximumAllowedFlakes setting. Update the Qualifiers used by the standard suite
definition.
🪄 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), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 97e5bd78-f638-401f-a3bd-dd3d11c42fb5
📒 Files selected for processing (1)
pkg/testsuites/standard_suites.go
| Qualifiers: []string{ | ||
| "name.contains('[Feature:Builds]')", | ||
| }, | ||
| Parallelism: 30, | ||
| MaximumAllowedFlakes: 0, | ||
| ClusterStabilityDuringTest: ginkgo.Stable, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Exclude flaky tests from the stable suite.
The qualifier still includes build tests marked [Flaky]. MaximumAllowedFlakes: 0 only disables retries; it does not prevent those tests from running, contradicting the suite’s “without flakes” contract. Add an explicit flaky-test exclusion.
This repeats the prior review feedback that the stable suite must not contain flaky tests.
Proposed fix
Qualifiers: []string{
- "name.contains('[Feature:Builds]')",
+ "name.contains('[Feature:Builds]') && !name.contains('[Flaky]')",
},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/testsuites/standard_suites.go` around lines 117 - 122, Add an explicit
qualifier to the stable suite configuration near the existing name qualifier so
tests marked [Flaky] are excluded, while preserving the current build-test
inclusion and MaximumAllowedFlakes setting. Update the Qualifiers used by the
standard suite definition.
|
@miyadav: The following tests 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. |
2 participants
/hold
Summary by CodeRabbit
openshift/stableandopenshift/active.openshift/stableandopenshift/activerun with high parallelism under stable cluster conditions and select tests using label-based qualifiers forSTABLEandACTIVE, respectively.openshift/stablealso disables allowed flake retries to keep results strict.