Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

grpc: retry: add test that exectures parallel requests #59490

Merged

Conversation

ggilmore
Copy link
Contributor

@ggilmore ggilmore commented Jan 10, 2024

During https://github.com/sourcegraph/sourcegraph/pull/59487, we discovered that the existing test suite for the retry package doesn't execute any requests in parallel due to stretchr/testify#187.

This PR explicit adds some basic tests that execute Unary / Streaming requests to see if they trip the race detector.

Test plan

CI

I also opened this example PR to show how these tests would catch the issue in https://github.com/sourcegraph/sourcegraph/pull/59487: https://github.com/sourcegraph/sourcegraph/pull/59491

@cla-bot cla-bot bot added the cla-signed label Jan 10, 2024
Copy link
Contributor Author

ggilmore commented Jan 10, 2024

@github-actions github-actions bot added the team/source Tickets under the purview of Source - the one Source to graph it all label Jan 10, 2024
@ggilmore ggilmore force-pushed the 01-10-grpc_retry_add_test_that_exectures_parallel_requests branch 2 times, most recently from ff9d123 to f4118a3 Compare January 10, 2024 21:05
@ggilmore ggilmore marked this pull request as ready for review January 10, 2024 21:21
@ggilmore ggilmore requested a review from a team January 10, 2024 21:22
internal/grpc/retry/retry_test.go Outdated Show resolved Hide resolved
@ggilmore ggilmore force-pushed the 01-10-grpc_retry_add_test_that_exectures_parallel_requests branch from f4118a3 to c39ec5f Compare January 10, 2024 21:27
@ggilmore ggilmore merged commit 88c6c59 into main Jan 10, 2024
19 checks passed
@ggilmore ggilmore deleted the 01-10-grpc_retry_add_test_that_exectures_parallel_requests branch January 10, 2024 21:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed team/source Tickets under the purview of Source - the one Source to graph it all
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants