be clever about detecting the generation change and try to detect possible test problems with the test templates

Author: metlos

pkg/template/nstemplatetiers/nstemplatetier_generator_test.go

@@ -284,7 +284,7 @@ func TestGenerateTiers(t *testing.T) {
 			require.Len(t, tierTmpls.Items, 16) // 4 items for advanced and base tiers + 3 for nocluster tier + 4 for appstudio
 			origTemplates := map[string]*toolchainv1alpha1.TierTemplate{}
 			for _, tmpl := range tierTmpls.Items {
-				origTemplates[tmpl.Name] = &tmpl
+				origTemplates[tmpl.Name] = tmpl.DeepCopy()
 			}
 
 			// when calling CreateOrUpdateResources a second time
@@ -299,10 +299,13 @@ func TestGenerateTiers(t *testing.T) {
 			require.Len(t, tierTmpls.Items, 16) // 4 items for advanced and base tiers + 3 for nocluster tier + 4 for appstudio
 			// check that the tier templates are unchanged
 			for _, tierTmpl := range tierTmpls.Items {
-				assert.True(t, reflect.DeepEqual(origTemplates[tierTmpl.Name].Spec, tierTmpl.Spec))
-			}
-
-			// verify that 4 NSTemplateTier CRs were created:
+				// these two should always mean the same thing. If they don't, it means there's an issue with serde of
+				// the templates where template json -> object in cluster -> template json doesn't yield the same thing.
+				// (the json reprentation is used to detect generation change in our test.Client).
+				// This is usually caused by e.g explicitly using a zero value of some property in the template file.
+				assert.True(t, reflect.DeepEqual(origTemplates[tierTmpl.Name].Spec, tierTmpl.Spec), "deep equal different on %T %s", tierTmpl, tierTmpl.Name)
+				assert.Equal(t, int64(1), tierTmpl.Generation, "generation different on %T %s", tierTmpl, tierTmpl.Name) // unchanged
+			} // verify that 4 NSTemplateTier CRs were created:
 			for _, tierName := range []string{"advanced", "base", "nocluster", "appstudio"} {
 				tier := toolchainv1alpha1.NSTemplateTier{}
 				err = clt.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: tierName}, &tier)

pkg/template/nstemplatetiers/testdata/nstemplatetiers/appstudio/cluster.yaml

@@ -19,7 +19,6 @@ objects:
         requests.ephemeral-storage: 7Gi
         count/persistentvolumeclaims: "5"
     selector:
-      annotations: null
       labels:
         matchLabels:
           toolchain.dev.openshift.com/space: ${SPACE_NAME}

pkg/template/nstemplatetiers/testdata/nstemplatetiers/base/cluster.yaml

@@ -17,12 +17,11 @@ objects:
         requests.storage: 7Gi
         persistentvolumeclaims: "5"
     selector:
-      annotations: null
       labels:
         matchLabels:
           toolchain.dev.openshift.com/space: ${SPACE_NAME}
 parameters:
 - name: SPACE_NAME
   required: true
 - name: CPU_LIMIT
-  value: 4000m
+  value: 4000m

pkg/test/client.go

@@ -15,6 +15,7 @@ import (
 	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake" //nolint: staticcheck // not deprecated anymore: see https://github.com/kubernetes-sigs/controller-runtime/pull/1101
+	kjson "sigs.k8s.io/json"
 )
 
 // NewFakeClient creates a fake K8s client with ability to override specific Get/List/Create/Update/StatusUpdate/Delete functions
@@ -201,7 +202,10 @@ func toMap(obj runtime.Object) (map[string]interface{}, error) {
 		return nil, err
 	}
 	m := map[string]interface{}{}
-	if err := json.Unmarshal(content, &m); err != nil {
+	if serr, err := kjson.UnmarshalStrict(content, &m); err != nil || len(serr) > 0 {
+		if err == nil {
+			err = fmt.Errorf("strict unmarshalling errors: %v", serr)
+		}
 		return nil, err
 	}