-
Notifications
You must be signed in to change notification settings - Fork 148
Test: replace context.TODO and context.Background to t.Context in uni… #4355
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi @richie-king! Welcome to the project! 🎉 Thanks for opening this pull request! |
|
✅ All required contributors have signed the F5 CLA for this PR. Thank you! |
I have hereby read the F5 CLA and agree to its terms |
|
recheck |
|
I have hereby read the F5 CLA and agree to its terms |
|
recheck |
bjee19
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @richie-king, thanks for this contribution! The changes are looking pretty good, however it looks like the PR is failing a pipeline regarding some lint issues, do you think you could address those?
|
I'm surprised there are only 17 files that needed updating. |
Sure, I will. I feel good about making CI pass |
I've checked all the test files. All the unit tests are included, except for the BDD tests, which do not use the T object. |
…ttests Signed-off-by: richie <mychenforeverl@gmail.com>
Signed-off-by: richie <mychenforeverl@gmail.com>
3c93131 to
40f8633
Compare
|
Some pipeline failures we're seeing on forks, not related to the PR. I'm looking into it. |
Proposed changes
Problem: Unit tests currently use context.TODO() and context.Background(), which can potentially lead to goroutine leaks and resource cleanup issues because these contexts are not automatically canceled when a test completes.
Solution: Replaced usage of context.TODO() and context.Background() with t.Context() in unit tests. This ensures that contexts are automatically canceled when tests complete and aligns the test suite with modern Go 1.24+ testing practices.
Testing: Ran all unit tests to ensure they pass with the new context handling.
Closes #3157
Checklist
Before creating a PR, run through this checklist and mark each as complete.
Release notes
If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.
Refactored unit tests to use
t.Context()instead ofcontext.TODO()orcontext.Background()to prevent potential goroutine leaks.