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

A basic set of EventEngine listener tests #31540

Merged
merged 4 commits into from Nov 9, 2022

Conversation

Vignesh2208
Copy link
Contributor

No description provided.

test/core/event_engine/test_suite/server_test.cc Outdated Show resolved Hide resolved
test/core/event_engine/test_suite/server_test.cc Outdated Show resolved Hide resolved
// by the Test EventEngine and exchange bi-di data over the connection.
// For each data transfer, verify that data written at one end of the stream
// equals data read at the other end of the stream.
TEST_F(EventEngineServerTest, ServerConnectExchangeBidiDataTransferTest) {
Copy link
Member

Choose a reason for hiding this comment

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

This test might flake on infrastructure network issues, which we have seen. We can watch it for a bit and see the flake rate. It might be nice to improve the test so that infrastructure flakes do not show up as test flakes/failures (e.g., add retries on identified poor network conditions), but we can consider that later. Just noting that this may flake through no fault of the code.

test/core/event_engine/test_suite/server_test.cc Outdated Show resolved Hide resolved
Copy link
Member

@drfloob drfloob left a comment

Choose a reason for hiding this comment

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

Looks pretty good! Just a few style-guide and syntax things remaining.

test/core/event_engine/test_suite/client_test.cc Outdated Show resolved Hide resolved
test/core/event_engine/test_suite/client_test.cc Outdated Show resolved Hide resolved
@Vignesh2208 Vignesh2208 merged commit 504d49d into grpc:master Nov 9, 2022
@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bloat/none imported Specifies if the PR has been imported to the internal repository lang/core per-call-memory/neutral per-channel-memory/neutral release notes: no Indicates if PR should not be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants