Skip to content

Update googletest to 1.17.0 #39313

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

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bazel/envoy_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
38 changes: 30 additions & 8 deletions bazel/googletest.patch
Original file line number Diff line number Diff line change
@@ -1,13 +1,35 @@
diff --git a/BUILD.bazel b/BUILD.bazel
index 8099642a85..3598661079 100644
index 53501454..fff6e491 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 = {
@@ -157,17 +151,6 @@ cc_library(
"@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": [],
}),
)

4 changes: 4 additions & 0 deletions bazel/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,10 @@ def _com_google_googletest():
"com_google_googletest",
patches = ["@envoy//bazel:googletest.patch"],
patch_args = ["-p1"],
repo_mapping = {
"@abseil-cpp": "@com_google_absl",
"@re2": "@com_googlesource_code_re2",
},
)

# TODO(jmarantz): replace the use of bind and external_deps with just
Expand Down
10 changes: 5 additions & 5 deletions bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -912,15 +912,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.17.0",
sha256 = "65fab701d9829d38cb77c14acdc431d2108bfdbf8979e40eb8ae567edf10b27c",
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 = "2025-04-30",
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",
Expand Down
2 changes: 1 addition & 1 deletion test/common/buffer/owned_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 8 additions & 2 deletions test/common/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,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",
Expand All @@ -214,7 +217,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",
Expand Down
4 changes: 3 additions & 1 deletion test/common/event/dispatcher_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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"),
Expand Down
15 changes: 15 additions & 0 deletions test/common/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ envoy_cc_test(
# Split to avoid compiler OOM, especially on ASAN.
"conn_manager_impl_test.cc",
"conn_manager_impl_test_2.cc",
"conn_manager_impl_test_3.cc",
],
rbe_pool = "2core",
shard_count = 5,
Expand All @@ -305,6 +306,20 @@ envoy_cc_test(
],
)

envoy_cc_test(
name = "conn_manager_misc_test",
srcs = ["conn_manager_misc_test.cc"],
rbe_pool = "2core",
shard_count = 5,
deps = [
":conn_manager_impl_test_base_lib",
":custom_header_extension_lib",
"//envoy/network:proxy_protocol_options_lib",
"//test/extensions/filters/network/common/fuzz/utils:network_filter_fuzzer_fakes_lib",
"//test/server:utility_lib",
],
)

envoy_cc_test(
name = "conn_manager_utility_test",
srcs = ["conn_manager_utility_test.cc"],
Expand Down
83 changes: 0 additions & 83 deletions test/common/http/conn_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
#include "test/test_common/logging.h"
#include "test/test_common/test_runtime.h"

#include "conn_manager_impl_test_base.h"

using testing::_;
using testing::An;
using testing::AnyNumber;
Expand Down Expand Up @@ -2715,87 +2713,6 @@ TEST_F(HttpConnectionManagerImplTest, TestPeriodicAccessLogging) {
filter->callbacks_->encodeHeaders(std::move(response_headers), true, "details");
}

class StreamErrorOnInvalidHttpMessageTest : public HttpConnectionManagerImplTest {
public:
void sendInvalidRequestAndVerifyConnectionState(bool stream_error_on_invalid_http_message,
bool send_complete_request = true) {
setup();

EXPECT_CALL(*codec_, dispatch(_))
.WillRepeatedly(Invoke([&](Buffer::Instance& data) -> Http::Status {
decoder_ = &conn_manager_->newStream(response_encoder_);

// These request headers are missing the necessary ":host"
RequestHeaderMapPtr headers{
new TestRequestHeaderMapImpl{{":method", "GET"}, {":path", "/"}}};
decoder_->decodeHeaders(std::move(headers), send_complete_request);
data.drain(0);
return Http::okStatus();
}));

auto* filter = new MockStreamFilter();
EXPECT_CALL(filter_factory_, createFilterChain(_))
.WillOnce(Invoke([&](FilterChainManager& manager) -> bool {
auto factory = createStreamFilterFactoryCb(StreamFilterSharedPtr{filter});
manager.applyFilterFactoryCb({}, factory);
return true;
}));
EXPECT_CALL(*filter, setDecoderFilterCallbacks(_));
EXPECT_CALL(*filter, setEncoderFilterCallbacks(_));

// codec stream error
EXPECT_CALL(response_encoder_, streamErrorOnInvalidHttpMessage())
.WillOnce(Return(stream_error_on_invalid_http_message));
EXPECT_CALL(*filter, encodeComplete());
EXPECT_CALL(*filter, encodeHeaders(_, true));
if (!stream_error_on_invalid_http_message) {
EXPECT_CALL(filter_callbacks_.connection_, close(_)).Times(AnyNumber());
if (send_complete_request) {
// The request is complete, so we should not flush close.
EXPECT_CALL(filter_callbacks_.connection_, close(Network::ConnectionCloseType::FlushWrite))
.Times(AnyNumber());
} else {
// If the request isn't complete, avoid a FIN/RST race with delay close.
EXPECT_CALL(filter_callbacks_.connection_,
close(Network::ConnectionCloseType::FlushWriteAndDelay))
.Times(AnyNumber());
}
}
EXPECT_CALL(response_encoder_, encodeHeaders(_, true))
.WillOnce(Invoke([&](const ResponseHeaderMap& headers, bool) -> void {
EXPECT_EQ("400", headers.getStatusValue());
EXPECT_EQ("missing_host_header",
filter->decoder_callbacks_->streamInfo().responseCodeDetails().value());
if (!stream_error_on_invalid_http_message) {
EXPECT_NE(nullptr, headers.Connection());
EXPECT_EQ("close", headers.getConnectionValue());
} else {
EXPECT_EQ(nullptr, headers.Connection());
}
}));

EXPECT_CALL(*filter, onStreamComplete());
EXPECT_CALL(*filter, onDestroy());

Buffer::OwnedImpl fake_input;
conn_manager_->onData(fake_input, false);
}
};

TEST_F(StreamErrorOnInvalidHttpMessageTest, ConnectionTerminatedIfCodecStreamErrorIsFalse) {
sendInvalidRequestAndVerifyConnectionState(false);
}

TEST_F(StreamErrorOnInvalidHttpMessageTest,
ConnectionTerminatedWithDelayIfCodecStreamErrorIsFalse) {
// Same as above, only with an incomplete request.
sendInvalidRequestAndVerifyConnectionState(false, false);
}

TEST_F(StreamErrorOnInvalidHttpMessageTest, ConnectionOpenIfCodecStreamErrorIsTrue) {
sendInvalidRequestAndVerifyConnectionState(true);
}

TEST_F(HttpConnectionManagerImplTest, TestAccessLogSsl) {
setup(SetupOpts().setSsl(true));

Expand Down
Loading