Do not emit stack traces for messages generated by SUCCEED()

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), SUCCEED() is a developer-authored
directive that indicates a success codepath. In fact, the documentation
states that this directive doesn't generate any output. Generating stack
traces for uses of SUCCEED() is wasted work since they are never
printed. If this were to change one day in the future, they still would
not be useful since any emitted message would include the file and line
number where SUCCEED was used. In addition to being noise in the output
in this case, symbolization of stack traces is not free. In some
Chromium configurations, symbolization for use of SUCCEED() can incur a
cost in excess of 25 seconds for a test that otherwise takes 0-1ms; see
https://crbug.com/1517343.

In this CL, we suppress generation and emission of stack traces for
kSuccess messages to reduce the overhead of SUCCEED().

PiperOrigin-RevId: 602832162
Change-Id: I557dd6a1d3e6ed6562daf727d69fd01fe914827b

2 files changed, 14 insertions, 8 deletions

diff --git a/googletest/src/gtest.cc b/googletest/src/gtest.cc
index 7dc8059..6d6ff1b 100644
--- a/googletest/src/gtest.cc
+++ b/googletest/src/gtest.cc
@@ -451,6 +451,19 @@ static bool ShouldRunTestSuite(const TestSuite* test_suite) {
   return test_suite->should_run();
 }
 
+namespace {
+
+// Returns true if test part results of type `type` should include a stack
+// trace.
+bool ShouldEmitStackTraceForResultType(TestPartResult::Type type) {
+  // Suppress emission of the stack trace for SUCCEED() since it likely never
+  // requires investigation, and GTEST_SKIP() since skipping is an intentional
+  // act by the developer rather than a failure requiring investigation.
+  return type != TestPartResult::kSuccess && type != TestPartResult::kSkip;
+}
+
+}  // namespace
+
 // AssertHelper constructor.
 AssertHelper::AssertHelper(TestPartResult::Type type, const char* file,
                            int line, const char* message)
@@ -463,10 +476,7 @@ void AssertHelper::operator=(const Message& message) const {
   UnitTest::GetInstance()->AddTestPartResult(
       data_->type, data_->file, data_->line,
       AppendUserMessage(data_->message, message),
-      // 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
+      ShouldEmitStackTraceForResultType(data_->type)
           ? UnitTest::GetInstance()->impl()->CurrentOsStackTraceExceptTop(1)
           : ""
       // Skips the stack frame for this function itself.
diff --git a/googletest/test/googletest-output-test-golden-lin.txt b/googletest/test/googletest-output-test-golden-lin.txt
index e06856a..533eb8c 100644
--- a/googletest/test/googletest-output-test-golden-lin.txt
+++ b/googletest/test/googletest-output-test-golden-lin.txt
@@ -696,7 +696,6 @@ Expected: 1 fatal failure
   Actual:
 googletest-output-test_.cc:#: Success:
 Succeeded
-Stack trace: (omitted)
 
 
 Stack trace: (omitted)
@@ -733,7 +732,6 @@ Expected: 1 non-fatal failure
   Actual:
 googletest-output-test_.cc:#: Success:
 Succeeded
-Stack trace: (omitted)
 
 
 Stack trace: (omitted)
@@ -770,7 +768,6 @@ Expected: 1 fatal failure
   Actual:
 googletest-output-test_.cc:#: Success:
 Succeeded
-Stack trace: (omitted)
 
 
 Stack trace: (omitted)
@@ -807,7 +804,6 @@ Expected: 1 non-fatal failure
   Actual:
 googletest-output-test_.cc:#: Success:
 Succeeded
-Stack trace: (omitted)
 
 
 Stack trace: (omitted)