Skip to content

🌱 Stop using context.Background()/TODO() in tests #3263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 28, 2025
+1,607 −1,803
Merged

🌱 Stop using context.Background()/TODO() in tests #3263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ linters:
- errchkjson
- errorlint
- exhaustive
- forbidigo
- ginkgolinter
- goconst
- gocritic
Expand All @@ -39,6 +40,12 @@ linters:
- unused
- whitespace
settings:
forbidigo:
forbid:
- pattern: context.Background
msg: Use ginkgos SpecContext or go testings t.Context instead
- pattern: context.TODO
msg: Use ginkgos SpecContext or go testings t.Context instead
govet:
disable:
- fieldalignment
Expand Down Expand Up @@ -94,6 +101,9 @@ linters:
- zz_generated.*\.go$
- .*conversion.*\.go$
rules:
- linters:
- forbidigo
path-except: _test\.go
- linters:
- gosec
text: 'G108: Profiling endpoint is automatically exposed on /debug/pprof'
Expand Down
31 changes: 8 additions & 23 deletions pkg/builder/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ var _ = Describe("application", func() {
})

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

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

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
doReconcileTest(ctx, "3", m, false, bldr)
})

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

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

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
doReconcileTest(ctx, "12", m, false, bldr)
})

It("should Reconcile Watches objects", func() {
It("should Reconcile Watches objects", func(ctx SpecContext) {
m, err := manager.New(cfg, manager.Options{})
Expect(err).NotTo(HaveOccurred())

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

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
doReconcileTest(ctx, "4", m, true, bldr)
})

It("should Reconcile without For", func() {
It("should Reconcile without For", func(ctx SpecContext) {
m, err := manager.New(cfg, manager.Options{})
Expect(err).NotTo(HaveOccurred())

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

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
doReconcileTest(ctx, "9", m, true, bldr)
})
})

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

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

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
doReconcileTest(ctx, "5", m, true, bldr)

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

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

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

doReconcileTest(ctx, "6", mgr, true, bldr1, bldr2)
})

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

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

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
doReconcileTest(ctx, "8", mgr, true, bldr)

By("Creating a new stateful set")
Expand Down Expand Up @@ -601,7 +586,7 @@ var _ = Describe("application", func() {
},
},
}
err := mgr.GetClient().Create(context.TODO(), set)
err := mgr.GetClient().Create(ctx, set)
Expect(err).NotTo(HaveOccurred())

By("Checking that the mapping function has been called")
Expand Down
36 changes: 18 additions & 18 deletions pkg/builder/webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func runTests(admissionReviewVersion string) {
close(stop)
})

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

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

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

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

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

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

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

ctx, cancel := context.WithCancel(context.Background())
ctx, cancel := context.WithCancel(specCtx)
cancel()
err = svr.Start(ctx)
if err != nil && !os.IsNotExist(err) {
Expand Down Expand Up @@ -375,7 +375,7 @@ func runTests(admissionReviewVersion string) {
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"`))
})

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

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

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

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

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

m, err := manager.New(cfg, manager.Options{})
ExpectWithOffset(1, err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -630,7 +630,7 @@ func runTests(admissionReviewVersion string) {
Expect(err).To(HaveOccurred())
})

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

ctx, cancel := context.WithCancel(context.Background())
ctx, cancel := context.WithCancel(specCtx)
cancel()
err = svr.Start(ctx)
if err != nil && !os.IsNotExist(err) {
Expand Down Expand Up @@ -713,7 +713,7 @@ func runTests(admissionReviewVersion string) {
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"`))
})

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

ctx, cancel := context.WithCancel(context.Background())
ctx, cancel := context.WithCancel(specCtx)
cancel()
err = svr.Start(ctx)
if err != nil && !os.IsNotExist(err) {
Expand Down
Loading