From 7740da57a162017a873cf2055bd18c83c7a4818f Mon Sep 17 00:00:00 2001 From: AJ Heller Date: Mon, 23 Oct 2023 08:54:53 -0700 Subject: [PATCH] [EventEngine] Re-enable skipped core/end2end EventEngine client tests (#34702) Let's not merge this PR this week, maybe 2023-10-23 at the earliest. This will allow time for flakes to shake out of the listener experiments (enabled in https://github.com/grpc/grpc/pull/34700) in isolation of client problems. --- test/core/end2end/end2end_tests.h | 4 ---- test/core/end2end/tests/binary_metadata.cc | 2 -- test/core/end2end/tests/call_creds.cc | 4 ---- test/core/end2end/tests/cancel_after_accept.cc | 2 -- test/core/end2end/tests/cancel_after_invoke.cc | 4 ---- test/core/end2end/tests/cancel_with_status.cc | 1 - test/core/end2end/tests/filtered_metadata.cc | 2 -- test/core/end2end/tests/retry_recv_initial_metadata.cc | 2 -- test/core/end2end/tests/simple_request.cc | 5 +---- 9 files changed, 1 insertion(+), 25 deletions(-) diff --git a/test/core/end2end/end2end_tests.h b/test/core/end2end/end2end_tests.h index d49bd4e4070..436b0527bf3 100644 --- a/test/core/end2end/end2end_tests.h +++ b/test/core/end2end/end2end_tests.h @@ -868,10 +868,6 @@ class CoreEnd2endTestRegistry { if (GetParam()->feature_mask & FEATURE_MASK_IS_MINSTACK) \ GTEST_SKIP() << "Skipping test for minstack" -#define SKIP_IF_USES_EVENT_ENGINE_CLIENT() \ - if (!g_is_fuzzing_core_e2e_tests && grpc_core::IsEventEngineClientEnabled()) \ - GTEST_SKIP() << "Skipping test to prevent it from using EventEngine client" - #define SKIP_IF_FUZZING() \ if (g_is_fuzzing_core_e2e_tests) GTEST_SKIP() << "Skipping test for fuzzing" diff --git a/test/core/end2end/tests/binary_metadata.cc b/test/core/end2end/tests/binary_metadata.cc index b4718a8c004..4e4123fd369 100644 --- a/test/core/end2end/tests/binary_metadata.cc +++ b/test/core/end2end/tests/binary_metadata.cc @@ -119,8 +119,6 @@ CORE_END2END_TEST(CoreEnd2endTest, CORE_END2END_TEST(CoreEnd2endTest, BinaryMetadataServerHttp2FallbackClientHttp2Fallback) { - // TODO(vigneshbabu): re-enable these before release - SKIP_IF_USES_EVENT_ENGINE_CLIENT(); BinaryMetadata(*this, false, false); } diff --git a/test/core/end2end/tests/call_creds.cc b/test/core/end2end/tests/call_creds.cc index 780d069a7dd..b90f681d0d2 100644 --- a/test/core/end2end/tests/call_creds.cc +++ b/test/core/end2end/tests/call_creds.cc @@ -288,8 +288,6 @@ CORE_END2END_TEST(PerCallCredsTest, CORE_END2END_TEST(PerCallCredsTest, RequestResponseWithPayloadAndDeletedInsecureCallCreds) { - // TODO(vigneshbabu): re-enable these before release - SKIP_IF_USES_EVENT_ENGINE_CLIENT(); TestRequestResponseWithPayloadAndDeletedCallCreds(*this, false); } @@ -305,8 +303,6 @@ CORE_END2END_TEST(PerCallCredsOnInsecureTest, CORE_END2END_TEST(PerCallCredsOnInsecureTest, RequestResponseWithPayloadAndDeletedInsecureCallCreds) { - // TODO(vigneshbabu): re-enable these before release - SKIP_IF_USES_EVENT_ENGINE_CLIENT(); TestRequestResponseWithPayloadAndDeletedCallCreds(*this, false); } diff --git a/test/core/end2end/tests/cancel_after_accept.cc b/test/core/end2end/tests/cancel_after_accept.cc index 70c26a3bce7..c5e68a18b93 100644 --- a/test/core/end2end/tests/cancel_after_accept.cc +++ b/test/core/end2end/tests/cancel_after_accept.cc @@ -78,8 +78,6 @@ CORE_END2END_TEST(CoreDeadlineTest, DeadlineAfterAccept) { } CORE_END2END_TEST(CoreClientChannelTest, DeadlineAfterAcceptWithServiceConfig) { - // TODO(vigneshbabu): re-enable these before release - SKIP_IF_USES_EVENT_ENGINE_CLIENT(); InitServer(ChannelArgs()); InitClient(ChannelArgs().Set( GRPC_ARG_SERVICE_CONFIG, diff --git a/test/core/end2end/tests/cancel_after_invoke.cc b/test/core/end2end/tests/cancel_after_invoke.cc index 8c3ba490c1b..5a1c8c9280d 100644 --- a/test/core/end2end/tests/cancel_after_invoke.cc +++ b/test/core/end2end/tests/cancel_after_invoke.cc @@ -109,25 +109,21 @@ void CancelAfterInvoke3(CoreEnd2endTest& test, } CORE_END2END_TEST(CoreEnd2endTest, CancelAfterInvoke6) { - // TODO(vigneshbabu): re-enable these before release CancelAfterInvoke6(*this, std::make_unique(), kCancelTimeout); } CORE_END2END_TEST(CoreEnd2endTest, CancelAfterInvoke5) { - // TODO(vigneshbabu): re-enable these before release CancelAfterInvoke5(*this, std::make_unique(), kCancelTimeout); } CORE_END2END_TEST(CoreEnd2endTest, CancelAfterInvoke4) { - // TODO(vigneshbabu): re-enable these before release CancelAfterInvoke4(*this, std::make_unique(), kCancelTimeout); } CORE_END2END_TEST(CoreEnd2endTest, CancelAfterInvoke3) { - // TODO(vigneshbabu): re-enable these before release CancelAfterInvoke3(*this, std::make_unique(), kCancelTimeout); } diff --git a/test/core/end2end/tests/cancel_with_status.cc b/test/core/end2end/tests/cancel_with_status.cc index c6d402f1482..0c4f5b2685d 100644 --- a/test/core/end2end/tests/cancel_with_status.cc +++ b/test/core/end2end/tests/cancel_with_status.cc @@ -83,7 +83,6 @@ CORE_END2END_TEST(CoreEnd2endTest, CancelWithStatus3) { } CORE_END2END_TEST(CoreEnd2endTest, CancelWithStatus4) { - // TODO(vigneshbabu): re-enable these before release auto c = NewClientCall("/foo").Timeout(Duration::Seconds(5)).Create(); CoreEnd2endTest::IncomingMetadata server_initial_metadata; CoreEnd2endTest::IncomingStatusOnClient server_status; diff --git a/test/core/end2end/tests/filtered_metadata.cc b/test/core/end2end/tests/filtered_metadata.cc index 6ec6858d367..f5b94af0a5f 100644 --- a/test/core/end2end/tests/filtered_metadata.cc +++ b/test/core/end2end/tests/filtered_metadata.cc @@ -74,8 +74,6 @@ void TestRequestResponseWithMetadataToBeFiltered( } CORE_END2END_TEST(CoreEnd2endTest, ContentLengthIsFiltered) { - // TODO(vigneshbabu): re-enable these before release - SKIP_IF_USES_EVENT_ENGINE_CLIENT(); TestRequestResponseWithMetadataToBeFiltered(*this, "content-length", "45"); } diff --git a/test/core/end2end/tests/retry_recv_initial_metadata.cc b/test/core/end2end/tests/retry_recv_initial_metadata.cc index 9c83255e2ac..9738cd5dee1 100644 --- a/test/core/end2end/tests/retry_recv_initial_metadata.cc +++ b/test/core/end2end/tests/retry_recv_initial_metadata.cc @@ -36,8 +36,6 @@ namespace { // - first attempt receives initial metadata before trailing metadata, // so no retry is done even though status was ABORTED CORE_END2END_TEST(RetryTest, RetryRecvInitialMetadata) { - // TODO(vigneshbabu): re-enable these before release - SKIP_IF_USES_EVENT_ENGINE_CLIENT(); InitServer(ChannelArgs()); InitClient(ChannelArgs().Set( GRPC_ARG_SERVICE_CONFIG, diff --git a/test/core/end2end/tests/simple_request.cc b/test/core/end2end/tests/simple_request.cc index b092c0b11bd..359489f6c2d 100644 --- a/test/core/end2end/tests/simple_request.cc +++ b/test/core/end2end/tests/simple_request.cc @@ -98,10 +98,7 @@ void SimpleRequestBody(CoreEnd2endTest& test) { expected_calls); } -CORE_END2END_TEST(CoreEnd2endTest, SimpleRequest) { - // TODO(vigneshbabu): re-enable these before release - SimpleRequestBody(*this); -} +CORE_END2END_TEST(CoreEnd2endTest, SimpleRequest) { SimpleRequestBody(*this); } CORE_END2END_TEST(CoreEnd2endTest, SimpleRequest10) { for (int i = 0; i < 10; i++) {