Skip to content

Commit b3590ea

Browse files
alvaroalemanalvaroaleman
committed
🌱 Stop using context.Background()/TODO() in tests
This change replaces the usage of `context.Background()` and `context.TODO()` in tests with ginkgos `SpecContext` or golangs `t.Context()` in order to ensure contexts are scoped to the test they are for and only valid for the duration of the test.
1 parent 98b5b22 commit b3590ea

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+1607
-1803
lines changed

‎.golangci.yml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ linters:
1717
- errchkjson
1818
- errorlint
1919
- exhaustive
20+
- forbidigo
2021
- ginkgolinter
2122
- goconst
2223
- gocritic
@@ -39,6 +40,12 @@ linters:
3940
- unused
4041
- whitespace
4142
settings:
43+
forbidigo:
44+
forbid:
45+
- pattern: context.Background
46+
msg: Use ginkgos SpecContext or go testings t.Context instead
47+
- pattern: context.TODO
48+
msg: Use ginkgos SpecContext or go testings t.Context instead
4249
govet:
4350
disable:
4451
- fieldalignment
@@ -94,6 +101,9 @@ linters:
94101
- zz_generated.*\.go$
95102
- .*conversion.*\.go$
96103
rules:
104+
- linters:
105+
- forbidigo
106+
path-except: _test\.go
97107
- linters:
98108
- gosec
99109
text: 'G108: Profiling endpoint is automatically exposed on /debug/pprof'

‎pkg/builder/controller_test.go‎

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ var _ = Describe("application", func() {
402402
})
403403

404404
Describe("Start with ControllerManagedBy", func() {
405-
It("should Reconcile Owns objects", func() {
405+
It("should Reconcile Owns objects", func(ctx SpecContext) {
406406
m, err := manager.New(cfg, manager.Options{})
407407
Expect(err).NotTo(HaveOccurred())
408408

@@ -411,12 +411,10 @@ var _ = Describe("application", func() {
411411
Named("deployment-0").
412412
Owns(&appsv1.ReplicaSet{})
413413

414-
ctx, cancel := context.WithCancel(context.Background())
415-
defer cancel()
416414
doReconcileTest(ctx, "3", m, false, bldr)
417415
})
418416

419-
It("should Reconcile Owns objects for every owner", func() {
417+
It("should Reconcile Owns objects for every owner", func(ctx SpecContext) {
420418
m, err := manager.New(cfg, manager.Options{})
421419
Expect(err).NotTo(HaveOccurred())
422420

@@ -425,12 +423,10 @@ var _ = Describe("application", func() {
425423
Named("deployment-1").
426424
Owns(&appsv1.ReplicaSet{}, MatchEveryOwner)
427425

428-
ctx, cancel := context.WithCancel(context.Background())
429-
defer cancel()
430426
doReconcileTest(ctx, "12", m, false, bldr)
431427
})
432428

433-
It("should Reconcile Watches objects", func() {
429+
It("should Reconcile Watches objects", func(ctx SpecContext) {
434430
m, err := manager.New(cfg, manager.Options{})
435431
Expect(err).NotTo(HaveOccurred())
436432

@@ -441,12 +437,10 @@ var _ = Describe("application", func() {
441437
handler.EnqueueRequestForOwner(m.GetScheme(), m.GetRESTMapper(), &appsv1.Deployment{}, handler.OnlyControllerOwner()),
442438
)
443439

444-
ctx, cancel := context.WithCancel(context.Background())
445-
defer cancel()
446440
doReconcileTest(ctx, "4", m, true, bldr)
447441
})
448442

449-
It("should Reconcile without For", func() {
443+
It("should Reconcile without For", func(ctx SpecContext) {
450444
m, err := manager.New(cfg, manager.Options{})
451445
Expect(err).NotTo(HaveOccurred())
452446

@@ -460,14 +454,12 @@ var _ = Describe("application", func() {
460454
handler.EnqueueRequestForOwner(m.GetScheme(), m.GetRESTMapper(), &appsv1.Deployment{}, handler.OnlyControllerOwner()),
461455
)
462456

463-
ctx, cancel := context.WithCancel(context.Background())
464-
defer cancel()
465457
doReconcileTest(ctx, "9", m, true, bldr)
466458
})
467459
})
468460

469461
Describe("Set custom predicates", func() {
470-
It("should execute registered predicates only for assigned kind", func() {
462+
It("should execute registered predicates only for assigned kind", func(ctx SpecContext) {
471463
m, err := manager.New(cfg, manager.Options{})
472464
Expect(err).NotTo(HaveOccurred())
473465

@@ -517,8 +509,6 @@ var _ = Describe("application", func() {
517509
Owns(&appsv1.ReplicaSet{}, WithPredicates(replicaSetPrct)).
518510
WithEventFilter(allPrct)
519511

520-
ctx, cancel := context.WithCancel(context.Background())
521-
defer cancel()
522512
doReconcileTest(ctx, "5", m, true, bldr)
523513

524514
Expect(deployPrctExecuted).To(BeTrue(), "Deploy predicated should be called at least once")
@@ -537,17 +527,14 @@ var _ = Describe("application", func() {
537527
Expect(err).NotTo(HaveOccurred())
538528
})
539529

540-
It("should support multiple controllers watching the same metadata kind", func() {
530+
It("should support multiple controllers watching the same metadata kind", func(ctx SpecContext) {
541531
bldr1 := ControllerManagedBy(mgr).For(&appsv1.Deployment{}, OnlyMetadata).Named("deployment-4")
542532
bldr2 := ControllerManagedBy(mgr).For(&appsv1.Deployment{}, OnlyMetadata).Named("deployment-5")
543533

544-
ctx, cancel := context.WithCancel(context.Background())
545-
defer cancel()
546-
547534
doReconcileTest(ctx, "6", mgr, true, bldr1, bldr2)
548535
})
549536

550-
It("should support watching For, Owns, and Watch as metadata", func() {
537+
It("should support watching For, Owns, and Watch as metadata", func(ctx SpecContext) {
551538
statefulSetMaps := make(chan *metav1.PartialObjectMetadata)
552539

553540
bldr := ControllerManagedBy(mgr).
@@ -571,8 +558,6 @@ var _ = Describe("application", func() {
571558
}),
572559
OnlyMetadata)
573560

574-
ctx, cancel := context.WithCancel(context.Background())
575-
defer cancel()
576561
doReconcileTest(ctx, "8", mgr, true, bldr)
577562

578563
By("Creating a new stateful set")
@@ -601,7 +586,7 @@ var _ = Describe("application", func() {
601586
},
602587
},
603588
}
604-
err := mgr.GetClient().Create(context.TODO(), set)
589+
err := mgr.GetClient().Create(ctx, set)
605590
Expect(err).NotTo(HaveOccurred())
606591

607592
By("Checking that the mapping function has been called")

‎pkg/builder/webhook_test.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ func runTests(admissionReviewVersion string) {
7979
close(stop)
8080
})
8181

82-
It("should scaffold a custom defaulting webhook", func() {
82+
It("should scaffold a custom defaulting webhook", func(specCtx SpecContext) {
8383
By("creating a controller manager")
8484
m, err := manager.New(cfg, manager.Options{})
8585
ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -124,7 +124,7 @@ func runTests(admissionReviewVersion string) {
124124
}
125125
}`)
126126

127-
ctx, cancel := context.WithCancel(context.Background())
127+
ctx, cancel := context.WithCancel(specCtx)
128128
cancel()
129129
err = svr.Start(ctx)
130130
if err != nil && !os.IsNotExist(err) {
@@ -155,7 +155,7 @@ func runTests(admissionReviewVersion string) {
155155
ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
156156
})
157157

158-
It("should scaffold a custom defaulting webhook with a custom path", func() {
158+
It("should scaffold a custom defaulting webhook with a custom path", func(specCtx SpecContext) {
159159
By("creating a controller manager")
160160
m, err := manager.New(cfg, manager.Options{})
161161
ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -202,7 +202,7 @@ func runTests(admissionReviewVersion string) {
202202
}
203203
}`)
204204

205-
ctx, cancel := context.WithCancel(context.Background())
205+
ctx, cancel := context.WithCancel(specCtx)
206206
cancel()
207207
err = svr.Start(ctx)
208208
if err != nil && !os.IsNotExist(err) {
@@ -234,7 +234,7 @@ func runTests(admissionReviewVersion string) {
234234
ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
235235
})
236236

237-
It("should scaffold a custom defaulting webhook which recovers from panics", func() {
237+
It("should scaffold a custom defaulting webhook which recovers from panics", func(specCtx SpecContext) {
238238
By("creating a controller manager")
239239
m, err := manager.New(cfg, manager.Options{})
240240
ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -278,7 +278,7 @@ func runTests(admissionReviewVersion string) {
278278
}
279279
}`)
280280

281-
ctx, cancel := context.WithCancel(context.Background())
281+
ctx, cancel := context.WithCancel(specCtx)
282282
cancel()
283283
err = svr.Start(ctx)
284284
if err != nil && !os.IsNotExist(err) {
@@ -298,7 +298,7 @@ func runTests(admissionReviewVersion string) {
298298
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: fake panic test [recovered]`))
299299
})
300300

301-
It("should scaffold a custom validating webhook", func() {
301+
It("should scaffold a custom validating webhook", func(specCtx SpecContext) {
302302
By("creating a controller manager")
303303
m, err := manager.New(cfg, manager.Options{})
304304
ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -345,7 +345,7 @@ func runTests(admissionReviewVersion string) {
345345
}
346346
}`)
347347

348-
ctx, cancel := context.WithCancel(context.Background())
348+
ctx, cancel := context.WithCancel(specCtx)
349349
cancel()
350350
err = svr.Start(ctx)
351351
if err != nil && !os.IsNotExist(err) {
@@ -375,7 +375,7 @@ func runTests(admissionReviewVersion string) {
375375
EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`))
376376
})
377377

378-
It("should scaffold a custom validating webhook with a custom path", func() {
378+
It("should scaffold a custom validating webhook with a custom path", func(specCtx SpecContext) {
379379
By("creating a controller manager")
380380
m, err := manager.New(cfg, manager.Options{})
381381
ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -424,7 +424,7 @@ func runTests(admissionReviewVersion string) {
424424
}
425425
}`)
426426

427-
ctx, cancel := context.WithCancel(context.Background())
427+
ctx, cancel := context.WithCancel(specCtx)
428428
cancel()
429429
err = svr.Start(ctx)
430430
if err != nil && !os.IsNotExist(err) {
@@ -455,7 +455,7 @@ func runTests(admissionReviewVersion string) {
455455
ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound))
456456
})
457457

458-
It("should scaffold a custom validating webhook which recovers from panics", func() {
458+
It("should scaffold a custom validating webhook which recovers from panics", func(specCtx SpecContext) {
459459
By("creating a controller manager")
460460
m, err := manager.New(cfg, manager.Options{})
461461
ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -497,7 +497,7 @@ func runTests(admissionReviewVersion string) {
497497
}
498498
}`)
499499

500-
ctx, cancel := context.WithCancel(context.Background())
500+
ctx, cancel := context.WithCancel(specCtx)
501501
cancel()
502502
err = svr.Start(ctx)
503503
if err != nil && !os.IsNotExist(err) {
@@ -519,9 +519,9 @@ func runTests(admissionReviewVersion string) {
519519
ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: fake panic test [recovered]`))
520520
})
521521

522-
It("should scaffold a custom validating webhook to validate deletes", func() {
522+
It("should scaffold a custom validating webhook to validate deletes", func(specCtx SpecContext) {
523523
By("creating a controller manager")
524-
ctx, cancel := context.WithCancel(context.Background())
524+
ctx, cancel := context.WithCancel(specCtx)
525525

526526
m, err := manager.New(cfg, manager.Options{})
527527
ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -630,7 +630,7 @@ func runTests(admissionReviewVersion string) {
630630
Expect(err).To(HaveOccurred())
631631
})
632632

633-
It("should scaffold a custom defaulting and validating webhook", func() {
633+
It("should scaffold a custom defaulting and validating webhook", func(specCtx SpecContext) {
634634
By("creating a controller manager")
635635
m, err := manager.New(cfg, manager.Options{})
636636
ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -678,7 +678,7 @@ func runTests(admissionReviewVersion string) {
678678
}
679679
}`)
680680

681-
ctx, cancel := context.WithCancel(context.Background())
681+
ctx, cancel := context.WithCancel(specCtx)
682682
cancel()
683683
err = svr.Start(ctx)
684684
if err != nil && !os.IsNotExist(err) {
@@ -713,7 +713,7 @@ func runTests(admissionReviewVersion string) {
713713
EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaultvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`))
714714
})
715715

716-
It("should scaffold a custom defaulting and validating webhook with a custom path for each of them", func() {
716+
It("should scaffold a custom defaulting and validating webhook with a custom path for each of them", func(specCtx SpecContext) {
717717
By("creating a controller manager")
718718
m, err := manager.New(cfg, manager.Options{})
719719
ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -765,7 +765,7 @@ func runTests(admissionReviewVersion string) {
765765
}
766766
}`)
767767

768-
ctx, cancel := context.WithCancel(context.Background())
768+
ctx, cancel := context.WithCancel(specCtx)
769769
cancel()
770770
err = svr.Start(ctx)
771771
if err != nil && !os.IsNotExist(err) {

0 commit comments

Comments
 (0)