Skip to content

Commit 10576d0

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4704 from kersten/chore/config-test-var-initialization
🌱 (chore): refactor unit tests to isolate mutable state with BeforeEach in pkg/config
2 parents 8c86374 + 8c3f6c4 commit 10576d0

File tree

4 files changed

+136
-83
lines changed

4 files changed

+136
-83
lines changed

‎pkg/config/errors_test.go

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,13 @@ import (
2626
)
2727

2828
var _ = Describe("UnsupportedVersionError", func() {
29-
err := UnsupportedVersionError{
30-
Version: Version{Number: 1},
31-
}
29+
var err UnsupportedVersionError
30+
31+
BeforeEach(func() {
32+
err = UnsupportedVersionError{
33+
Version: Version{Number: 1},
34+
}
35+
})
3236

3337
Context("Error", func() {
3438
It("should return the correct error message", func() {
@@ -38,10 +42,14 @@ var _ = Describe("UnsupportedVersionError", func() {
3842
})
3943

4044
var _ = Describe("UnsupportedFieldError", func() {
41-
err := UnsupportedFieldError{
42-
Version: Version{Number: 1},
43-
Field: "name",
44-
}
45+
var err UnsupportedFieldError
46+
47+
BeforeEach(func() {
48+
err = UnsupportedFieldError{
49+
Version: Version{Number: 1},
50+
Field: "name",
51+
}
52+
})
4553

4654
Context("Error", func() {
4755
It("should return the correct error message", func() {
@@ -51,14 +59,18 @@ var _ = Describe("UnsupportedFieldError", func() {
5159
})
5260

5361
var _ = Describe("ResourceNotFoundError", func() {
54-
err := ResourceNotFoundError{
55-
GVK: resource.GVK{
56-
Group: "group",
57-
Domain: "my.domain",
58-
Version: "v1",
59-
Kind: "Kind",
60-
},
61-
}
62+
var err ResourceNotFoundError
63+
64+
BeforeEach(func() {
65+
err = ResourceNotFoundError{
66+
GVK: resource.GVK{
67+
Group: "group",
68+
Domain: "my.domain",
69+
Version: "v1",
70+
Kind: "Kind",
71+
},
72+
}
73+
})
6274

6375
Context("Error", func() {
6476
It("should return the correct error message", func() {
@@ -68,9 +80,13 @@ var _ = Describe("ResourceNotFoundError", func() {
6880
})
6981

7082
var _ = Describe("PluginKeyNotFoundError", func() {
71-
err := PluginKeyNotFoundError{
72-
Key: "go.kubebuilder.io/v1",
73-
}
83+
var err PluginKeyNotFoundError
84+
85+
BeforeEach(func() {
86+
err = PluginKeyNotFoundError{
87+
Key: "go.kubebuilder.io/v1",
88+
}
89+
})
7490

7591
Context("Error", func() {
7692
It("should return the correct error message", func() {
@@ -81,10 +97,15 @@ var _ = Describe("PluginKeyNotFoundError", func() {
8197

8298
var _ = Describe("MarshalError", func() {
8399
var (
84-
wrapped = fmt.Errorf("error message")
85-
err = MarshalError{Err: wrapped}
100+
wrapped error
101+
err MarshalError
86102
)
87103

104+
BeforeEach(func() {
105+
wrapped = fmt.Errorf("wrapped error")
106+
err = MarshalError{Err: wrapped}
107+
})
108+
88109
Context("Error", func() {
89110
It("should return the correct error message", func() {
90111
Expect(err.Error()).To(Equal(fmt.Sprintf("error marshalling project configuration: %v", wrapped)))
@@ -100,10 +121,15 @@ var _ = Describe("MarshalError", func() {
100121

101122
var _ = Describe("UnmarshalError", func() {
102123
var (
103-
wrapped = fmt.Errorf("error message")
104-
err = UnmarshalError{Err: wrapped}
124+
wrapped error
125+
err UnmarshalError
105126
)
106127

128+
BeforeEach(func() {
129+
wrapped = fmt.Errorf("wrapped error")
130+
err = UnmarshalError{Err: wrapped}
131+
})
132+
107133
Context("Error", func() {
108134
It("should return the correct error message", func() {
109135
Expect(err.Error()).To(Equal(fmt.Sprintf("error unmarshalling project configuration: %v", wrapped)))

‎pkg/config/registry_test.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,15 @@ import (
2323

2424
var _ = Describe("registry", func() {
2525
var (
26-
version = Version{}
27-
f = func() Config { return nil }
26+
version Version
27+
f func() Config
2828
)
2929

30+
BeforeEach(func() {
31+
version = Version{}
32+
f = func() Config { return nil }
33+
})
34+
3035
AfterEach(func() {
3136
registry = make(map[Version]func() Config)
3237
})

‎pkg/config/v3/config_test.go

Lines changed: 74 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,15 @@ var _ = Describe("Cfg", func() {
4545
)
4646

4747
var (
48-
c Cfg
48+
c Cfg
49+
pluginChain []string
50+
otherPluginChain []string
51+
)
4952

53+
BeforeEach(func() {
5054
pluginChain = []string{"go.kubebuilder.io/v2"}
51-
5255
otherPluginChain = []string{"go.kubebuilder.io/v3"}
53-
)
5456

55-
BeforeEach(func() {
5657
c = Cfg{
5758
Version: Version,
5859
Domain: domain,
@@ -136,6 +137,12 @@ var _ = Describe("Cfg", func() {
136137

137138
Context("Resources", func() {
138139
var (
140+
res resource.Resource
141+
resWithoutPlural resource.Resource
142+
checkResource func(result, expected resource.Resource)
143+
)
144+
145+
BeforeEach(func() {
139146
res = resource.Resource{
140147
GVK: resource.GVK{
141148
Group: "group",
@@ -157,35 +164,35 @@ var _ = Describe("Cfg", func() {
157164
},
158165
}
159166
resWithoutPlural = res.Copy()
160-
)
161167

162-
// As some of the tests insert directly into the slice without using the interface methods,
163-
// regular plural forms should not be present in here. rsWithoutPlural is used for this purpose.
164-
resWithoutPlural.Plural = ""
165-
166-
// Auxiliary function for GetResource, AddResource and UpdateResource tests
167-
checkResource := func(result, expected resource.Resource) {
168-
Expect(result.GVK.IsEqualTo(expected.GVK)).To(BeTrue())
169-
Expect(result.Plural).To(Equal(expected.Plural))
170-
Expect(result.Path).To(Equal(expected.Path))
171-
if expected.API == nil {
172-
Expect(result.API).To(BeNil())
173-
} else {
174-
Expect(result.API).NotTo(BeNil())
175-
Expect(result.API.CRDVersion).To(Equal(expected.API.CRDVersion))
176-
Expect(result.API.Namespaced).To(Equal(expected.API.Namespaced))
177-
}
178-
Expect(result.Controller).To(Equal(expected.Controller))
179-
if expected.Webhooks == nil {
180-
Expect(result.Webhooks).To(BeNil())
181-
} else {
182-
Expect(result.Webhooks).NotTo(BeNil())
183-
Expect(result.Webhooks.WebhookVersion).To(Equal(expected.Webhooks.WebhookVersion))
184-
Expect(result.Webhooks.Defaulting).To(Equal(expected.Webhooks.Defaulting))
185-
Expect(result.Webhooks.Validation).To(Equal(expected.Webhooks.Validation))
186-
Expect(result.Webhooks.Conversion).To(Equal(expected.Webhooks.Conversion))
168+
// As some of the tests insert directly into the slice without using the interface methods,
169+
// regular plural forms should not be present in here. rsWithoutPlural is used for this purpose.
170+
resWithoutPlural.Plural = ""
171+
172+
// Auxiliary function for GetResource, AddResource and UpdateResource tests
173+
checkResource = func(result, expected resource.Resource) {
174+
Expect(result.GVK.IsEqualTo(expected.GVK)).To(BeTrue())
175+
Expect(result.Plural).To(Equal(expected.Plural))
176+
Expect(result.Path).To(Equal(expected.Path))
177+
if expected.API == nil {
178+
Expect(result.API).To(BeNil())
179+
} else {
180+
Expect(result.API).NotTo(BeNil())
181+
Expect(result.API.CRDVersion).To(Equal(expected.API.CRDVersion))
182+
Expect(result.API.Namespaced).To(Equal(expected.API.Namespaced))
183+
}
184+
Expect(result.Controller).To(Equal(expected.Controller))
185+
if expected.Webhooks == nil {
186+
Expect(result.Webhooks).To(BeNil())
187+
} else {
188+
Expect(result.Webhooks).NotTo(BeNil())
189+
Expect(result.Webhooks.WebhookVersion).To(Equal(expected.Webhooks.WebhookVersion))
190+
Expect(result.Webhooks.Defaulting).To(Equal(expected.Webhooks.Defaulting))
191+
Expect(result.Webhooks.Validation).To(Equal(expected.Webhooks.Validation))
192+
Expect(result.Webhooks.Conversion).To(Equal(expected.Webhooks.Conversion))
193+
}
187194
}
188-
}
195+
})
189196

190197
DescribeTable("ResourcesLength should return the number of resources",
191198
func(n int) {
@@ -354,6 +361,11 @@ var _ = Describe("Cfg", func() {
354361
)
355362

356363
var (
364+
c0, c1, c2 Cfg
365+
pluginCfg PluginConfig
366+
)
367+
368+
BeforeEach(func() {
357369
c0 = Cfg{
358370
Version: Version,
359371
Domain: domain,
@@ -386,11 +398,11 @@ var _ = Describe("Cfg", func() {
386398
},
387399
},
388400
}
389-
pluginConfig = PluginConfig{
401+
pluginCfg = PluginConfig{
390402
Data1: "plugin value 1",
391403
Data2: "plugin value 2",
392404
}
393-
)
405+
})
394406

395407
It("DecodePluginConfig should fail for no plugin config object", func() {
396408
var pluginCfg PluginConfig
@@ -407,32 +419,36 @@ var _ = Describe("Cfg", func() {
407419
})
408420

409421
DescribeTable("DecodePluginConfig should retrieve the plugin data correctly",
410-
func(inputConfig Cfg, expectedPluginConfig PluginConfig) {
422+
func(getCfg func() Cfg, expected func() PluginConfig) {
411423
var pluginCfg PluginConfig
412-
Expect(inputConfig.DecodePluginConfig(key, &pluginCfg)).To(Succeed())
413-
Expect(pluginCfg).To(Equal(expectedPluginConfig))
424+
Expect(getCfg().DecodePluginConfig(key, &pluginCfg)).To(Succeed())
425+
Expect(pluginCfg).To(Equal(expected()))
414426
},
415-
Entry("for an empty plugin config object", c1, PluginConfig{}),
416-
Entry("for a full plugin config object", c2, pluginConfig),
427+
Entry("for an empty plugin config object", func() Cfg { return c1 }, func() PluginConfig { return PluginConfig{} }),
428+
Entry("for a full plugin config object", func() Cfg { return c2 }, func() PluginConfig { return pluginCfg }),
417429
// TODO (coverage): add cases where yaml.Marshal returns an error
418430
// TODO (coverage): add cases where yaml.Unmarshal returns an error
419431
)
420432

421433
DescribeTable("EncodePluginConfig should encode the plugin data correctly",
422-
func(pluginConfig PluginConfig, expectedConfig Cfg) {
423-
Expect(c.EncodePluginConfig(key, pluginConfig)).To(Succeed())
424-
Expect(c).To(Equal(expectedConfig))
434+
func(getPluginCfg func() PluginConfig, expectedCfg func() Cfg) {
435+
Expect(c.EncodePluginConfig(key, getPluginCfg())).To(Succeed())
436+
Expect(c).To(Equal(expectedCfg()))
425437
},
426-
Entry("for an empty plugin config object", PluginConfig{}, c1),
427-
Entry("for a full plugin config object", pluginConfig, c2),
438+
Entry("for an empty plugin config object", func() PluginConfig { return PluginConfig{} }, func() Cfg { return c1 }),
439+
Entry("for a full plugin config object", func() PluginConfig { return pluginCfg }, func() Cfg { return c2 }),
428440
// TODO (coverage): add cases where yaml.Marshal returns an error
429441
// TODO (coverage): add cases where yaml.Unmarshal returns an error
430442
)
431443
})
432444

433445
Context("Persistence", func() {
434446
var (
435-
// BeforeEach is called after the entries are evaluated, and therefore, c is not available
447+
c1, c2 Cfg
448+
s1, s1bis, s2 string
449+
)
450+
451+
BeforeEach(func() {
436452
c1 = Cfg{
437453
Version: Version,
438454
Domain: domain,
@@ -472,8 +488,8 @@ var _ = Describe("Cfg", func() {
472488
Kind: "Kind",
473489
},
474490
Plural: "kindes",
475-
API: &resource.API{},
476-
Webhooks: &resource.Webhooks{},
491+
API: nil,
492+
Webhooks: nil,
477493
},
478494
{
479495
GVK: resource.GVK{
@@ -564,22 +580,23 @@ resources:
564580
webhookVersion: v1
565581
version: "3"
566582
`
567-
)
583+
})
568584

569585
DescribeTable("MarshalYAML should succeed",
570-
func(c Cfg, content string) {
571-
b, err := c.MarshalYAML()
586+
func(getCfg func() Cfg, getContent func() string) {
587+
b, err := getCfg().MarshalYAML()
572588
Expect(err).NotTo(HaveOccurred())
573-
Expect(string(b)).To(Equal(content))
589+
Expect(string(b)).To(Equal(getContent()))
574590
},
575-
Entry("for a basic configuration", c1, s1),
576-
Entry("for a full configuration", c2, s2),
591+
Entry("for a basic configuration", func() Cfg { return c1 }, func() string { return s1 }),
592+
Entry("for a full configuration", func() Cfg { return c2 }, func() string { return s2 }),
577593
)
578594

579595
DescribeTable("UnmarshalYAML should succeed",
580-
func(content string, c Cfg) {
596+
func(getContent func() string, getCfg func() Cfg) {
581597
var unmarshalled Cfg
582-
Expect(unmarshalled.UnmarshalYAML([]byte(content))).To(Succeed())
598+
Expect(unmarshalled.UnmarshalYAML([]byte(getContent()))).To(Succeed())
599+
c := getCfg()
583600
Expect(unmarshalled.Version.Compare(c.Version)).To(Equal(0))
584601
Expect(unmarshalled.Domain).To(Equal(c.Domain))
585602
Expect(unmarshalled.Repository).To(Equal(c.Repository))
@@ -590,9 +607,9 @@ version: "3"
590607
Expect(unmarshalled.Plugins).To(HaveLen(len(c.Plugins)))
591608
// TODO: fully test Plugins field and not on its length
592609
},
593-
Entry("basic", s1, c1),
594-
Entry("full", s2, c2),
595-
Entry("string layout", s1bis, c1),
610+
Entry("basic", func() string { return s1 }, func() Cfg { return c1 }),
611+
Entry("full", func() string { return s2 }, func() Cfg { return c2 }),
612+
Entry("string layout", func() string { return s1bis }, func() Cfg { return c1 }),
596613
)
597614

598615
DescribeTable("UnmarshalYAML should fail",

‎pkg/config/version_test.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,13 @@ var _ = Describe("Version", func() {
2929
// Parse, String and Validate are tested by MarshalJSON and UnmarshalJSON
3030

3131
Context("Compare", func() {
32-
// Test Compare() by sorting a list.
3332
var (
33+
versions []Version
34+
sortedVersions []Version
35+
)
36+
37+
BeforeEach(func() {
38+
// Test Compare() by sorting a list.
3439
versions = []Version{
3540
{Number: 2, Stage: stage.Alpha},
3641
{Number: 44, Stage: stage.Alpha},
@@ -56,7 +61,7 @@ var _ = Describe("Version", func() {
5661
{Number: 44, Stage: stage.Alpha},
5762
{Number: 44, Stage: stage.Alpha},
5863
}
59-
)
64+
})
6065

6166
It("sorts a valid list of versions correctly", func() {
6267
sort.Slice(versions, func(i int, j int) bool {

0 commit comments

Comments
 (0)