envoyproxy · asedeno · Oct 18, 2024 · Oct 18, 2024 · Oct 18, 2024 · Oct 18, 2024
diff --git a/bazel/envoy_test.bzl b/bazel/envoy_test.bzl
@@ -194,7 +194,7 @@ def envoy_cc_test(
         ] + envoy_pch_deps(repository, "//test:test_pch"),
         # from https://github.com/google/googletest/blob/6e1970e2376c14bf658eb88f655a054030353f9f/googlemock/src/gmock.cc#L51
         # 2 - by default, mocks act as StrictMocks.
-        args = args + ["--gmock_default_mock_behavior=2"],
+        args = ["--gmock_default_mock_behavior=2"] + args,
         tags = coverage_tags,
         local = local,
         shard_count = shard_count,

diff --git a/bazel/googletest.patch b/bazel/googletest.patch
@@ -1,13 +1,35 @@
 diff --git a/BUILD.bazel b/BUILD.bazel
-index 8099642a85..3598661079 100644
+index e407ae29..b1e3b7fb 100644
 --- a/BUILD.bazel
 +++ b/BUILD.bazel
-@@ -40,7 +40,7 @@ exports_files(["LICENSE"])
-
- config_setting(
-     name = "windows",
--    constraint_values = ["@bazel_tools//platforms:windows"],
-+    constraint_values = ["@platforms//os:windows"],
+@@ -56,12 +56,6 @@ config_setting(
+     constraint_values = ["@platforms//os:openbsd"],
  )
-
+
+-# NOTE: Fuchsia is not an officially supported platform.
+-config_setting(
+-    name = "fuchsia",
+-    constraint_values = ["@platforms//os:fuchsia"],
+-)
+-
  config_setting(
+     name = "msvc_compiler",
+     flag_values = {
+@@ -153,17 +147,6 @@ cc_library(
+             "@com_googlesource_code_re2//:re2",
+         ],
+         "//conditions:default": [],
+-    }) + select({
+-        # `gtest-death-test.cc` has `EXPECT_DEATH` that spawns a process,
+-        # expects it to crash and inspects its logs with the given matcher,
+-        # so that's why these libraries are needed.
+-        # Otherwise, builds targeting Fuchsia would fail to compile.
+-        ":fuchsia": [
+-            "@fuchsia_sdk//pkg/fdio",
+-            "@fuchsia_sdk//pkg/syslog",
+-            "@fuchsia_sdk//pkg/zx",
+-        ],
+-        "//conditions:default": [],
+     }),
+ )
+
diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl
@@ -933,15 +933,15 @@ REPOSITORY_LOCATIONS_SPEC = dict(
         project_url = "https://github.com/google/googletest",
         # Pick up fix for MOCK_METHOD compilation with clang-cl for Windows (resolved after 1.10.0)
         # see https://github.com/google/googletest/issues/2490
-        version = "a4ab0abb93620ce26efad9de9296b73b16e88588",
-        sha256 = "7897bfaa5ad39a479177cfb5c3ce010184dbaee22a7c3727b212282871918751",
+        version = "1.15.2",
+        sha256 = "7b42b4d6ed48810c5362c265a17faebe90dc2373c885e5216439d37927f02926",
         strip_prefix = "googletest-{version}",
-        urls = ["https://github.com/google/googletest/archive/{version}.tar.gz"],
-        release_date = "2020-09-10",
+        urls = ["https://github.com/google/googletest/releases/download/v{version}/googletest-{version}.tar.gz"],
+        release_date = "2024-07-31",
         use_category = ["test_only"],
         cpe = "cpe:2.3:a:google:google_test:*",
         license = "BSD-3-Clause",
-        license_url = "https://github.com/google/googletest/blob/{version}/LICENSE",
+        license_url = "https://github.com/google/googletest/blob/v{version}/LICENSE",
     ),
     com_google_protobuf = dict(
         project_name = "Protocol Buffers",

@@ -181,6 +181,11 @@ build:mobile-remote-ci-linux-asan --config=mobile-clang-asan
 build:mobile-remote-ci-linux-asan --config=mobile-remote-ci-linux-clang
 build:mobile-remote-ci-linux-asan --config=remote-ci
 build:mobile-remote-ci-linux-asan --build_tests_only
+# This is probably not the right place to put this, since this target
+# isn't engflow-specific. However, we only need to enable rbe_pool for
+# asan builds right now and this should help unblock testing on the PR.
+# TODO(asedeno): Figure out something better before merging.
+build:mobile-remote-ci-linux-asan --define=engflow_rbe=true
 test:mobile-remote-ci-linux-asan --test_env=ENVOY_IP_TEST_VERSIONS=v4only
 
 #############################################################################

@@ -18,6 +18,7 @@ envoy_cc_test(
 envoy_cc_test(
     name = "internal_engine_test",
     srcs = ["internal_engine_test.cc"],
+    rbe_pool = "6gig",
     repository = "@envoy",
     deps = [
         "//library/cc:engine_builder_lib",

@@ -31,6 +31,7 @@ envoy_extension_cc_test(
         "platform_bridge_filter_integration_test.cc",
     ],
     extension_names = ["envoy.filters.http.platform_bridge"],
+    rbe_pool = "6gig",
 exec_properties = exec_properties | select({ 
     repository + "//bazel:engflow_rbe": {"Pool": rbe_pool} if rbe_pool else {}, 
     "//conditions:default": {}, 
 }) 
 exec_properties = exec_properties | select({ 
     repository + "//bazel:engflow_rbe": {"Pool": rbe_pool} if rbe_pool else {}, 
     "//conditions:default": {}, 
 }) 
     repository = "@envoy",
     deps = [
         "//library/common/api:external_api_lib",

@@ -21,6 +21,7 @@ envoy_cc_test(
         ],
         "//conditions:default": [],
     }),
+    rbe_pool = "6gig",
     repository = "@envoy",
     shard_count = 6,
     deps = [
@@ -46,6 +47,7 @@ envoy_cc_test_library(
     hdrs = [
         "base_client_integration_test.h",
     ],
+    rbe_pool = "6gig",
     repository = "@envoy",
     deps = [
         "//library/cc:engine_builder_lib",
@@ -70,6 +72,7 @@ envoy_cc_test_library(
     data = [
         "@envoy//test/config/integration/certs",
     ],
+    rbe_pool = "6gig",
     repository = "@envoy",
     deps = [
         "//library/common:engine_common_lib",

diff --git a/test/common/buffer/owned_impl_test.cc b/test/common/buffer/owned_impl_test.cc
@@ -1090,7 +1090,7 @@ TEST_F(OwnedImplTest, PrependEmpty) {
   EXPECT_EQ(0, buf.length());
 }
 
-TEST(OverflowDetectingUInt64, Arithmetic) {
+TEST(OverflowDetectingUInt64DeathTest, Arithmetic) {
   OverflowDetectingUInt64 length;
   length += 1;
   length -= 1;

diff --git a/test/common/common/BUILD b/test/common/common/BUILD
@@ -202,7 +202,10 @@ envoy_cc_test(
 envoy_cc_test(
     name = "log_verbosity_update_test",
     srcs = ["log_verbosity_update_test.cc"],
-    args = ["--enable-fine-grain-logging"],
+    args = [
+        "--",
+        "--enable-fine-grain-logging",
+    ],
     rbe_pool = "6gig",
     deps = [
         "//source/common/common:minimal_logger_lib",
@@ -213,7 +216,10 @@ envoy_cc_test(
 envoy_cc_test(
     name = "fine_grain_log_macros_test",
     srcs = ["log_macros_test.cc"],
-    args = ["--enable-fine-grain-logging"],
+    args = [
+        "--",
+        "--enable-fine-grain-logging",
+    ],
     rbe_pool = "6gig",
     deps = [
         "//source/common/common:minimal_logger_lib",

diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc
@@ -1600,6 +1600,8 @@ class DispatcherConnectionTest : public testing::Test {
   DispatcherPtr dispatcher_;
 };
 
+using DispatcherConnectionDeathTest = DispatcherConnectionTest;
+
 TEST_F(DispatcherConnectionTest, CreateTcpConnection) {
   for (auto ip_version : TestEnvironment::getIpVersionsForTest()) {
     SCOPED_TRACE(Network::Test::addressVersionAsString(ip_version));
@@ -1615,7 +1617,7 @@ TEST_F(DispatcherConnectionTest, CreateTcpConnection) {
 
 // If the internal connection factory is not linked, envoy will be dead when creating connection to
 // internal address.
-TEST_F(DispatcherConnectionTest, CreateEnvoyInternalConnectionWhenFactoryNotExist) {
+TEST_F(DispatcherConnectionDeathTest, CreateEnvoyInternalConnectionWhenFactoryNotExist) {
   EXPECT_DEATH(
       dispatcher_->createClientConnection(
           std::make_shared<Network::Address::EnvoyInternalInstance>("listener_internal_address"),

diff --git a/test/common/http/header_mutation_test.cc b/test/common/http/header_mutation_test.cc
@@ -272,7 +272,7 @@ TEST(HeaderMutationsTest, BasicOrder) {
   }
 }
 
-TEST(HeaderMutationTest, Death) {
+TEST(HeaderMutationDeathTest, Death) {
   ProtoHeaderMutatons proto_mutations;
   proto_mutations.Add();
 

diff --git a/test/common/listener_manager/BUILD b/test/common/listener_manager/BUILD
@@ -41,6 +41,7 @@ envoy_cc_test_library(
     name = "listener_manager_impl_test_lib",
     hdrs = ["listener_manager_impl_test.h"],
     data = ["//test/common/tls/test_data:certs"],
+    rbe_pool = "6gig",
     deps = [
         ":config_cc_proto",
         "//source/common/formatter:formatter_extension_lib",

diff --git a/test/common/quic/envoy_quic_client_session_test.cc b/test/common/quic/envoy_quic_client_session_test.cc
@@ -225,6 +225,11 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam<quic::ParsedQui
 INSTANTIATE_TEST_SUITE_P(EnvoyQuicClientSessionTests, EnvoyQuicClientSessionTest,
                          testing::ValuesIn(quic::CurrentSupportedHttp3Versions()));
 
+using EnvoyQuicClientSessionDeathTest = EnvoyQuicClientSessionTest;
+
+INSTANTIATE_TEST_SUITE_P(EnvoyQuicClientSessionTests, EnvoyQuicClientSessionDeathTest,
+                         testing::ValuesIn(quic::CurrentSupportedHttp3Versions()));
+
 TEST_P(EnvoyQuicClientSessionTest, ShutdownNoOp) { http_connection_->shutdownNotice(); }
 
 TEST_P(EnvoyQuicClientSessionTest, NewStream) {
@@ -445,29 +450,29 @@ TEST_P(EnvoyQuicClientSessionTest, VerifyContext) {
   EXPECT_TRUE(verify_context.extraValidationContext().callbacks->ioHandle().isOpen());
 }
 
-TEST_P(EnvoyQuicClientSessionTest, VerifyContextAbortOnRaiseEvent) {
+TEST_P(EnvoyQuicClientSessionDeathTest, VerifyContextAbortOnRaiseEvent) {
   auto& verify_context =
       dynamic_cast<EnvoyQuicProofVerifyContext&>(crypto_stream_factory_.lastVerifyContext().ref());
   EXPECT_DEATH(verify_context.extraValidationContext().callbacks->raiseEvent(
                    Network::ConnectionEvent::Connected),
                "unexpectedly reached");
 }
 
-TEST_P(EnvoyQuicClientSessionTest, VerifyContextAbortOnShouldDrainReadBuffer) {
+TEST_P(EnvoyQuicClientSessionDeathTest, VerifyContextAbortOnShouldDrainReadBuffer) {
   auto& verify_context =
       dynamic_cast<EnvoyQuicProofVerifyContext&>(crypto_stream_factory_.lastVerifyContext().ref());
   EXPECT_DEATH(verify_context.extraValidationContext().callbacks->shouldDrainReadBuffer(),
                "unexpectedly reached");
 }
 
-TEST_P(EnvoyQuicClientSessionTest, VerifyContextAbortOnSetTransportSocketIsReadable) {
+TEST_P(EnvoyQuicClientSessionDeathTest, VerifyContextAbortOnSetTransportSocketIsReadable) {
   auto& verify_context =
       dynamic_cast<EnvoyQuicProofVerifyContext&>(crypto_stream_factory_.lastVerifyContext().ref());
   EXPECT_DEATH(verify_context.extraValidationContext().callbacks->setTransportSocketIsReadable(),
                "unexpectedly reached");
 }
 
-TEST_P(EnvoyQuicClientSessionTest, VerifyContextAbortOnFlushWriteBuffer) {
+TEST_P(EnvoyQuicClientSessionDeathTest, VerifyContextAbortOnFlushWriteBuffer) {
   auto& verify_context =
       dynamic_cast<EnvoyQuicProofVerifyContext&>(crypto_stream_factory_.lastVerifyContext().ref());
   EXPECT_DEATH(verify_context.extraValidationContext().callbacks->flushWriteBuffer(),

diff --git a/test/common/quic/quic_filter_manager_connection_impl_test.cc b/test/common/quic/quic_filter_manager_connection_impl_test.cc
@@ -66,6 +66,8 @@ class QuicFilterManagerConnectionImplTest : public ::testing::Test {
   TestQuicFilterManagerConnectionImpl impl_;
 };
 
+using QuicFilterManagerConnectionImplDeathTest = QuicFilterManagerConnectionImplTest;
+
 TEST_F(QuicFilterManagerConnectionImplTest, ConnectionInfoProviderSharedPtr) {
   EXPECT_TRUE(impl_.connectionInfoProviderSharedPtr() != nullptr);
   impl_.closeNow();
@@ -113,19 +115,19 @@ TEST_F(QuicFilterManagerConnectionImplTest, RawWrite) {
   EXPECT_ENVOY_BUG(impl_.rawWrite(data, true), "unexpected call to rawWrite");
 }
 
-TEST_F(QuicFilterManagerConnectionImplTest, BufferLimit) {
+TEST_F(QuicFilterManagerConnectionImplDeathTest, BufferLimit) {
   EXPECT_DEATH(impl_.bufferLimit(), "not implemented");
 }
 
 TEST_F(QuicFilterManagerConnectionImplTest, SetBufferLimits) {
   EXPECT_ENVOY_BUG(impl_.setBufferLimits(1), "unexpected call to setBufferLimits");
 }
 
-TEST_F(QuicFilterManagerConnectionImplTest, GetWriteBuffer) {
+TEST_F(QuicFilterManagerConnectionImplDeathTest, GetWriteBuffer) {
   EXPECT_DEATH(impl_.getWriteBuffer(), "not implemented");
 }
 
-TEST_F(QuicFilterManagerConnectionImplTest, EnableHalfClose) {
+TEST_F(QuicFilterManagerConnectionImplDeathTest, EnableHalfClose) {
   impl_.enableHalfClose(false); // No-op
   EXPECT_DEATH(impl_.enableHalfClose(true), "Quic connection doesn't support half close.");
 }

diff --git a/test/common/runtime/BUILD b/test/common/runtime/BUILD
@@ -64,6 +64,7 @@ envoy_cc_test(
     name = "runtime_flag_override_test",
     srcs = ["runtime_flag_override_test.cc"],
     args = [
+        "--",
         "--runtime-feature-override-for-tests=envoy.reloadable_features.test_feature_false",
         "--runtime-feature-disable-for-tests=envoy.reloadable_features.test_feature_true",
     ],
@@ -78,6 +79,7 @@ envoy_cc_test(
     name = "runtime_flag_override_noop_test",
     srcs = ["runtime_flag_override_noop_test.cc"],
     args = [
+        "--",
         "--runtime-feature-override-for-tests=envoy.reloadable_features.test_feature_true",
         "--runtime-feature-disable-for-tests=envoy.reloadable_features.test_feature_false",
     ],

@@ -22,6 +22,9 @@ namespace {
 
 class CrashIntegrationTest : public Event::TestUsingSimulatedTime,
                              public HttpProtocolIntegrationTest {
+public:
+  void SetUp() override { GTEST_FLAG_SET(death_test_style, "threadsafe"); }
+
 protected:
   void initializeFilter(const std::string& filter_config) {
     config_helper_.prependFilter(filter_config);
@@ -69,8 +72,8 @@ TEST_P(CrashIntegrationTestAllProtocols, UnwindsTrackedObjectStack) {
   //  - Http(1|2)::ConnectionImpl
   //  - Network::ConnectionImpl
   const std::string death_string = GetParam().downstream_protocol == Http::CodecType::HTTP2
-                                       ? "ActiveStream.*Http2::ConnectionImpl.*ConnectionImpl"
-                                       : "ActiveStream.*Http1::ConnectionImpl.*ConnectionImpl";
+                                       ? "(?s)ActiveStream.*Http2::ConnectionImpl.*ConnectionImpl"
+                                       : "(?s)ActiveStream.*Http1::ConnectionImpl.*ConnectionImpl";
   EXPECT_DEATH(sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 1024),
                death_string);
 }
@@ -93,7 +96,7 @@ TEST_P(CrashIntegrationTestAllProtocols, ResponseCrashDumpsTheCorrespondingReque
   // Check that we dump the downstream request
   EXPECT_DEATH(
       sendRequestAndWaitForResponse(default_request_headers_, 0, kill_response_headers, 1024),
-      "Dumping corresponding downstream request.*UpstreamRequest.*request_headers:");
+      "(?s)Dumping corresponding downstream request.*UpstreamRequest.*request_headers:");
 }
 
 TEST_P(CrashIntegrationTestAllProtocols, DecodeContinueDoesNotAddTrackedObjectIfExists) {
@@ -164,7 +167,7 @@ TEST_P(CrashIntegrationTestAllProtocols, DecodeContinueAddsCrashContextIfNoneExi
   //  - ActiveStream
   //  - Network::ConnectionImpl
   EXPECT_DEATH(sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 1024),
-               "ActiveStream.*.*ConnectionImpl");
+               "(?s)ActiveStream.*.*ConnectionImpl");
 }
 
 TEST_P(CrashIntegrationTestAllProtocols, EncodeContinueDoesNotAddTrackedObjectIfExists) {
@@ -231,7 +234,7 @@ TEST_P(CrashIntegrationTestAllProtocols, EncodeContinueAddsCrashContextIfNoneExi
   //  - ActiveStream
   //  - Network::ConnectionImpl
   EXPECT_DEATH(sendRequestAndWaitForResponse(default_request_headers_, 0, kill_response_headers, 0),
-               "ActiveStream.*.*ConnectionImpl");
+               "(?s)ActiveStream.*.*ConnectionImpl");
 }
 
 #endif

@@ -10,6 +10,9 @@ namespace {
 
 class KillRequestFilterIntegrationTest : public Event::TestUsingSimulatedTime,
                                          public HttpProtocolIntegrationTest {
+public:
+  void SetUp() override { GTEST_FLAG_SET(death_test_style, "threadsafe"); }
+
 protected:
   void initializeFilter(const std::string& filter_config) {
     config_helper_.prependFilter(filter_config);