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

Integration tests + improved thread safety #15

Merged
merged 2 commits into from
Oct 10, 2023
Merged

Conversation

reidbuzby
Copy link
Contributor

Adds integration testing against example projects in both kotlin and java. Included is a test that attempts to send 1000 requests at the same time to the example backend (not all 1000 actually get sent at once due to the concurrency limitation on the OkHttpClient) in order to test the concurrency and thread safety of the package.

This test highlighted an issue with the old withKonigKontext function in the kotlin implementation. Since gRPC Context utilizes ThreadLocal, mutliple Kotlin coroutines can run on the same thread and potentially have access to the wrong gRPC Context. The grpc-kotlin package ran into this issue as well, and implemented a custom CoroutineContext element specifically for ensuring that a given coroutine only can access its proper corresponding gRPC Context. I have utilized the same class to ensure that withKonigKontext is concurrency safe. Also, since grpc-kotlin has already solved this issue, any kotlin coroutine server implementations that register the KonigKontextInterceptor will also be concurrency safe. With these changes, all kotlin implementations should be concurrency safe.

All Java implementations were already concurrency safe since concurrent threads in Java are each their own thread and thus cannot have access to an incorrect gRPC Context.

Grpc-kotlin also has example projects inside the same repo as the library itself, so I'm going to use that as an example to try and see if I can get those projects moved into this repo. That way any breaking changes will require updating the examples and integration tests can be run on PRs

@reidbuzby reidbuzby force-pushed the reid/integration-tests branch 8 times, most recently from 0094a42 to bda5c20 Compare October 9, 2023 00:08
@reidbuzby reidbuzby linked an issue Oct 9, 2023 that may be closed by this pull request
@reidbuzby reidbuzby force-pushed the reid/integration-tests branch 2 times, most recently from ef1aa20 to 85658f5 Compare October 9, 2023 19:27
@reidbuzby
Copy link
Contributor Author

Actually running these tests comes in a follow up PR here: #15

@reidbuzby reidbuzby merged commit 44a43ed into main Oct 10, 2023
2 checks passed
@reidbuzby reidbuzby deleted the reid/integration-tests branch October 10, 2023 21:01
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.

Feature: Integration testing
2 participants