test: Improve registry+v1 render regression test (#2103)

Author: camilamacedo86

.github/workflows/test-regression.yaml

@@ -0,0 +1,31 @@
+name: test-regression
+
+on:
+  workflow_dispatch:
+  pull_request:
+  merge_group:
+  push:
+    branches:
+    - main
+
+jobs:
+  test-regression:
+    runs-on: ubuntu-latest
+    steps:
+
+    - uses: actions/checkout@v4
+
+    - uses: actions/setup-go@v5
+      with:
+        go-version-file: go.mod
+
+    - name: Run regression tests
+      run: |
+        make test-regression
+
+    - uses: codecov/[email protected]
+      with:
+        disable_search: true
+        files: coverage/regression.out
+        flags: unit
+        token: ${{ secrets.CODECOV_TOKEN }}

.gitignore

@@ -42,3 +42,6 @@ site
 .tiltbuild/
 .catalogd-tmp/
 .vscode
+
+# Tmporary files and directories
+/test/regression/convert/testdata/tmp/*

Makefile

@@ -172,15 +172,9 @@ generate: $(CONTROLLER_GEN) #EXHELP Generate code containing DeepCopy, DeepCopyI
 	$(CONTROLLER_GEN) --load-build-tags=$(GO_BUILD_TAGS) object:headerFile="hack/boilerplate.go.txt" paths="./..."
 
 .PHONY: verify
-verify: k8s-pin kind-verify-versions fmt generate manifests crd-ref-docs generate-test-data #HELP Verify all generated code is up-to-date. Runs k8s-pin instead of just tidy.
+verify: k8s-pin kind-verify-versions fmt generate manifests crd-ref-docs #HELP Verify all generated code is up-to-date. Runs k8s-pin instead of just tidy.
 	git diff --exit-code
 
-# Renders registry+v1 bundles in test/convert
-# Used by CI in verify to catch regressions in the registry+v1 -> plain conversion code
-.PHONY: generate-test-data
-generate-test-data:
-	go run test/convert/generate-manifests.go
-
 .PHONY: fix-lint
 fix-lint: $(GOLANGCI_LINT) #EXHELP Fix lint issues
 	$(GOLANGCI_LINT) run --fix --build-tags $(GO_BUILD_TAGS) $(GOLANGCI_LINT_ARGS)
@@ -209,7 +203,7 @@ verify-crd-compatibility: $(CRD_DIFF) manifests
 #SECTION Test
 
 .PHONY: test
-test: manifests generate fmt lint test-unit test-e2e #HELP Run all tests.
+test: manifests generate fmt lint test-unit test-e2e test-regression #HELP Run all tests.
 
 .PHONY: e2e
 e2e: #EXHELP Run the e2e tests.
@@ -252,6 +246,12 @@ test-unit: $(SETUP_ENVTEST) envtest-k8s-bins #HELP Run the unit tests
                 $(UNIT_TEST_DIRS) \
                 -test.gocoverdir=$(COVERAGE_UNIT_DIR)
 
+COVERAGE_REGRESSION_DIR := $(ROOT_DIR)/coverage/regression
+.PHONY: test-regression
+test-regression: #HELP Run regression test
+	rm -rf $(COVERAGE_REGRESSION_DIR) && mkdir -p $(COVERAGE_REGRESSION_DIR)
+	go test -count=1 -v ./test/regression/... -cover -coverprofile ${ROOT_DIR}/coverage/regression.out -test.gocoverdir=$(COVERAGE_REGRESSION_DIR)
+
 .PHONY: image-registry
 E2E_REGISTRY_IMAGE=localhost/e2e-test-registry:devel
 image-registry: export GOOS=linux

codecov.yml

@@ -1,6 +1,8 @@
 codecov:
   notify:
-    after_n_builds: 3
+    # Configure the 4 builds to wait before sending a notification.
+    # test-unit, test-regression, test-e2e and test-experimental-e2e.
+    after_n_builds: 4
 
 # Configure the paths to include in coverage reports.
 # Exclude documentation, YAML configurations, and test files.

test/convert/README.md

@@ -0,0 +1,120 @@
+/*
+## registry+v1 bundle regression test
+
+This test in convert_test.go verifies that rendering registry+v1 bundles to manifests
+always produces the same files and content.
+
+It runs:     go run generate-manifests.go -output-dir=./testdata/tmp/rendered/
+Then compares: ./testdata/tmp/rendered/ vs ./testdata/expected-manifests/
+
+Files are sorted by kind/namespace/name for consistency.
+
+To update expected output (only on purpose), run:
+
+	go run generate-manifests.go -output-dir=./testdata/expected-manifests/
+*/
+package main
+
+import (
+	"bytes"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/require"
+)
+
+// Test_RenderedOutputMatchesExpected runs generate-manifests.go,
+// then compares the generated files in ./testdata/tmp/rendered/
+// against expected-manifests/.
+// It fails if any file differs or is missing.
+// TMP dir is cleaned up after test ends.
+func Test_RenderedOutput_MatchesExpected(t *testing.T) {
+	tmpRoot := "./testdata/tmp/rendered/"
+	expectedRoot := "./testdata/expected-manifests/"
+
+	// Remove the temporary output directory always
+	t.Cleanup(func() {
+		_ = os.RemoveAll("./testdata/tmp")
+	})
+
+	// Call the generate-manifests.go script to generate the manifests
+	// in the temporary directory.
+	cmd := exec.Command("go", "run", "generate-manifests.go", "-output-dir="+tmpRoot)
+	cmd.Stderr = os.Stderr
+	cmd.Stdout = os.Stdout
+
+	err := cmd.Run()
+	require.NoError(t, err, "failed to generate manifests")
+
+	// Compare structure + contents
+	err = compareDirs(expectedRoot, tmpRoot)
+	require.NoError(t, err, "rendered output differs from expected")
+}
+
+// compareDirs compares expectedRootPath and generatedRootPath directories recursively.
+// It returns an error if any file is missing, extra, or has content mismatch.
+// On mismatch, it includes a detailed diff using cmp.Diff.
+func compareDirs(expectedRootPath, generatedRootPath string) error {
+	// Step 1: Ensure every expected file exists in actual and contents match
+	err := filepath.Walk(expectedRootPath, func(expectedPath string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if info.IsDir() {
+			return nil
+		}
+
+		relPath, err := filepath.Rel(expectedRootPath, expectedPath)
+		if err != nil {
+			return err
+		}
+		actualPath := filepath.Join(generatedRootPath, relPath)
+
+		expectedBytes, err := os.ReadFile(expectedPath)
+		if err != nil {
+			return fmt.Errorf("failed to read expected file: %s", expectedPath)
+		}
+		actualBytes, err := os.ReadFile(actualPath)
+		if err != nil {
+			return fmt.Errorf("missing file: %s", relPath)
+		}
+
+		if !bytes.Equal(expectedBytes, actualBytes) {
+			diff := cmp.Diff(string(expectedBytes), string(actualBytes))
+			return fmt.Errorf("file content mismatch at: %s\nDiff (-expected +actual):\n%s", relPath, diff)
+		}
+		return nil
+	})
+	if err != nil {
+		return err
+	}
+
+	// Step 2: Ensure actual does not contain unexpected files
+	err = filepath.Walk(generatedRootPath, func(actualPath string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if info.IsDir() {
+			return nil
+		}
+
+		relPath, err := filepath.Rel(generatedRootPath, actualPath)
+		if err != nil {
+			return err
+		}
+		expectedPath := filepath.Join(expectedRootPath, relPath)
+
+		_, err = os.Stat(expectedPath)
+		if os.IsNotExist(err) {
+			return fmt.Errorf("unexpected extra file: %s", relPath)
+		} else if err != nil {
+			return fmt.Errorf("error checking expected file: %s", expectedPath)
+		}
+		return nil
+	})
+	return err
+}

test/regression/convert/convert_test.go

@@ -1,7 +1,20 @@
+// generate-manifests.go
+//
+// Renders registry+v1 bundles into YAML manifests for regression testing.
+// Used by tests to make sure output from the BundleRenderer stays consistent.
+//
+// By default, writes to ./testdata/tmp/generate/.
+// To update expected output, run:
+//
+//	go run generate-manifests.go -output-dir=./testdata/expected-manifests/
+//
+// Only re-generate if you intentionally change rendering behavior.
+// Note that if the test fails is likely a regression in the renderer.
 package main
 
 import (
 	"cmp"
+	"flag"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -16,11 +29,26 @@ import (
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1"
 )
 
+// This is a helper for a regression test to make sure the renderer output doesn't change.
+//
+// It renders known bundles into YAML files and writes them to a target output dir.
+// By default, it writes to a temp path used in tests:
+//
+//	./testdata/tmp/rendered/
+//
+// If you want to update the expected output, run it with:
+//
+//	go run generate-manifests.go -output-dir=./testdata/expected-manifests/
+//
+// Note: Expected output should never change unless the renderer changes which is unlikely.
+// If the convert_test.go test fails, it likely means a regression was introduced in the renderer.
 func main() {
 	bundleRootDir := "testdata/bundles/"
-	outputRootDir := "test/convert/expected-manifests/"
+	defaultOutputDir := "./testdata/tmp/rendered/"
+	outputRootDir := flag.String("output-dir", defaultOutputDir, "path to write rendered manifests to")
+	flag.Parse()
 
-	if err := os.RemoveAll(outputRootDir); err != nil {
+	if err := os.RemoveAll(*outputRootDir); err != nil {
 		fmt.Printf("error removing output directory: %v\n", err)
 		os.Exit(1)
 	}
@@ -52,7 +80,7 @@ func main() {
 		},
 	} {
 		bundlePath := filepath.Join(bundleRootDir, tc.bundle)
-		generatedManifestPath := filepath.Join(outputRootDir, tc.bundle, tc.testCaseName)
+		generatedManifestPath := filepath.Join(*outputRootDir, tc.bundle, tc.testCaseName)
 		if err := generateManifests(generatedManifestPath, bundlePath, tc.installNamespace, tc.watchNamespace); err != nil {
 			fmt.Printf("Error generating manifests: %v", err)
 			os.Exit(1)