From cfe5076a8b99ce7979b0b5d328ff8a9199e04eef Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Tue, 16 Jan 2024 11:18:39 -0800 Subject: [PATCH] Do not emit stack traces for messages generated by GTEST_SKIP() Stack traces in assertion failures are an extremely useful tool for developers tasked with investigating failing tests. It's difficult to understate this. In contrast to ordinary test assertions (e.g., ASSERT_TRUE or EXPECT_FALSE), GTEST_SKIP is a developer-authored directive to skip one or more tests. Stack traces emitted in skip messages do not give the developer useful information, as the skip message itself contains the code location where GTEST_SKIP was used. In addition to being noise in the output, symbolization of stack traces is not free. In some Chromium configurations, symbolization in a skipped test can incur a cost in excess of 25 seconds for a test that otherwise takes 0-1ms; see https://crbug.com/1517343#c9. In this CL, we suppress generation and emission of stack traces for kSkip messages to reduce the output noise and overhead of GTEST_SKIP(). PiperOrigin-RevId: 598899010 Change-Id: I46926fed452c8d7edcb3d636d8fed42cb6c0a9e9 --- googletest/src/gtest.cc | 7 ++++++- .../test/googletest-output-test-golden-lin.txt | 1 - googletest/test/gtest_xml_output_unittest.py | 15 +++++++++------ 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/googletest/src/gtest.cc b/googletest/src/gtest.cc index 479b2ee3..46212e74 100644 --- a/googletest/src/gtest.cc +++ b/googletest/src/gtest.cc @@ -459,7 +459,12 @@ void AssertHelper::operator=(const Message& message) const { UnitTest::GetInstance()->AddTestPartResult( data_->type, data_->file, data_->line, AppendUserMessage(data_->message, message), - UnitTest::GetInstance()->impl()->CurrentOsStackTraceExceptTop(1) + // Suppress emission of the stack trace for GTEST_SKIP() since skipping is + // an intentional act by the developer rather than a failure requiring + // investigation. + data_->type != TestPartResult::kSkip + ? UnitTest::GetInstance()->impl()->CurrentOsStackTraceExceptTop(1) + : "" // Skips the stack frame for this function itself. ); // NOLINT } diff --git a/googletest/test/googletest-output-test-golden-lin.txt b/googletest/test/googletest-output-test-golden-lin.txt index 6ddf822f..e06856a2 100644 --- a/googletest/test/googletest-output-test-golden-lin.txt +++ b/googletest/test/googletest-output-test-golden-lin.txt @@ -970,7 +970,6 @@ googletest-output-test_.cc:#: Skipped [----------] 1 test from TestSuiteThatSkipsInSetUp googletest-output-test_.cc:#: Skipped Skip entire test suite -Stack trace: (omitted) [ RUN ] TestSuiteThatSkipsInSetUp.ShouldNotRun googletest-output-test_.cc:#: Skipped diff --git a/googletest/test/gtest_xml_output_unittest.py b/googletest/test/gtest_xml_output_unittest.py index 422569e4..c3fea2c0 100755 --- a/googletest/test/gtest_xml_output_unittest.py +++ b/googletest/test/gtest_xml_output_unittest.py @@ -112,20 +112,23 @@ Invalid characters in brackets []%(stack)s]]> - + - + - +