Add list all-tests command for aggregated test listing#31105
Open
petr-muller wants to merge 4 commits intoopenshift:mainfrom
Open
Add list all-tests command for aggregated test listing#31105petr-muller wants to merge 4 commits intoopenshift:mainfrom
petr-muller wants to merge 4 commits intoopenshift:mainfrom
Conversation
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci
Bot
added
the
approved
|
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 (7)
🚧 Files skipped from review as they are similar to previous changes (6)
WalkthroughAdds an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "all-tests Command"
participant Extractor as "ExtractAllTestBinaries"
participant Binaries as "Extension Binaries"
participant Resolver as "Suite Resolver"
participant Output
User->>CLI: invoke all-tests [--suite NAME] [--output FORMAT]
CLI->>Extractor: ExtractAllTestBinaries(parallelism, WithPayloadOnly)
Extractor->>Binaries: enumerate & extract payload binaries (registry auth without cluster)
Binaries-->>Extractor: extracted binaries
CLI->>Binaries: call Info() for each binary (parallel, timeouts)
Binaries-->>CLI: Info or error
alt suite specified
CLI->>Resolver: resolve suite name -> qualifiers (internal then advertised)
Resolver->>Binaries: query advertised suite metadata as needed
Binaries-->>Resolver: suite metadata
Resolver-->>CLI: qualifiers
end
CLI->>CLI: filter aggregated specs by qualifiers, sort by spec.Name
CLI->>Output: format results (newline / json / yaml)
Output-->>User: print results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/cmd/openshift-tests/list/all_tests.go`:
- Around line 106-108: The suite resolution currently calls
resolveSuiteQualifiers(ctx, suiteName, allBinaries) which can re-include
binaries that previously failed Info(); change it to use the filtered set
availableBinaries so the --suite logic skips binaries that already failed.
Update the call site where suiteName is non-empty to pass availableBinaries
instead of allBinaries (referencing suiteName and resolveSuiteQualifiers),
ensuring the rest of the control flow and qualifiers handling remains unchanged.
In `@pkg/test/extensions/util.go`:
- Around line 303-306: The os.Stat check currently treats any non-ENOENT error
as success; change the logic so that only err == nil is treated as a valid file
presence (use ciProfilePullSecretPath and the os.Stat result), and explicitly
surface other non-ENOENT errors back to the caller (e.g., return an error when
err != nil && !os.IsNotExist(err)). Update the block around the os.Stat call
that references ciProfilePullSecretPath so it returns the path on err == nil,
and returns the underlying os.Stat error for other failures instead of assuming
the file is usable.
🪄 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: 383eae15-9442-43a8-8a2c-5508b9225b67
📒 Files selected for processing (6)
pkg/cmd/openshift-tests/list/all_tests.gopkg/cmd/openshift-tests/list/root.gopkg/test/extensions/binary.gopkg/test/extensions/util.gotest/extended/router/config_manager.gotest/extended/router/config_manager_ingress.go
openshift-merge-bot
Bot
added
the
ready-for-human-review
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/test/extensions/payload_compliance_test.go (1)
17-18: ⚡ Quick winReplace placeholder issue links in
knownInfoFailures.Line 17 and Line 18 still point to
issues/XXXX, which makes the exemption list hard to track and clean up. Please replace with real upstream issue IDs (or remove the exemption if no issue exists yet).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/test/extensions/payload_compliance_test.go` around lines 17 - 18, The entries in the knownInfoFailures slice/variable (specifically the items "ovn-kubernetes-tests-ext" and "ccm-aws-tests") still reference placeholder issue URLs (/issues/XXXX); update those entries to point to the actual upstream issue IDs (replace the XXXX with the real issue numbers in the comment URLs) or remove the exemption lines entirely if no real issue exists, so that knownInfoFailures only contains accurate issue links or no stale placeholders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/test/extensions/payload_compliance_test.go`:
- Around line 17-18: The entries in the knownInfoFailures slice/variable
(specifically the items "ovn-kubernetes-tests-ext" and "ccm-aws-tests") still
reference placeholder issue URLs (/issues/XXXX); update those entries to point
to the actual upstream issue IDs (replace the XXXX with the real issue numbers
in the comment URLs) or remove the exemption lines entirely if no real issue
exists, so that knownInfoFailures only contains accurate issue links or no stale
placeholders.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: cf72af05-63b9-4680-bf27-4213df03269c
📒 Files selected for processing (1)
pkg/test/extensions/payload_compliance_test.go
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/extended/extension/payload_compliance.go`:
- Around line 17-20: Update the knownInfoFailures set to use the actual CCM
binary name by replacing the "ccm-aws-tests" entry with
"cloud-controller-manager-aws-tests-ext" so the exemption in the
knownInfoFailures variable matches the value returned by TestBinary.Name() after
ungzipFile extraction.
🪄 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: 9b0ff526-3641-4638-861b-8dd3be76298f
📒 Files selected for processing (2)
pkg/test/extensions/binary.gotest/extended/extension/payload_compliance.go
Move oc.AdminKubeClient() calls from Describe body into BeforeEach in router config manager tests. Ginkgo Describe bodies run during tree building, so deferring cluster access to BeforeEach avoids unnecessary coupling between test discovery and cluster availability. This improves compatibility with workflows that build the test tree without a live cluster connection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add functional options pattern to ExtractAllTestBinaries so callers can skip non-payload extension discovery, which requires cluster access via oc client. This enables listing tests from payload binaries without a cluster connection. Also adds Name() and HasInfo() on TestBinary, and DetermineRegistryAuthFilePathWithoutCluster() for resolving registry auth without a cluster. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a new "openshift-tests list all-tests" subcommand that lists tests from all extension binaries in the release payload. Unlike "list tests" which operates on a single OTE component, this command aggregates tests across all binaries — the same set that "run" operates on. The --suite flag filters tests by a suite's CEL qualifiers, working with both origin-defined suites (like openshift/network/third-party) and suites advertised by extension binaries. This enables validating suite selections without cluster access or --dry-run. Does not require cluster access. Extension binaries that fail info (e.g. due to missing KUBECONFIG) are skipped with a warning. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a ginkgo test that extracts all payload extension binaries and verifies each responds to the OTE "info" command. Binaries with known upstream issues are exempted; the test fails if an exempted binary starts succeeding, prompting removal from the exemption list. Runs as part of openshift/conformance/parallel. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
petr-muller
force-pushed
the
ocpbugs-84257-list-tests-vs-run-tests
branch
from
be64966 to
ee96fdb
Compare
Member
Author
|
/test all |
Member
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: coderabbitai[bot], petr-muller 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 |
Contributor
|
Scheduling required tests: |
Contributor
|
@petr-muller: 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. |
|
Risk analysis has seen new tests most likely introduced by this PR. New Test Risks for sha: ee96fdb
New tests seen in this PR at sha: ee96fdb
|
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
1 participant
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
openshift-tests list all-testssubcommand that lists tests from all extension binaries in the release payload — the same aggregated set thatrunoperates on--suiteflag to filter tests by any suite's CEL qualifiers (origin-defined or extension-advertised), without requiring cluster accessoc.AdminKubeClient()during ginkgo tree buildingMotivation
There is currently no way to see the aggregated test list across all extension binaries, or verify what an origin-defined suite selects, without
run --dry-run(which requires cluster access). This was identified while investigating OCPBUGS-84257 where a broken suite qualifier caused zero tests to be selected.list testsis the OTE contract command — each extension binary lists its own tests. But origin-defined suites likeopenshift/network/third-partyare not visible to individual extension binaries.list all-testsfills this gap at the orchestrator level.Example usage
List all tests from all payload binaries:
Filter by a suite:
Structured output:
Example run against a 5.0 payload
Registry authentication is needed to pull from the CI registry (e.g. via
oc registry loginor
REGISTRY_AUTH_FILE):Known extension binary issues
When running against a real payload, two extension binaries currently fail their
infocall because they unconditionally require KUBECONFIG even for non-cluster commands:ovn-kubernetes-tests-ext): callsgetKubeConfig()andocpinfraprovider.New()unconditionally inmain()for EVPN capability detectionccm-aws-tests): callsinitFrameworkForTests()unconditionally which requires KUBECONFIGlist all-testshandles this gracefully — binaries that failinfoare skipped with a warning and the remaining binaries' tests are listed successfully. Fixes for both upstream repos have been prototyped and tested locally:ocpInfrais nil,shouldIncludeTest()includes all eligible testsinitFrameworkForTests()into cluster-independent defaults (runs at startup) and cluster-dependent init (deferred toAddBeforeAll)These will be submitted as separate PRs to their respective repos.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Tests