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
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 @@
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 @@
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 @@
Actual:
googletest-output-test_.cc:#: Success:
Succeeded
-Stack trace: (omitted)
Stack trace: (omitted)
@@ -733,7 +732,6 @@
Actual:
googletest-output-test_.cc:#: Success:
Succeeded
-Stack trace: (omitted)
Stack trace: (omitted)
@@ -770,7 +768,6 @@
Actual:
googletest-output-test_.cc:#: Success:
Succeeded
-Stack trace: (omitted)
Stack trace: (omitted)
@@ -807,7 +804,6 @@
Actual:
googletest-output-test_.cc:#: Success:
Succeeded
-Stack trace: (omitted)
Stack trace: (omitted)
