Commit e9421ec4 authored by Gabriel Charette's avatar Gabriel Charette Committed by Commit Bot

[tests] Fix death tests under dcheck_is_configurable.

Prior to this CL: all death tests had been failing on win-asan
(dcheck_is_configurable = true) for a while with error
"Result: failed to die.".

LOG_DCHECK defaults to LOG_INFO severity in these builds, with severity
upgraded to LOG_FATAL by FeatureList::SetInstance() if
--gtest_internal_run_death_test is in the process' command-line.
However, SetInstance() is only invoked on a per-test basis via a
TestEventListener, while TestEventListeners are deliberately skipped in
death-test sub-processes. This results in death-test sub-processes
running with non-FATAL DCHECKs, causing death-test expectations to fail.

Full diagnosis @ crbug.com/1057995#c11

This CL changes the production default (LOG_INFO) to a test default
(LOG_FATAL) in --gtest_internal_run_death_test processes under
DCHECK_IS_CONFIGURABLE.

R=chromium-metrics-reviews@google.com, wez@chromium.org

Bug: 1057995
Change-Id: Ia7d3f92d72dddc74e17cc9416c0b586dae1fc221
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2208298
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: default avatarWez <wez@chromium.org>
Auto-Submit: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771124}
parent 3427cf46
...@@ -321,6 +321,9 @@ void FeatureList::SetInstance(std::unique_ptr<FeatureList> instance) { ...@@ -321,6 +321,9 @@ void FeatureList::SetInstance(std::unique_ptr<FeatureList> instance) {
#if defined(DCHECK_IS_CONFIGURABLE) #if defined(DCHECK_IS_CONFIGURABLE)
// Update the behaviour of LOG_DCHECK to match the Feature configuration. // Update the behaviour of LOG_DCHECK to match the Feature configuration.
// DCHECK is also forced to be FATAL if we are running a death-test. // DCHECK is also forced to be FATAL if we are running a death-test.
// TODO(crbug.com/1057995#c11): --gtest_internal_run_death_test doesn't
// currently run through this codepath, mitigated in
// base::TestSuite::Initialize() for now.
// TODO(asvitkine): If we find other use-cases that need integrating here // TODO(asvitkine): If we find other use-cases that need integrating here
// then define a proper API/hook for the purpose. // then define a proper API/hook for the purpose.
if (FeatureList::IsEnabled(kDCheckIsFatalFeature) || if (FeatureList::IsEnabled(kDCheckIsFatalFeature) ||
......
...@@ -590,6 +590,16 @@ void TestSuite::Initialize() { ...@@ -590,6 +590,16 @@ void TestSuite::Initialize() {
} }
#endif #endif
#if defined(DCHECK_IS_CONFIGURABLE)
// Default the configurable DCHECK level to FATAL when running death tests'
// child process, so that they behave as expected.
// TODO(crbug.com/1057995): Remove this in favor of the codepath in
// FeatureList::SetInstance() when/if OnTestStart() TestEventListeners
// are fixed to be invoked in the child process as expected.
if (command_line->HasSwitch("gtest_internal_run_death_test"))
logging::LOG_DCHECK = logging::LOG_FATAL;
#endif
#if defined(OS_IOS) #if defined(OS_IOS)
InitIOSTestMessageLoop(); InitIOSTestMessageLoop();
#endif // OS_IOS #endif // OS_IOS
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment