Skip to content

Test-only tech-debt cleanup: Make TestCtx use stdlib testing.Context() instead of a DIY version #7669

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

Merged
merged 1 commit into from
Aug 7, 2025

Conversation

bbrks
Copy link
Member

@bbrks bbrks commented Aug 7, 2025

This functions very similarly to the existing DIY version we had written prior to 1.24 but uses stdlib functionality now available.

The only difference is potentially the order of cancellation. Previously it was stacked in Cleanup() in the order TestCtx was called, now the stdlib version always cancels prior to running test cleanup. This is better and more predictable behavior anyway, but wasn't something we were previously capable of controlling.

Integration Tests

… function very similarly - just prior to test Cleanup() the test context is cancelled)
@Copilot Copilot AI review requested due to automatic review settings August 7, 2025 12:32
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the TestCtx function to leverage Go's stdlib testing.Context() method instead of a custom implementation using context.WithCancel(). This modernizes the code to use standard library functionality introduced in Go 1.24.

Key changes:

  • Replaced manual context creation and cancellation with testing.TB.Context()
  • Removed explicit cleanup registration since stdlib handles cancellation automatically
  • Improved cancellation timing to occur before test cleanup rather than during it

@bbrks bbrks merged commit 6396718 into main Aug 7, 2025
27 checks passed
@bbrks bbrks deleted the use_new_go_test_context_as_parent branch August 7, 2025 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants