Move EXPECT_DCHECK_DEATH from base/task_scheduler and use it in relevant base/ tests

base/test/gtest_util.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_TEST_GTEST_UTIL_H_
 #define BASE_TEST_GTEST_UTIL_H_
 
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/compiler_specific.h"
+#include "base/logging.h"
+#include "build/build_config.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+// EXPECT/ASSERT_DCHECK_DEATH is intended to replace EXPECT/ASSERT_DEBUG_DEATH
+// when the death is expected to be caused by a DCHECK. Contrary to
+// EXPECT/ASSERT_DEBUG_DEATH however, it doesn't execute the statement in non-
+// dcheck builds as DCHECKs are intended to catch things that should never
+// happen and as such executing the statement results in undefined behavior
+// (|statement| is compiled in unsupported configurations nonetheless).
+// Death tests misbehave on Android.
+#if DCHECK_IS_ON() && defined(GTEST_HAS_DEATH_TEST) && !defined(OS_ANDROID)
+
+#define EXPECT_DCHECK_DEATH(statement, regex) EXPECT_DEATH(statement, regex)
+#define ASSERT_DCHECK_DEATH(statement, regex) ASSERT_DEATH(statement, regex)
+
+#else
+// DCHECK_IS_ON() && defined(GTEST_HAS_DEATH_TEST) && !defined(OS_ANDROID)
+
+// Macro copied from gtest-death-test-internal.h as it's (1) internal for now
+// and (2) only defined if !GTEST_HAS_DEATH_TEST which is only a subset of the
+// conditions in which it's needed here.
+// TODO(gab): Expose macro in upstream gtest repo for consumers like us that
+// want more specific death tests and remove this hack.
+# define GTEST_UNSUPPORTED_DEATH_TEST(statement, regex, terminator) \
+    GTEST_AMBIGUOUS_ELSE_BLOCKER_ \

[danakj, 2016/07/22 21:15:46]

While I understand you copy/pasted this out of gtest, I'd prefer we drop
references to more internal gtest things if possible (and change "copied from"
to "inspired by" i guess).

Do we need this ELSE_BLOCKER_ for our tests?

Can we get away without the call to PartialMatch?

Can we make our own AlwaysTrue/AlwaysFalse and use them (also the
SUPPRESS_UNREACHABLE is just another AlwaysTrue).

[gab, 2016/07/25 15:46:21]

I haven't looked into this macro closely so the answer to most of the above is
"I don't know"... I'd much rather copy it as-is and replace it with the gtest
one soon (already working on making this method public in gtest
https://critique.corp.google.com/#review/128201478 as well as working on rolling
gtest http://crbug.com/630705) then try to make our own version.

[danakj, 2016/07/25 18:45:59]

In the event that it all goes to ashes, I just prefer we do this as simply as
possible, and not depend on more internal gtest things.

AlwaysTrue is simply a function that returns true:
https://cs.chromium.org/chromium/src/testing/gtest/src/gtest.cc?rcl=146944535...

AlwaysFalse() is !AlwaysTrue():
https://cs.chromium.org/chromium/src/testing/gtest/include/gtest/internal/gte...

You could probably do these with lambdas inside the macro if u wanted to (scoped
inside a do {} while(0))

The macro is not that complicated when you break it down. I don't think we ever
write if (asda) EXPECT_DCHECK_DEATH in a test, so we can drop the ELSE_BLOCKER.

No one has written EXPECT_DCHECK_DEATH << "stuff", which would do nothing with
the stuff anyways, so we could drop the last else and use do{}while(0).

GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_ is just if (AlwaysTrue()) {
statement }:
https://cs.chromium.org/chromium/src/testing/gtest/include/gtest/internal/gte...

Or we could wait a bit and see how cl/128201478 goes.

[gab, 2016/07/27 17:00:50]

https://cl/128201478 is approved and we have consensus on
http://crbug.com/630705 on how to perform the gtest roll. And even if it somehow
goes to ashes, I guess the TODO above puts me on the hook for fixing it (but I
don't see how it's possible that this would break without my gtest CL being
around as it would require rolling gtest to a version that doesn't support these
internal methods but also doesn't have my CL -- the current version of gtest
still has the same copy of this macro as our DEPS from a year ago).

+    if (::testing::internal::AlwaysTrue()) { \
+      GTEST_LOG_(WARNING) \
+          << "Death tests are not supported on this platform.\n" \
+          << "Statement '" #statement "' cannot be verified."; \
+    } else if (::testing::internal::AlwaysFalse()) { \
+      ::testing::internal::RE::PartialMatch(".*", (regex)); \
+      GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_(statement); \
+      terminator; \
+    } else \

[danakj, 2016/07/22 21:15:46]

nit: {}

[gab, 2016/07/25 15:46:21]

This appears to be intentional. It doesn't compile with {} as this is this
macro's way of forcing a semi-colon after it... We could instead wrap the whole
thing in do {...} while(0), but as mentioned above, I hate advanced macros and
would much rather copy this one as-is than try to out fancy it...

+      ::testing::Message()
+
+#define EXPECT_DCHECK_DEATH(statement, regex) \
+    GTEST_UNSUPPORTED_DEATH_TEST(statement, regex, )
+#define ASSERT_DCHECK_DEATH(statement, regex) \
+    GTEST_UNSUPPORTED_DEATH_TEST(statement, regex, return)
+
+#undef GTEST_UNSUPPORTED_DEATH_TEST
+
+#endif
+// DCHECK_IS_ON() && defined(GTEST_HAS_DEATH_TEST) && !defined(OS_ANDROID)
 
 namespace base {
 
 class FilePath;
 
 struct TestIdentifier {
   TestIdentifier();
   TestIdentifier(const TestIdentifier& other);
 
   std::string test_case_name;
@@ skipping 17 matching lines @@
 
 // Reads the list of gtest-based tests from |path| into |output|.
 // Returns true on success.
 bool ReadTestNamesFromFile(
     const FilePath& path,
     std::vector<TestIdentifier>* output) WARN_UNUSED_RESULT;
 
 }  // namespace base
 
 #endif  // BASE_TEST_GTEST_UTIL_H_