Skip to content

NO-JIRA: Register the e2e tests for accept in the subfolder#2300

Open
hongkailiu wants to merge 1 commit into
openshift:mainfrom
hongkailiu:add-missing-imports
Open

NO-JIRA: Register the e2e tests for accept in the subfolder#2300
hongkailiu wants to merge 1 commit into
openshift:mainfrom
hongkailiu:add-missing-imports

Conversation

@hongkailiu

@hongkailiu hongkailiu commented Jul 6, 2026

Copy link
Copy Markdown
Member

It was missed in #2294.

With the fix, the generated metadata from the build binary recognizes the tests in the subfolder:

$ CGO_CFLAGS="-I/opt/homebrew/opt/heimdal/include" make build && ./oc-tests-ext update
$ cat .openshift-tests-extension/openshift_payload_oc.json| grep Serial | grep accept
      "name": "[sig-cli][OCPFeatureGate:ClusterUpdateAcceptRisks] oc can operate accept risks [Serial]",
$ rg 'accept risks' ./test/e2e
  ./test/e2e/admin/upgrade/accept/accept.go
  62:     g.It("can operate accept risks [Serial]", g.Label("tech-preview"), func() {

Apparently, my verification there looked at a wrong case.

Pushing the metadata to the repo (cvo does it) may void a mistake like this but it might not be common practice and it might cause other troubles like this one (internal slack conversation). So I am not going there.

Notice that the above test case is not executed in the presubmit oc-ote-serial. However, this is not related to #2294 because it has been like that from day 1 #2170 . It might be caused by [OCPFeatureGate:ClusterUpdateAcceptRisks] in the test name (although I have no proof for it). But I have proof that the case is executed in some other jobs.


Update

It might be caused by [OCPFeatureGate:ClusterUpdateAcceptRisks] in the test name (although I have no proof for it)

The proof came.
#2302
The presubmit https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_oc/2302/pull-ci-openshift-oc-main-e2e-aws-oc-ote-serial/2074471953715957760 comes with

$ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_oc/2302/pull-ci-openshift-oc-main-e2e-aws-oc-ote-serial/2074471953715957760/artifacts/e2e-aws-oc-ote-serial/openshift-e2e-test/artifacts/e2e.log
started: 0/1/2 "[sig-cli] oc can operate accept risks [Serial]"

skip [github.com/openshift/oc/test/e2e/util.go:1506]: This test is skipped because the Tech Preview NoUpgrade is not enabled

skipped: (100ms) 2026-07-07T14:00:32 "[sig-cli] oc can operate accept risks [Serial]"

started: 0/2/2 "[sig-cli] Workloads test oc works well Author:yinzhou-NonPreRelease-Longduration-High-45307-Critical-45327-check oc adm prune deployments to prune RS [Serial][Timeout:30m]"

passed: (1m8s) 2026-07-07T14:01:40 "[sig-cli] Workloads test oc works well Author:yinzhou-NonPreRelease-Longduration-High-45307-Critical-45327-check oc adm prune deployments to prune RS [Serial][Timeout:30m]"

Writing JUnit report to /logs/artifacts/junit/junit_e2e__20260707-135236.xml

Writing extension test results JSON to /logs/artifacts/junit/extension_test_result_e2e__20260707-135236.json
Writing extension test results HTML to /logs/artifacts/junit/extension_test_result_e2e__20260707-135236-summary.html
Writing extension test results HTML to /logs/artifacts/junit/extension_test_result_e2e__20260707-135236-everything.html
1 pass, 0 flaky, 1 skip (9m4s)

Summary by CodeRabbit

  • Tests
    • Included an additional test suite in the test binary so its scenarios are discovered and run automatically.

It was missed in openshift#2294

With the fix, the generated metadata from the build binary recognizes
the tests in the subfolder:

```console
$ CGO_CFLAGS="-I/opt/homebrew/opt/heimdal/include" make build && ./oc-tests-ext update
$ cat .openshift-tests-extension/openshift_payload_oc.json| grep Serial | grep accept
      "name": "[sig-cli][OCPFeatureGate:ClusterUpdateAcceptRisks] oc can operate accept risks [Serial]",
$ rg 'accept risks' ./test/e2e
  ./test/e2e/admin/upgrade/accept/accept.go
  62:     g.It("can operate accept risks [Serial]", g.Label("tech-preview"), func() {
```
@openshift-ci-robot

Copy link
Copy Markdown

@hongkailiu: This pull request explicitly references no jira issue.

Details

In response to this:

It was missed in #2294

With the fix, the generated metadata from the build binary recognizes the tests in the subfolder:

$ CGO_CFLAGS="-I/opt/homebrew/opt/heimdal/include" make build && ./oc-tests-ext update
$ cat .openshift-tests-extension/openshift_payload_oc.json| grep Serial | grep accept
     "name": "[sig-cli][OCPFeatureGate:ClusterUpdateAcceptRisks] oc can operate accept risks [Serial]",
$ rg 'accept risks' ./test/e2e
 ./test/e2e/admin/upgrade/accept/accept.go
 62:     g.It("can operate accept risks [Serial]", g.Label("tech-preview"), func() {

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.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 6, 2026
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 6, 2026
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Walkthrough

A single blank import for the test/e2e/admin/upgrade/accept package was added to cmd/oc-tests-ext/dependencymagnet.go, registering its Ginkgo tests during test binary initialization.

Changes

Test registration import

Layer / File(s) Summary
Add blank import for accept test package
cmd/oc-tests-ext/dependencymagnet.go
Adds a blank import (_) for the admin/upgrade/accept e2e test package to register its tests at startup.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

  • openshift/oc#2294: Restructures/refactors the admin/upgrade/accept test package that this PR's new blank import now registers.

Suggested reviewers: ardaguclu, tchap, wking

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The added blank import registers tests in accept.go, whose Ginkgo titles are static literals with no dynamic interpolation or generated values.
Test Structure And Quality ✅ Passed No Ginkgo test logic was changed; this PR only adds a blank import to register an existing test package, so the test-quality checklist is not applicable.
Microshift Test Compatibility ✅ Passed The added import only registers existing accept tests, and that suite already calls SkipIfMicroShift plus TechPreviewNoUpgrade before using ClusterVersions.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The registered test only exercises ClusterVersion APIs and oc adm upgrade accept; it has MicroShift/TP skips and no multi-node or HA assumptions.
Topology-Aware Scheduling Compatibility ✅ Passed The only change is a blank import in a test-binary helper; no manifests, controllers, replicas, or node scheduling logic were modified.
Ote Binary Stdout Contract ✅ Passed Only change is a blank import to register Ginkgo tests; no stdout writes in main/init/TestMain/suite setup were added.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The newly registered accept e2e test only talks to the cluster API/oc CLI and contains no hardcoded IPv4 or external-internet dependencies.
No-Weak-Crypto ✅ Passed Only change is a blank import of an e2e test package; no crypto code, weak algorithms, or secret comparisons were added.
Container-Privileges ✅ Passed PR only adds a blank import in cmd/oc-tests-ext/dependencymagnet.go; no container/K8s manifests or new privileged settings were changed.
No-Sensitive-Data-In-Logs ✅ Passed Change only adds a blank import; no logging or sensitive data exposure was introduced in the diff.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the change: registering the accept e2e tests from the subfolder.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot requested review from atiratree and tchap July 6, 2026 22:14
@openshift-ci

openshift-ci Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hongkailiu
Once this PR has been reviewed and has the lgtm label, please assign ardaguclu for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
cmd/oc-tests-ext/dependencymagnet.go (1)

1-2: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Update the file header. It still says registration moved to e2e.RegisterTests(), but this package continues to register tests through blank imports. Keep the comment aligned with the actual mechanism.

🤖 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 `@cmd/oc-tests-ext/dependencymagnet.go` around lines 1 - 2, The file header in
dependencymagnet.go is out of sync with how tests are actually registered.
Update the top-of-file comment to reflect the current blank-import based
registration mechanism used by this package, and remove the outdated reference
to e2e.RegisterTests() so the comment matches the actual behavior.
🤖 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.

Outside diff comments:
In `@cmd/oc-tests-ext/dependencymagnet.go`:
- Around line 1-2: The file header in dependencymagnet.go is out of sync with
how tests are actually registered. Update the top-of-file comment to reflect the
current blank-import based registration mechanism used by this package, and
remove the outdated reference to e2e.RegisterTests() so the comment matches the
actual behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: fcaea1c0-3002-47f3-95d4-a6fd333fcd5d

📥 Commits

Reviewing files that changed from the base of the PR and between 439aada and c37aa3d.

📒 Files selected for processing (1)
  • cmd/oc-tests-ext/dependencymagnet.go

@hongkailiu hongkailiu changed the title [wip]NO-JIRA: Register the e2e tests for accept in the subfolder NO-JIRA: Register the e2e tests for accept in the subfolder Jul 7, 2026
@hongkailiu

Copy link
Copy Markdown
Member Author

/test e2e-aws-ovn

@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 7, 2026
@hongkailiu

Copy link
Copy Markdown
Member Author

/verified by @hongkailiu

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jul 7, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@hongkailiu: This PR has been marked as verified by @hongkailiu.

Details

In response to this:

/verified by @hongkailiu

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.

@hongkailiu

Copy link
Copy Markdown
Member Author

/test e2e-aws-ovn

@openshift-ci

openshift-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@hongkailiu: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants