Skip to content
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

test: clean up deadlines set in tests #6506

Merged
merged 3 commits into from
Aug 8, 2023
Merged

Conversation

dfawley
Copy link
Member

@dfawley dfawley commented Aug 4, 2023

A straw finally broke the camel's back. Many of our tests don't set timeouts and hang for minutes. This change fixes them, and also standardizes on the use of defaultTest[Short]Timeout everywhere. My goal is to eventually put something like this into vet.sh to make it never happen again:

# Only allow context.Background() to be used with a timeout.
$ find . -name '*_test.go' | xargs grep -n context\.Background | grep -v context.WithTimeout
# Only allow defaultTestTimeout for context.Background() timeouts.
$ find . -name '*_test.go' | xargs grep -n context\.WithTimeout\(context.Background | grep -v defaultTest.*Timeout

There are currently 130 left in our entire tree. Some of these will need to be exempted somehow, or reworked a little to first create a ctx with a timeout and then shorten it from there.

We probably also should look out for selects without a context, bare reads from channels, and any wg.Wait() calls.

RELEASE NOTES: none

@dfawley dfawley added this to the 1.58 Release milestone Aug 4, 2023
@dfawley dfawley merged commit 4832deb into grpc:master Aug 8, 2023
1 check passed
@dfawley dfawley deleted the testDeadlines branch August 8, 2023 16:23
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants