-
Notifications
You must be signed in to change notification settings - Fork 35
OCPQE-30673: migrate some clustercatalog cases to ote #525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@Xia-Zhao-rh: This pull request references OCPQE-30673 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 sub-task to target the "4.21.0" version, but no target version was set. In 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Xia-Zhao-rh The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
please run the cases with |
|
@Xia-Zhao-rh: This pull request references OCPQE-30673 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 sub-task to target the "4.21.0" version, but no target version was set. In 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. |
|
/payload-aggregate periodic-ci-openshift-release-master-ci-4.21-e2e-gcp-ovn-techpreview 5 |
|
@Xia-Zhao-rh: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/3ad72d70-aa63-11f0-88de-a85375f57e42-0 |
|
|
/hold |
|
/test verify-commits |
|
/retest verify-commits |
|
@tmshort: The The following commands are available to trigger optional jobs: Use In 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. |
|
Interesting... verify-commits is passing‽‽ |
|
/test verify-commits |
1 similar comment
|
/test verify-commits |
|
|
||
| }) | ||
|
|
||
| g.It("PolarionID:69123-[Skipped:Disconnected]Catalogd clustercatalog offer the operator content through http server", g.Label("LEVEL0"), func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this "PolarionID:69123"
Those tests are not sending a signal to the Sippy
Also, I understand that they are not using the old system anymore
So, could we remove it and keep the names meaningful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we adding g.Label("LEVEL0") ?
Are we setting this one not to run those and produce data for Sippy?
| var ( | ||
| baseDir = exutil.FixturePath("testdata", "olm") | ||
| clustercatalogTemplate = filepath.Join(baseDir, "clustercatalog.yaml") | ||
| clustercatalog = olmv1util.ClusterCatalogDescription{ | ||
| Name: "clustercatalog-69123", | ||
| Imageref: "quay.io/olmqe/olmtest-operator-index:nginxolm69123", | ||
| Template: clustercatalogTemplate, | ||
| } | ||
| ) | ||
| g.By("Create clustercatalog") | ||
| defer clustercatalog.Delete(oc) | ||
| clustercatalog.Create(oc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| var ( | |
| baseDir = exutil.FixturePath("testdata", "olm") | |
| clustercatalogTemplate = filepath.Join(baseDir, "clustercatalog.yaml") | |
| clustercatalog = olmv1util.ClusterCatalogDescription{ | |
| Name: "clustercatalog-69123", | |
| Imageref: "quay.io/olmqe/olmtest-operator-index:nginxolm69123", | |
| Template: clustercatalogTemplate, | |
| } | |
| ) | |
| g.By("Create clustercatalog") | |
| defer clustercatalog.Delete(oc) | |
| clustercatalog.Create(oc) | |
| unique := rand.String(4) | |
| catName := "catalog-" + unique | |
| imageRef := "quay.io/olmqe/olmtest-operator-index:nginxolm69123" | |
| By("creating catalog with ....") // WHAT 69123 means? | |
| cleanup, err := helpers.CreateClusterCatalog(ctx, catName, imageRef) | |
| Expect(err).NotTo(HaveOccurred(), "failed to create ClusterCatalog") | |
| DeferCleanup(cleanup) |
I noticed we already have helpers to create and clean up Cluster Catalogues with an image: https://github.com/openshift/operator-framework-operator-controller/blob/main/openshift/tests-extension/pkg/helpers/cluster_catalog.go#L24-L52
Could we please use those helpers here instead?
It’ll help reduce extra dependencies and make the code easier to maintain long term.
(When we spread similar logic in different places, it gets harder to keep things consistent.)
Example for reference:
🔗
operator-framework-operator-controller/openshift/tests-extension/test/olmv1-catalog.go
Lines 205 to 213 in 0e8f138
| It("should fail to install if it has an invalid reference", func(ctx SpecContext) { | |
| unique := rand.String(4) | |
| catName := "bad-catalog-" + unique | |
| imageRef := "example.com/does-not-exist:latest" | |
| By("creating the malformed catalog with an invalid image ref") | |
| cleanup, err := helpers.CreateClusterCatalog(ctx, catName, imageRef) | |
| Expect(err).NotTo(HaveOccurred(), "failed to create ClusterCatalog") | |
| DeferCleanup(cleanup) |
Thanks a lot 🙏
| o.Expect(channelData[0].Name).To(o.ContainSubstring("candidate-v0.0")) | ||
|
|
||
| bundlesName := olmv1util.GetBundlesNameByPakcage(unmarshalContent.Bundles, "nginx69123") | ||
| o.Expect(bundlesName[0]).To(o.ContainSubstring("nginx69123.v0.0.1")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tests is not using OLM at all.
What it is testing out?
What is the goal of this tests?
|
Hi @Xia-Zhao-rh, You can ignore my comments on this PR — I understand you’re consolidating everything under the QE directory now.
Anyway, this is probably already part of your plan — I’m just putting it into words. 😊 |
|
Hi, @camilamacedo86 |
|
/test verify-commits |
|
/test verify-commits |
|
/test verify-commits |
5596607 to
54cedd7
Compare
|
/retest |
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
54cedd7 to
8263114
Compare
|
/retest |
|
@Xia-Zhao-rh: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
This commit migrates 15 clustercatalog-related test cases from openshift-tests-private to the OpenShift Tests Extension (OTE) framework, following the migration guidelines in test/qe/README.md.
Files Modified:
Testing:
Assisted-by: Claude Code