TaskScheduler [5/9] Task Tracker

base/task_scheduler/test_utils.h

Index: base/task_scheduler/test_utils.h
diff --git a/base/task_scheduler/test_utils.h b/base/task_scheduler/test_utils.h
index bafd09aa2a294acd38794af431cbc5901f34ccd0..74eb4b0c6909601386f4bb967bd9f060277b0e9f 100644
--- a/base/task_scheduler/test_utils.h
+++ b/base/task_scheduler/test_utils.h
@@ -13,7 +13,7 @@
 #if DCHECK_IS_ON() && defined(GTEST_HAS_DEATH_TEST) && !defined(OS_ANDROID)
 #define EXPECT_DCHECK_DEATH(statement, regex) EXPECT_DEATH(statement, regex)
 #else
-#define EXPECT_DCHECK_DEATH(statement, regex)
+#define EXPECT_DCHECK_DEATH(statement, regex) statement

[gab, 2016/03/21 17:42:30]

Thought you'd convinced me not to do this? What changed?

[fdoray, 2016/03/21 19:08:07]

I explained why we sometimes put multiple statements inside the
EXPECT_DCHECK_DEATH macro [1], but I never said that we shouldn't execute
|statement| when !DCHECK_IS_ON(). Pros/cons:

We should always run |statement|:
- To be consistent with EXPECT_DEBUG_DEATH() [2].
- To be able to verify the state after a DCHECK failure. For example, I would
like to verify that BLOCK_SHUTDOWN tasks posted after shutdown don't run in a
!DCHECK_IS_ON() build.

We should only run |statement| when DCHECK_IS_ON():
- There is no need to verify what happens after a DCHECK because the style guide
says that a DCHECK verifies a condition that "must always be true". The state
after a DCHECK might be invalid.

Note that existing SchedulerLock tests pass regardless of whether |statement| is
executed.

[1]
https://codereview.chromium.org/1706123002/diff/400001/base/task_scheduler/sc...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/testing/gtest/incl...

[gab, 2016/03/21 19:53:25]

Hmm, this is surprising, that probably at the very least makes them flaky. The
DCHECK are catching potential for race conditions, by running this code we run
racy code which is probably undeterministically flaky :-(.

I'd say testing what happens after a DCHECK doesn't make sense (i.e. +1 to your
second point).

 #endif
 
 #endif  // BASE_TASK_SCHEDULER_TEST_UTILS_H_