From f61ffce971708a8e5164adbec08c9b899c722e08 Mon Sep 17 00:00:00 2001 From: Dom Del Nano Date: Tue, 2 Dec 2025 12:08:59 +0000 Subject: [PATCH] Remove use of protobuf debug APIs in test assertions to fix breaking change (https://protobuf.dev/news/2024-12-04/) Signed-off-by: Dom Del Nano (cherry picked from commit 720d2df01e456bc2e8c6758ab0ebecfa3bd5c248) --- .../compiler_error_context_test.cc | 2 +- .../distributed_plan/distributed_plan_test.cc | 4 +++- src/carnot/planner/docs/doc_extractor_main.cc | 12 +++++++++--- src/common/base/status_test.cc | 4 ++-- src/common/testing/protobuf.h | 12 ++++++++++++ .../services/agent/pem/tracepoint_manager_test.cc | 12 ++++++------ 6 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/carnot/planner/compiler_error_context/compiler_error_context_test.cc b/src/carnot/planner/compiler_error_context/compiler_error_context_test.cc index 76852c0832f..580f6962174 100644 --- a/src/carnot/planner/compiler_error_context/compiler_error_context_test.cc +++ b/src/carnot/planner/compiler_error_context/compiler_error_context_test.cc @@ -51,7 +51,7 @@ TEST(CompilerErrorContextStatus, Default) { ASSERT_TRUE(status_pb.context().Is()); status_pb.context().UnpackTo(&errorgroup_out); - EXPECT_EQ(errorgroup_in.DebugString(), errorgroup_out.DebugString()); + EXPECT_TRUE(google::protobuf::util::MessageDifferencer::Equals(errorgroup_in, errorgroup_out)); for (int64_t i = 0; i < errorgroup_in.errors_size(); i++) { auto error_parent_out = errorgroup_in.errors(i); auto error_out = error_parent_out.line_col_error(); diff --git a/src/carnot/planner/distributed/distributed_plan/distributed_plan_test.cc b/src/carnot/planner/distributed/distributed_plan/distributed_plan_test.cc index b8dece83c11..5146a4445cf 100644 --- a/src/carnot/planner/distributed/distributed_plan/distributed_plan_test.cc +++ b/src/carnot/planner/distributed/distributed_plan/distributed_plan_test.cc @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -208,7 +209,8 @@ TEST_F(DistributedPlanTest, construction_test) { for (const auto& [carnot_id, carnot_info] : carnot_id_to_carnot_info_map) { CarnotInstance* carnot_instance = physical_plan->Get(carnot_id); - EXPECT_THAT(carnot_instance->carnot_info(), EqualsProto(carnot_info.DebugString())); + EXPECT_TRUE(google::protobuf::util::MessageDifferencer::Equals(carnot_instance->carnot_info(), + carnot_info)); EXPECT_THAT(carnot_instance->QueryBrokerAddress(), carnot_info.query_broker_address()); auto new_graph = std::make_shared(); SwapGraphBeingBuilt(new_graph); diff --git a/src/carnot/planner/docs/doc_extractor_main.cc b/src/carnot/planner/docs/doc_extractor_main.cc index da5059b188a..e3b9472e07d 100644 --- a/src/carnot/planner/docs/doc_extractor_main.cc +++ b/src/carnot/planner/docs/doc_extractor_main.cc @@ -16,11 +16,14 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include +#include + +#include + #include "src/carnot/planner/docs/doc_extractor.h" #include "src/carnot/udf_exporter/udf_exporter.h" -#include - namespace px { namespace carnot { namespace planner { @@ -79,8 +82,11 @@ int main(int argc, char** argv) { } auto docs = docs_or_s.ConsumeValueOrDie(); + std::string text_output; + google::protobuf::TextFormat::PrintToString(docs, &text_output); + std::ofstream output_file; output_file.open(FLAGS_output_file); - output_file << docs.DebugString(); + output_file << text_output; output_file.close(); } diff --git a/src/common/base/status_test.cc b/src/common/base/status_test.cc index 8d349be7da4..5772258aee8 100644 --- a/src/common/base/status_test.cc +++ b/src/common/base/status_test.cc @@ -111,7 +111,7 @@ TEST(Status, context_copy_tests) { Status s2 = s1; EXPECT_TRUE(s2.has_context()); EXPECT_EQ(s1, s2); - EXPECT_EQ(s1.context()->DebugString(), s2.context()->DebugString()); + EXPECT_TRUE(google::protobuf::util::MessageDifferencer::Equals(*s1.context(), *s2.context())); } TEST(Status, context_vs_no_context_status) { @@ -120,7 +120,7 @@ TEST(Status, context_vs_no_context_status) { EXPECT_NE(s1, s2); EXPECT_FALSE(s2.has_context()); EXPECT_EQ(s2.context(), nullptr); - EXPECT_NE(s1.ToProto().DebugString(), s2.ToProto().DebugString()); + EXPECT_FALSE(google::protobuf::util::MessageDifferencer::Equals(s1.ToProto(), s2.ToProto())); } TEST(StatusAdapter, proto_with_context_test) { diff --git a/src/common/testing/protobuf.h b/src/common/testing/protobuf.h index dfd6091a4e6..5a9fed3d16b 100644 --- a/src/common/testing/protobuf.h +++ b/src/common/testing/protobuf.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include "src/common/base/logging.h" @@ -109,6 +110,17 @@ inline ::testing::PolymorphicMatcher EqualsProto(std::string return ::testing::MakePolymorphicMatcher(EqualsProtoMatcher(std::move(text_pb))); } +// Overload that takes a protobuf message directly. +// This avoids the need to call DebugString() which in newer protobuf versions +// adds a "goo.gle/debugonly" prefix that breaks parsing. +template >> +inline ::testing::PolymorphicMatcher EqualsProto(const ProtoType& proto) { + std::string text_pb; + google::protobuf::TextFormat::PrintToString(proto, &text_pb); + return ::testing::MakePolymorphicMatcher(EqualsProtoMatcher(std::move(text_pb))); +} + inline ::testing::PolymorphicMatcher Partially( const ::testing::PolymorphicMatcher& matcher) { return ::testing::MakePolymorphicMatcher( diff --git a/src/vizier/services/agent/pem/tracepoint_manager_test.cc b/src/vizier/services/agent/pem/tracepoint_manager_test.cc index 9cd85aed2af..fd54f7badb2 100644 --- a/src/vizier/services/agent/pem/tracepoint_manager_test.cc +++ b/src/vizier/services/agent/pem/tracepoint_manager_test.cc @@ -115,8 +115,8 @@ TEST_F(TracepointManagerTest, CreateTracepoint) { tracepoint->set_name("test_tracepoint"); EXPECT_CALL(stirling_, - RegisterTracepoint(tracepoint_id, ::testing::Pointee(testing::proto::EqualsProto( - tracepoint->DebugString())))); + RegisterTracepoint(tracepoint_id, + ::testing::Pointee(testing::proto::EqualsProto(*tracepoint)))); EXPECT_OK(tracepoint_manager_->HandleMessage(std::move(msg))); EXPECT_CALL(stirling_, GetTracepointInfo(tracepoint_id)) @@ -152,8 +152,8 @@ TEST_F(TracepointManagerTest, CreateTracepointFailed) { tracepoint->set_name("test_tracepoint"); EXPECT_CALL(stirling_, - RegisterTracepoint(tracepoint_id, ::testing::Pointee(testing::proto::EqualsProto( - tracepoint->DebugString())))); + RegisterTracepoint(tracepoint_id, + ::testing::Pointee(testing::proto::EqualsProto(*tracepoint)))); EXPECT_OK(tracepoint_manager_->HandleMessage(std::move(msg))); EXPECT_CALL(stirling_, GetTracepointInfo(tracepoint_id)) @@ -185,8 +185,8 @@ TEST_F(TracepointManagerTest, CreateTracepointPreconditionFailed) { tracepoint->set_name("test_tracepoint"); EXPECT_CALL(stirling_, - RegisterTracepoint(tracepoint_id, ::testing::Pointee(testing::proto::EqualsProto( - tracepoint->DebugString())))); + RegisterTracepoint(tracepoint_id, + ::testing::Pointee(testing::proto::EqualsProto(*tracepoint)))); EXPECT_OK(tracepoint_manager_->HandleMessage(std::move(msg))); EXPECT_CALL(stirling_, GetTracepointInfo(tracepoint_id))