Merge pull request kubernetes-sigs#4707 from kersten/chore/resource-tests-use-beforeeach

🌱  (chore): refactor unit tests to isolate mutable state with BeforeEach in resource-related tests

Author: k8s-ci-robot

pkg/model/resource/api_test.go

@@ -116,24 +116,32 @@ var _ = Describe("API", func() {
 
 	Context("IsEmpty", func() {
 		var (
-			none    = API{}
+			none       API
+			cluster    API
+			namespaced API
+		)
+
+		BeforeEach(func() {
+			none = API{}
 			cluster = API{
 				CRDVersion: v1,
 			}
 			namespaced = API{
 				CRDVersion: v1,
 				Namespaced: true,
 			}
-		)
+		})
 
 		It("should return true fo an empty object", func() {
 			Expect(none.IsEmpty()).To(BeTrue())
 		})
 
 		DescribeTable("should return false for non-empty objects",
-			func(api API) { Expect(api.IsEmpty()).To(BeFalse()) },
-			Entry("cluster-scope", cluster),
-			Entry("namespace-scope", namespaced),
+			func(getAPI func() API) {
+				Expect(getAPI().IsEmpty()).To(BeFalse())
+			},
+			Entry("cluster-scope", func() API { return cluster }),
+			Entry("namespace-scope", func() API { return namespaced }),
 		)
 	})
 })

pkg/model/resource/gvk_test.go

@@ -32,13 +32,21 @@ var _ = Describe("GVK", func() {
 		internalVersion = "__internal"
 	)
 
-	gvk := GVK{Group: group, Domain: domain, Version: version, Kind: kind}
+	var gvk GVK
+
+	BeforeEach(func() {
+		gvk = GVK{Group: group, Domain: domain, Version: version, Kind: kind}
+	})
 
 	Context("Validate", func() {
 		DescribeTable("should pass valid GVKs",
-			func(gvk GVK) { Expect(gvk.Validate()).To(Succeed()) },
-			Entry("Standard GVK", gvk),
-			Entry("Version (__internal)", GVK{Group: group, Domain: domain, Version: internalVersion, Kind: kind}),
+			func(get func() GVK) {
+				Expect(get().Validate()).To(Succeed())
+			},
+			Entry("Standard GVK", func() GVK { return gvk }),
+			Entry("Version (__internal)", func() GVK {
+				return GVK{Group: group, Domain: domain, Version: internalVersion, Kind: kind}
+			}),
 		)
 
 		DescribeTable("should fail for invalid GVKs",
@@ -68,10 +76,16 @@ var _ = Describe("GVK", func() {
 
 	Context("QualifiedGroup", func() {
 		DescribeTable("should return the correct string",
-			func(gvk GVK, qualifiedGroup string) { Expect(gvk.QualifiedGroup()).To(Equal(qualifiedGroup)) },
-			Entry("fully qualified resource", gvk, group+"."+domain),
-			Entry("empty group name", GVK{Domain: domain, Version: version, Kind: kind}, domain),
-			Entry("empty domain", GVK{Group: group, Version: version, Kind: kind}, group),
+			func(get func() GVK, qualifiedGroup string) {
+				Expect(get().QualifiedGroup()).To(Equal(qualifiedGroup))
+			},
+			Entry("fully qualified resource", func() GVK { return gvk }, group+"."+domain),
+			Entry("empty group name", func() GVK {
+				return GVK{Domain: domain, Version: version, Kind: kind}
+			}, domain),
+			Entry("empty domain", func() GVK {
+				return GVK{Group: group, Version: version, Kind: kind}
+			}, group),
 		)
 	})
 
@@ -81,11 +95,21 @@ var _ = Describe("GVK", func() {
 		})
 
 		DescribeTable("should return false for different resources",
-			func(other GVK) { Expect(gvk.IsEqualTo(other)).To(BeFalse()) },
-			Entry("different kind", GVK{Group: group, Domain: domain, Version: version, Kind: "Kind2"}),
-			Entry("different version", GVK{Group: group, Domain: domain, Version: "v2", Kind: kind}),
-			Entry("different domain", GVK{Group: group, Domain: "other.domain", Version: version, Kind: kind}),
-			Entry("different group", GVK{Group: "group2", Domain: domain, Version: version, Kind: kind}),
+			func(get func() GVK) {
+				Expect(gvk.IsEqualTo(get())).To(BeFalse())
+			},
+			Entry("different kind", func() GVK {
+				return GVK{Group: group, Domain: domain, Version: version, Kind: "Kind2"}
+			}),
+			Entry("different version", func() GVK {
+				return GVK{Group: group, Domain: domain, Version: "v2", Kind: kind}
+			}),
+			Entry("different domain", func() GVK {
+				return GVK{Group: group, Domain: "other.domain", Version: version, Kind: kind}
+			}),
+			Entry("different group", func() GVK {
+				return GVK{Group: "group2", Domain: domain, Version: version, Kind: kind}
+			}),
 		)
 	})
 })

pkg/model/resource/resource_test.go

@@ -17,6 +17,8 @@ limitations under the License.
 package resource
 
 import (
+	"strings"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
@@ -32,6 +34,11 @@ var _ = Describe("Resource", func() {
 	)
 
 	var (
+		gvk GVK
+		res Resource
+	)
+
+	BeforeEach(func() {
 		gvk = GVK{
 			Group:   group,
 			Domain:  domain,
@@ -42,7 +49,7 @@ var _ = Describe("Resource", func() {
 			GVK:    gvk,
 			Plural: plural,
 		}
-	)
+	})
 
 	Context("Validate", func() {
 		It("should succeed for a valid Resource", func() {
@@ -69,6 +76,13 @@ var _ = Describe("Resource", func() {
 		)
 
 		var (
+			resNoGroup     Resource
+			resNoDomain    Resource
+			resHyphenGroup Resource
+			resDotGroup    Resource
+		)
+
+		BeforeEach(func() {
 			resNoGroup = Resource{
 				GVK: GVK{
 					// Empty group
@@ -101,24 +115,28 @@ var _ = Describe("Resource", func() {
 					Kind:    kind,
 				},
 			}
-		)
+		})
 
 		DescribeTable("PackageName should return the correct string",
-			func(res Resource, packageName string) { Expect(res.PackageName()).To(Equal(packageName)) },
-			Entry("fully qualified resource", res, group),
-			Entry("empty group name", resNoGroup, safeDomain),
-			Entry("empty domain", resNoDomain, group),
-			Entry("hyphen-containing group", resHyphenGroup, safeGroup),
-			Entry("dot-containing group", resDotGroup, safeGroup),
+			func(getRes func() Resource, expected string) {
+				Expect(getRes().PackageName()).To(Equal(expected))
+			},
+			Entry("fully qualified resource", func() Resource { return res }, group),
+			Entry("empty group name", func() Resource { return resNoGroup }, safeDomain),
+			Entry("empty domain", func() Resource { return resNoDomain }, group),
+			Entry("hyphen-containing group", func() Resource { return resHyphenGroup }, safeGroup),
+			Entry("dot-containing group", func() Resource { return resDotGroup }, safeGroup),
 		)
 
 		DescribeTable("ImportAlias",
-			func(res Resource, importAlias string) { Expect(res.ImportAlias()).To(Equal(importAlias)) },
-			Entry("fully qualified resource", res, groupVersion),
-			Entry("empty group name", resNoGroup, domainVersion),
-			Entry("empty domain", resNoDomain, groupVersion),
-			Entry("hyphen-containing group", resHyphenGroup, safeAlias),
-			Entry("dot-containing group", resDotGroup, safeAlias),
+			func(getRes func() Resource, expected string) {
+				Expect(getRes().ImportAlias()).To(Equal(expected))
+			},
+			Entry("fully qualified resource", func() Resource { return res }, groupVersion),
+			Entry("empty group name", func() Resource { return resNoGroup }, domainVersion),
+			Entry("empty domain", func() Resource { return resNoDomain }, groupVersion),
+			Entry("hyphen-containing group", func() Resource { return resHyphenGroup }, safeAlias),
+			Entry("dot-containing group", func() Resource { return resDotGroup }, safeAlias),
 		)
 	})
 
@@ -199,22 +217,24 @@ var _ = Describe("Resource", func() {
 			webhookVersion = "v1"
 		)
 
-		res = Resource{
-			GVK:    gvk,
-			Plural: plural,
-			Path:   path,
-			API: &API{
-				CRDVersion: crdVersion,
-				Namespaced: true,
-			},
-			Controller: true,
-			Webhooks: &Webhooks{
-				WebhookVersion: webhookVersion,
-				Defaulting:     true,
-				Validation:     true,
-				Conversion:     true,
-			},
-		}
+		BeforeEach(func() {
+			res = Resource{
+				GVK:    gvk,
+				Plural: plural,
+				Path:   path,
+				API: &API{
+					CRDVersion: crdVersion,
+					Namespaced: true,
+				},
+				Controller: true,
+				Webhooks: &Webhooks{
+					WebhookVersion: webhookVersion,
+					Defaulting:     true,
+					Validation:     true,
+					Conversion:     true,
+				},
+			}
+		})
 
 		It("should return an exact copy", func() {
 			other := res.Copy()
@@ -423,16 +443,22 @@ var _ = Describe("Resource", func() {
 	})
 
 	Context("Replacer", func() {
-		replacer := res.Replacer()
+		var replacer *strings.Replacer
+
+		BeforeEach(func() {
+			replacer = res.Replacer()
+		})
 
 		DescribeTable("should replace the following strings",
-			func(pattern, result string) { Expect(replacer.Replace(pattern)).To(Equal(result)) },
-			Entry("no pattern", "version", "version"),
-			Entry("pattern `%[group]`", "%[group]", res.Group),
-			Entry("pattern `%[version]`", "%[version]", res.Version),
-			Entry("pattern `%[kind]`", "%[kind]", "kind"),
-			Entry("pattern `%[plural]`", "%[plural]", res.Plural),
-			Entry("pattern `%[package-name]`", "%[package-name]", res.PackageName()),
+			func(pattern string, expected func() string) {
+				Expect(replacer.Replace(pattern)).To(Equal(expected()))
+			},
+			Entry("no pattern", "version", func() string { return "version" }),
+			Entry("pattern `%[group]`", "%[group]", func() string { return res.Group }),
+			Entry("pattern `%[version]`", "%[version]", func() string { return res.Version }),
+			Entry("pattern `%[kind]`", "%[kind]", func() string { return "kind" }),
+			Entry("pattern `%[plural]`", "%[plural]", func() string { return res.Plural }),
+			Entry("pattern `%[package-name]`", "%[package-name]", func() string { return res.PackageName() }),
 		)
 	})
 })

pkg/model/resource/webhooks_test.go

@@ -195,7 +195,20 @@ var _ = Describe("Webhooks", func() {
 
 	Context("IsEmpty", func() {
 		var (
-			none       = Webhooks{}
+			none       Webhooks
+			defaulting Webhooks
+			validation Webhooks
+			conversion Webhooks
+
+			defaultingAndValidation Webhooks
+			defaultingAndConversion Webhooks
+			validationAndConversion Webhooks
+
+			all Webhooks
+		)
+
+		BeforeEach(func() {
+			none = Webhooks{}
 			defaulting = Webhooks{
 				WebhookVersion: "v1",
 				Defaulting:     true,
@@ -238,21 +251,23 @@ var _ = Describe("Webhooks", func() {
 				Validation:     true,
 				Conversion:     true,
 			}
-		)
+		})
 
 		It("should return true fo an empty object", func() {
 			Expect(none.IsEmpty()).To(BeTrue())
 		})
 
 		DescribeTable("should return false for non-empty objects",
-			func(webhooks Webhooks) { Expect(webhooks.IsEmpty()).To(BeFalse()) },
-			Entry("defaulting", defaulting),
-			Entry("validation", validation),
-			Entry("conversion", conversion),
-			Entry("defaulting and validation", defaultingAndValidation),
-			Entry("defaulting and conversion", defaultingAndConversion),
-			Entry("validation and conversion", validationAndConversion),
-			Entry("defaulting and validation and conversion", all),
+			func(get func() Webhooks) {
+				Expect(get().IsEmpty()).To(BeFalse())
+			},
+			Entry("defaulting", func() Webhooks { return defaulting }),
+			Entry("validation", func() Webhooks { return validation }),
+			Entry("conversion", func() Webhooks { return conversion }),
+			Entry("defaulting and validation", func() Webhooks { return defaultingAndValidation }),
+			Entry("defaulting and conversion", func() Webhooks { return defaultingAndConversion }),
+			Entry("validation and conversion", func() Webhooks { return validationAndConversion }),
+			Entry("defaulting and validation and conversion", func() Webhooks { return all }),
 		)
 	})
 })