Skip to content

Commit 6fdb4c3

Browse files
Abseil Teamcopybara-github
authored andcommitted
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
1 parent fc0076f commit 6fdb4c3

File tree

2 files changed

+14
-8
lines changed

2 files changed

+14
-8
lines changed

googletest/src/gtest.cc

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,19 @@ static bool ShouldRunTestSuite(const TestSuite* test_suite) {
451451
return test_suite->should_run();
452452
}
453453

454+
namespace {
455+
456+
// Returns true if test part results of type `type` should include a stack
457+
// trace.
458+
bool ShouldEmitStackTraceForResultType(TestPartResult::Type type) {
459+
// Suppress emission of the stack trace for SUCCEED() since it likely never
460+
// requires investigation, and GTEST_SKIP() since skipping is an intentional
461+
// act by the developer rather than a failure requiring investigation.
462+
return type != TestPartResult::kSuccess && type != TestPartResult::kSkip;
463+
}
464+
465+
} // namespace
466+
454467
// AssertHelper constructor.
455468
AssertHelper::AssertHelper(TestPartResult::Type type, const char* file,
456469
int line, const char* message)
@@ -463,10 +476,7 @@ void AssertHelper::operator=(const Message& message) const {
463476
UnitTest::GetInstance()->AddTestPartResult(
464477
data_->type, data_->file, data_->line,
465478
AppendUserMessage(data_->message, message),
466-
// Suppress emission of the stack trace for GTEST_SKIP() since skipping is
467-
// an intentional act by the developer rather than a failure requiring
468-
// investigation.
469-
data_->type != TestPartResult::kSkip
479+
ShouldEmitStackTraceForResultType(data_->type)
470480
? UnitTest::GetInstance()->impl()->CurrentOsStackTraceExceptTop(1)
471481
: ""
472482
// Skips the stack frame for this function itself.

googletest/test/googletest-output-test-golden-lin.txt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -696,7 +696,6 @@ Expected: 1 fatal failure
696696
Actual:
697697
googletest-output-test_.cc:#: Success:
698698
Succeeded
699-
Stack trace: (omitted)
700699

701700

702701
Stack trace: (omitted)
@@ -733,7 +732,6 @@ Expected: 1 non-fatal failure
733732
Actual:
734733
googletest-output-test_.cc:#: Success:
735734
Succeeded
736-
Stack trace: (omitted)
737735

738736

739737
Stack trace: (omitted)
@@ -770,7 +768,6 @@ Expected: 1 fatal failure
770768
Actual:
771769
googletest-output-test_.cc:#: Success:
772770
Succeeded
773-
Stack trace: (omitted)
774771

775772

776773
Stack trace: (omitted)
@@ -807,7 +804,6 @@ Expected: 1 non-fatal failure
807804
Actual:
808805
googletest-output-test_.cc:#: Success:
809806
Succeeded
810-
Stack trace: (omitted)
811807

812808

813809
Stack trace: (omitted)

0 commit comments

Comments
 (0)