Update installed package unit tests (#524)

This commit does two things.

First: It splits the `TestMakeInstalledPackageVariables` test into two tests:
* One for when `ForceSemverUpgradeConstraints` feature gate enabled (semver)
* One for when `ForceSemverUpgradeConstraints` feature gate disabled (legacy semantics)

Both tests are now table-style tests.

This is done mainly so that we can maintain test data for different semantics separately (e.g. no need to maintain channel entires for semver tests).

Second: Adds extra coverage for disabled state of the `ForceSemverUpgradeConstraints`
feature gate. Previously we were not covering some cases for this state of the gate.
Example of such case is when `UpgradeConstraintPolicy` field on `Operator` is set to `Ignore`.

Signed-off-by: Mikalai Radchuk <[email protected]>

Author: m1kola

internal/resolution/variablesources/bundle_deployment_test.go

@@ -15,66 +15,10 @@ import (
 	olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
 	"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/util/uuid"
-	"k8s.io/utils/pointer"
-
 	"github.com/operator-framework/deppy/pkg/deppy"
 	rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
 )
 
-func fakeOperator(name, packageName string, upgradeConstraintPolicy operatorsv1alpha1.UpgradeConstraintPolicy) operatorsv1alpha1.Operator {
-	return operatorsv1alpha1.Operator{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: name,
-			// We manually set a fake UID here because the code we test
-			// uses UID to determine Operator CR which
-			// owns `BundleDeployment`
-			UID: uuid.NewUUID(),
-		},
-		Spec: operatorsv1alpha1.OperatorSpec{
-			PackageName:             packageName,
-			UpgradeConstraintPolicy: upgradeConstraintPolicy,
-		},
-	}
-}
-
-func bundleDeployment(name, image string, owner *operatorsv1alpha1.Operator) rukpakv1alpha1.BundleDeployment {
-	bd := rukpakv1alpha1.BundleDeployment{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: name,
-		},
-		Spec: rukpakv1alpha1.BundleDeploymentSpec{
-			ProvisionerClassName: "core-rukpak-io-plain",
-			Template: &rukpakv1alpha1.BundleTemplate{
-				Spec: rukpakv1alpha1.BundleSpec{
-					ProvisionerClassName: "core-rukpak-io-plain",
-					Source: rukpakv1alpha1.BundleSource{
-						Image: &rukpakv1alpha1.ImageSource{
-							Ref: image,
-						},
-					},
-				},
-			},
-		},
-	}
-
-	if owner != nil {
-		bd.SetOwnerReferences([]metav1.OwnerReference{
-			{
-				APIVersion:         operatorsv1alpha1.GroupVersion.String(),
-				Kind:               "Operator",
-				Name:               owner.Name,
-				UID:                owner.UID,
-				Controller:         pointer.Bool(true),
-				BlockOwnerDeletion: pointer.Bool(true),
-			},
-		})
-	}
-
-	return bd
-}
-
 var _ = Describe("BundleDeploymentVariableSource", func() {
 	var betaChannel catalogmetadata.Channel
 	var stableChannel catalogmetadata.Channel
@@ -139,7 +83,7 @@ var _ = Describe("BundleDeploymentVariableSource", func() {
 		fakeOperator := fakeOperator("test-operator", "prometheus", operatorsv1alpha1.UpgradeConstraintPolicyEnforce)
 		operators := []operatorsv1alpha1.Operator{fakeOperator}
 		bundleDeployments := []rukpakv1alpha1.BundleDeployment{
-			bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", &fakeOperator),
+			fakeBundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35", &fakeOperator),
 		}
 
 		bdVariableSource := variablesources.NewBundleDeploymentVariableSource(operators, bundleDeployments, testBundleList, &MockRequiredPackageSource{})
@@ -164,7 +108,7 @@ var _ = Describe("BundleDeploymentVariableSource", func() {
 		fakeOperator := fakeOperator("test-operator", "prometheus", operatorsv1alpha1.UpgradeConstraintPolicyEnforce)
 		operators := []operatorsv1alpha1.Operator{fakeOperator}
 		bundleDeployments := []rukpakv1alpha1.BundleDeployment{
-			bundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:nonexistent", &fakeOperator),
+			fakeBundleDeployment("prometheus", "quay.io/operatorhubio/prometheus@sha256:nonexistent", &fakeOperator),
 		}
 
 		bdVariableSource := variablesources.NewBundleDeploymentVariableSource(operators, bundleDeployments, testBundleList, &MockRequiredPackageSource{})

internal/resolution/variablesources/fake_object_utils_test.go

@@ -0,0 +1,63 @@
+package variablesources_test
+
+import (
+	operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/uuid"
+	"k8s.io/utils/pointer"
+
+	rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
+)
+
+func fakeOperator(name, packageName string, upgradeConstraintPolicy operatorsv1alpha1.UpgradeConstraintPolicy) operatorsv1alpha1.Operator {
+	return operatorsv1alpha1.Operator{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: name,
+			// We manually set a fake UID here because the code we test
+			// uses UID to determine Operator CR which
+			// owns `BundleDeployment`
+			UID: uuid.NewUUID(),
+		},
+		Spec: operatorsv1alpha1.OperatorSpec{
+			PackageName:             packageName,
+			UpgradeConstraintPolicy: upgradeConstraintPolicy,
+		},
+	}
+}
+
+func fakeBundleDeployment(name, bundleImage string, owner *operatorsv1alpha1.Operator) rukpakv1alpha1.BundleDeployment {
+	bd := rukpakv1alpha1.BundleDeployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: name,
+		},
+		Spec: rukpakv1alpha1.BundleDeploymentSpec{
+			ProvisionerClassName: "core-rukpak-io-plain",
+			Template: &rukpakv1alpha1.BundleTemplate{
+				Spec: rukpakv1alpha1.BundleSpec{
+					ProvisionerClassName: "core-rukpak-io-plain",
+					Source: rukpakv1alpha1.BundleSource{
+						Image: &rukpakv1alpha1.ImageSource{
+							Ref: bundleImage,
+						},
+					},
+				},
+			},
+		},
+	}
+
+	if owner != nil {
+		bd.SetOwnerReferences([]metav1.OwnerReference{
+			{
+				APIVersion:         operatorsv1alpha1.GroupVersion.String(),
+				Kind:               "Operator",
+				Name:               owner.Name,
+				UID:                owner.UID,
+				Controller:         pointer.Bool(true),
+				BlockOwnerDeletion: pointer.Bool(true),
+			},
+		})
+	}
+
+	return bd
+}