Add {ASSERT,EXPECT}_DCHECK macros for tests

Add {ASSERT,EXPECT}_DCHECK macros for tests

The existing {ASSERT,EXPECT}_DFATAL macros were not suitable for DCHECKs
since for release builds where DCHECK_IS_ON(), the DCHECK still runs at
LOG_DCHECK severity which is FATAL, whereas the *_DFATAL macros are
expecting LOG_DFATAL severity which is only ERROR in release builds.

BUG=none

Committed: https://crrev.com/4501a135f39f0acb7f3864f188bae59f62fd64d5
Cr-Commit-Position: refs/heads/master@{#410330}

[johnme, 2016-08-05 13:27:53]

johnme@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[johnme, 2016-08-05 13:28:48]

I've factored this patch out from https://codereview.chromium.org/2111973002/,
which will introduce the first usage of the EXPECT_DCHECK macro.

[johnme, 2016-08-05 13:30:13]

On 2016/08/05 13:28:48, johnme wrote:
> I've factored this patch out from https://codereview.chromium.org/2111973002/,
> which will introduce the first usage of the EXPECT_DCHECK macro.

in test GetTokenWithAndWithoutSubtype of
https://codereview.chromium.org/2111973002/diff/120001/components/gcm_driver/...

[Paweł Hajdan Jr., 2016-08-05 14:35:07]

https://codereview.chromium.org/2217803003/diff/1/net/test/gtest_util.h
File net/test/gtest_util.h (right):

https://codereview.chromium.org/2217803003/diff/1/net/test/gtest_util.h#newco...
net/test/gtest_util.h:133: if (true) {                           \
Could you explain more this weird "if (true)" code? Is it already present
anywhere else?

[johnme, 2016-08-05 16:57:37]

johnme@chromium.org changed reviewers:
+ rch@chromium.org

[johnme, 2016-08-05 16:57:37]

+rch FYI, as I'm refactoring your macros and you may know of reasons why they
were the way they were

https://codereview.chromium.org/2217803003/diff/1/net/test/gtest_util.h
File net/test/gtest_util.h (right):

https://codereview.chromium.org/2217803003/diff/1/net/test/gtest_util.h#newco...
net/test/gtest_util.h:133: if (true) {                           \
On 2016/08/05 14:35:07, Paweł Hajdan Jr. wrote:
> Could you explain more this weird "if (true)" code? Is it already present
> anywhere else?

I copy-pasted from EXPECT_DEBUG_DFATAL and ASSERT_DEBUG_DFATAL above. The
intention is to run |statement|, whilst preventing |regex| being considered
unused. The if (true) { ... } else is just a macro wrapper
(http://stackoverflow.com/questions/154136).

But I agree that it's problematic:
- it's unnecessarily complicated/confusing
- GTEST_AMBIGUOUS_ELSE_BLOCKER_ (to avoid a "dangling else") is documented as
being for use internal to gtest only
- it's unclear why there's an unreachable GTEST_NONFATAL_FAILURE_("") at the end

So I've updated both these and the ones above to use a more typical do { ... }
while(false) macro wrapper (which is more common in chrome).

[Paweł Hajdan Jr., 2016-08-08 09:02:28]

LGTM

[johnme, 2016-08-08 10:55:53]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2016-08-08 10:56:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-08 11:45:36]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-08 11:47:40]

Description was changed from

==========
Add {ASSERT,EXPECT}_DCHECK macros for tests

The existing {ASSERT,EXPECT}_DFATAL macros were not suitable for DCHECKs
since for release builds where DCHECK_IS_ON(), the DCHECK still runs at
LOG_DCHECK severity which is FATAL, whereas the *_DFATAL macros are
expecting LOG_DFATAL severity which is only ERROR in release builds.

BUG=none
==========

to

==========
Add {ASSERT,EXPECT}_DCHECK macros for tests

The existing {ASSERT,EXPECT}_DFATAL macros were not suitable for DCHECKs
since for release builds where DCHECK_IS_ON(), the DCHECK still runs at
LOG_DCHECK severity which is FATAL, whereas the *_DFATAL macros are
expecting LOG_DFATAL severity which is only ERROR in release builds.

BUG=none

Committed: https://crrev.com/4501a135f39f0acb7f3864f188bae59f62fd64d5
Cr-Commit-Position: refs/heads/master@{#410330}
==========

[commit-bot: I haz the power, 2016-08-08 11:47:41]

Patchset 2 (id:??) landed as
https://crrev.com/4501a135f39f0acb7f3864f188bae59f62fd64d5
Cr-Commit-Position: refs/heads/master@{#410330}