🌱 [Monorepo]: Move e2e tests from catalogd to operator-controller #1726

camilamacedo86 · 2025-02-07T01:30:11Z

Motivated by: #1712

netlify · 2025-02-07T01:30:29Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`f0c952e`
🔍 Latest deploy log	https://app.netlify.com/sites/olmv1/deploys/67aa2b644dc76e0008f17b0f
😎 Deploy Preview	https://deploy-preview-1726--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov · 2025-02-07T01:50:25Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.22%. Comparing base (ae41bfc) to head (5737736).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1726      +/-   ##
==========================================
+ Coverage   68.02%   68.22%   +0.19%     
==========================================
  Files          59       58       -1     
  Lines        5004     4988      -16     
==========================================
- Hits         3404     3403       -1     
+ Misses       1358     1343      -15     
  Partials      242      242

Flag	Coverage Δ
e2e	`52.84% <ø> (-0.09%)`	⬇️
unit	`55.43% <ø> (+0.15%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

camilamacedo86 · 2025-02-07T11:04:16Z

test/catalogd-e2e/unpack_test.go

@@ -79,7 +79,7 @@ var _ = Describe("ClusterCatalog Unpacking", func() {
 			actualFBC, err := ReadTestCatalogServerContents(ctx, catalog, c, kubeClient)
 			Expect(err).To(Not(HaveOccurred()))

-			expectedFBC, err := os.ReadFile("../../testdata/catalogs/test-catalog/expected_all.json")
+			expectedFBC, err := os.ReadFile("../../catalogd/testdata/catalogs/test-catalog/expected_all.json")


We cannot move now the testdata/catalogs to the root because it will broke the sync:

https://github.com/openshift/release/blob/master/ci-operator/config/openshift/operator-framework-operator-controller/openshift-operator-framework-operator-controller-main.yaml#L26-L27

LalatenduMohanty · 2025-02-07T16:20:20Z

catalogd/Makefile


 image-registry: ## Setup in-cluster image registry
-	./test/tools/imageregistry/registry.sh $(ISSUER_KIND) $(ISSUER_NAME)
+	./../test/tools/imageregistry/registry.sh $(ISSUER_KIND) $(ISSUER_NAME)


Do we need this in both Makefiles (i.e. operator-controller and catalogd)?

In a follow-up, we can probably combine both. But for now, I felt better keep in this away because they are not the same code implementation.

See

.PHONY: image-registry E2E_REGISTRY_IMAGE=localhost/e2e-test-registry:devel image-registry: export GOOS=linux image-registry: export GOARCH=amd64 image-registry: ## Build the testdata catalog used for e2e tests and push it to the image registry go build $(GO_BUILD_FLAGS) -tags '$(GO_BUILD_TAGS)' -ldflags '$(GO_BUILD_LDFLAGS)' -gcflags '$(GO_BUILD_GCFLAGS)' -asmflags '$(GO_BUILD_ASMFLAGS)' -o ./testdata/registry/bin/registry ./testdata/registry/registry.go go build $(GO_BUILD_FLAGS) -tags '$(GO_BUILD_TAGS)' -ldflags '$(GO_BUILD_LDFLAGS)' -gcflags '$(GO_BUILD_GCFLAGS)' -asmflags '$(GO_BUILD_ASMFLAGS)' -o ./testdata/push/bin/push ./testdata/push/push.go $(CONTAINER_RUNTIME) build -f ./testdata/Dockerfile -t $(E2E_REGISTRY_IMAGE) ./testdata $(CONTAINER_RUNTIME) save $(E2E_REGISTRY_IMAGE) | $(KIND) load image-archive /dev/stdin --name $(KIND_CLUSTER_NAME) ./testdata/build-test-registry.sh $(E2E_REGISTRY_NAMESPACE) $(E2E_REGISTRY_NAME) $(E2E_REGISTRY_IMAGE) <- Call another implementation

catalogd-image-registry: ## Setup in-cluster image registry ./test/tools/imageregistry/registry.sh $(ISSUER_KIND) $(ISSUER_NAME)

Makefile

dtfranz · 2025-02-07T18:59:32Z

Makefile

@@ -209,6 +213,39 @@ test-e2e: KUSTOMIZE_BUILD_DIR := config/overlays/e2e
 test-e2e: GO_BUILD_FLAGS := -cover
 test-e2e: run image-registry e2e e2e-coverage kind-clean #HELP Run e2e test suite on local kind cluster


If I'm reading the Makefile correctly, we have:
test-e2e - runs operator-controller e2e only
catalogd-e2e - runs catalogd e2e only

I think we need some more clear separation between the e2e for the two components. I'd prefer this as a follow-up if we do change it, but IMO we should have:
test-e2e - runs both operator-controller and catalogd e2e
catalogd-e2e - runscatalogd e2e only
operator-controller-e2e - runs operator-controller e2e only

We could then have the github actions run the catalogd-e2e and operator-controller-e2e in two separate jobs as it does now, but with slightly more clear job names. WDYT?

Again though, don't do that on this PR if you agree, I think this one is big enough already :)

HI @dtfranz

I also want :-) That is what I expected we have at some point:

test-e2e - runs both operator-controller and catalogd e2e
upgrade-e2e runs for both

But that needs to be follow ups.
We need to do step by step

We need change the tests to no longer use Omega Ginkgo

Then, we can start to began to combine

We will need to combine as well the image registry and going on

We need to split. Otherwise, it is too much for a single go

dtfranz

Left a comment on the Makefile above but otherwise /lgtm!

perdasilva

The PR takes another step in the direction of merging operator-controller and catalogd by moving the catalogd tools and e2e tests under the top-level test and tools directories. Only the required changes to the catalogd Makefile and e2e GHA were made in order to ensure the tests are still being correctly executed by CI. Follow-ups will continue the work of merging the projects.

Awesome work <3 thank you!

grokspawn · 2025-02-10T16:25:34Z

apidiff failure is due to moving an e2e test, unavoidable as we merge the directory hierarchies.
/lgtm
we should merge it anyway.

…d cleanup makefile

…ger used

camilamacedo86 · 2025-02-10T16:38:11Z

@grokspawn rebased with main :-)

camilamacedo86 requested a review from a team as a code owner February 7, 2025 01:30

camilamacedo86 force-pushed the catalogd-e2e branch from d956d77 to dcd88ca Compare February 7, 2025 01:36

camilamacedo86 changed the title ~~🌱 [Monorepo]: Move e2e tests from catalogd to operator-controller~~ WIP: 🌱 [Monorepo]: Move e2e tests from catalogd to operator-controller Feb 7, 2025

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 Feb 7, 2025

camilamacedo86 force-pushed the catalogd-e2e branch from b3d3c79 to 2ab4f40 Compare February 7, 2025 10:57

camilamacedo86 commented Feb 7, 2025

View reviewed changes

camilamacedo86 force-pushed the catalogd-e2e branch from 2ab4f40 to 9e8f2d5 Compare February 7, 2025 11:05

camilamacedo86 changed the title ~~WIP: 🌱 [Monorepo]: Move e2e tests from catalogd to operator-controller~~ 🌱 [Monorepo]: Move e2e tests from catalogd to operator-controller Feb 7, 2025

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 Feb 7, 2025

camilamacedo86 enabled auto-merge February 7, 2025 11:12

camilamacedo86 disabled auto-merge February 7, 2025 11:12

camilamacedo86 force-pushed the catalogd-e2e branch from 448c0ee to 38d9bae Compare February 7, 2025 11:44

LalatenduMohanty reviewed Feb 7, 2025

View reviewed changes

Makefile Show resolved Hide resolved

camilamacedo86 requested a review from LalatenduMohanty February 7, 2025 17:14

camilamacedo86 commented Feb 7, 2025

View reviewed changes

Makefile Show resolved Hide resolved

dtfranz reviewed Feb 7, 2025

View reviewed changes

dtfranz previously approved these changes Feb 7, 2025

View reviewed changes

perdasilva previously approved these changes Feb 10, 2025

View reviewed changes

perdasilva enabled auto-merge February 10, 2025 16:02

camilamacedo86 disabled auto-merge February 10, 2025 16:07

camilamacedo86 enabled auto-merge February 10, 2025 16:07

camilamacedo86 added this pull request to the merge queue Feb 10, 2025

camilamacedo86 added 3 commits February 10, 2025 16:37

[Monorepo]: Move e2e tests from catalogd to operator-controller

4679fed

[Monorepo]: Move catalogd upgrade e2e tests to operator-controller an…

773d4a7

…d cleanup makefile

[Monorepo]: Delete catalogd/scripts/install.tpl.sh since it is no lon…

f0c952e

…ger used

camilamacedo86 dismissed perdasilva’s stale review via f0c952e February 10, 2025 16:37

camilamacedo86 dismissed dtfranz’s stale review via f0c952e February 10, 2025 16:37

camilamacedo86 force-pushed the catalogd-e2e branch from 5737736 to f0c952e Compare February 10, 2025 16:37

auto-merge was automatically disabled February 10, 2025 16:42
Pull Request is not mergeable

Merged via the queue into operator-framework:main with commit 4a007b0 Feb 10, 2025
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

🌱 [Monorepo]: Move e2e tests from catalogd to operator-controller #1726

🌱 [Monorepo]: Move e2e tests from catalogd to operator-controller #1726

Uh oh!

camilamacedo86 commented Feb 7, 2025 •

edited

Loading

Uh oh!

netlify bot commented Feb 7, 2025 •

edited

Loading

Uh oh!

codecov bot commented Feb 7, 2025 •

edited

Loading

Uh oh!

camilamacedo86 Feb 7, 2025

Uh oh!

LalatenduMohanty Feb 7, 2025

Uh oh!

camilamacedo86 Feb 7, 2025

Uh oh!

Uh oh!

Uh oh!

dtfranz Feb 7, 2025

Uh oh!

camilamacedo86 Feb 7, 2025

Uh oh!

dtfranz left a comment

Uh oh!

perdasilva left a comment

Uh oh!

grokspawn commented Feb 10, 2025 •

edited

Loading

Uh oh!

camilamacedo86 commented Feb 10, 2025

Uh oh!

Uh oh!

Uh oh!

		@@ -209,6 +213,39 @@ test-e2e: KUSTOMIZE_BUILD_DIR := config/overlays/e2e
		test-e2e: GO_BUILD_FLAGS := -cover
		test-e2e: run image-registry e2e e2e-coverage kind-clean #HELP Run e2e test suite on local kind cluster

🌱 [Monorepo]: Move e2e tests from catalogd to operator-controller #1726

🌱 [Monorepo]: Move e2e tests from catalogd to operator-controller #1726

Uh oh!

Conversation

camilamacedo86 commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

codecov bot commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

camilamacedo86 Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

LalatenduMohanty Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dtfranz Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

dtfranz left a comment

Choose a reason for hiding this comment

Uh oh!

perdasilva left a comment

Choose a reason for hiding this comment

Uh oh!

grokspawn commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

camilamacedo86 commented Feb 10, 2025

Uh oh!

Uh oh!

Uh oh!

camilamacedo86 commented Feb 7, 2025 •

edited

Loading

netlify bot commented Feb 7, 2025 •

edited

Loading

codecov bot commented Feb 7, 2025 •

edited

Loading

grokspawn commented Feb 10, 2025 •

edited

Loading