Make CHECK_EQ and friends work properly in an if/else structure without braces.

base/logging_unittest.cc

 // Copyright (c) 2011 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.
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ skipping 216 matching lines @@
 
 TEST_F(LoggingTest, DcheckReleaseBehavior) {
   int some_variable = 1;
   // These should still reference |some_variable| so we don't get
   // unused variable warnings.
   DCHECK(some_variable) << "test";
   DPCHECK(some_variable) << "test";
   DCHECK_EQ(some_variable, 1) << "test";
 }
 
+TEST_F(LoggingTest, DCheckStatements) {
+  SetLogAssertHandler(&LogSink);
+
+  EXPECT_EQ(0, log_sink_call_count);
+
+  bool reached = false;

[Sigurður Ásgeirsson, 2015/06/25 12:25:09]

It's totally opaque what's being tested here and to what end. I dare say that if
this test were to ever fail, I'd be tempted to "fix" the test.
Some running commentary, perhaps?

[erikwright (departed), 2015/07/07 18:30:56]

I've updated this particular test case with some syntactic sugar.

Is it easier to understand now, or does it need commenting still?

Based on your feedback I'll update the others.

+  if (false)
+    DCHECK(false);
+  else
+    DCHECK(reached = true);
+  ASSERT_EQ(DCHECK_IS_ON(), reached);
+
+  reached = false;
+  if (true)
+    DCHECK(reached = true);
+  else
+    DCHECK(false);
+  ASSERT_EQ(DCHECK_IS_ON(), reached);
+
+  reached = false;
+  switch (2) {
+    case 1:
+      DCHECK(false);
+    case 2:
+      DCHECK(reached=true);
+    default:
+      break;
+  }
+  ASSERT_EQ(DCHECK_IS_ON(), reached);
+
+  // Cases with failing checks.
+  EXPECT_EQ(0, log_sink_call_count);
+
+  reached = false;
+  if (false)
+    DCHECK(reached = true);
+  else
+    DCHECK(false);
+  ASSERT_FALSE(reached);
+  ASSERT_EQ(DCHECK_IS_ON() ? 1 : 0, log_sink_call_count);
+
+  reached = false;
+  if (true)
+    DCHECK(false);
+  else
+    DCHECK(reached = true);
+  ASSERT_FALSE(reached);
+  ASSERT_EQ(DCHECK_IS_ON() ? 2 : 0, log_sink_call_count);
+
+  reached = false;
+  switch (2) {
+    case 1:
+      DCHECK(reached = true);
+    case 2:
+      DCHECK(false);
+    default:
+      break;
+  }
+  ASSERT_FALSE(reached);
+  ASSERT_EQ(DCHECK_IS_ON() ? 3 : 0, log_sink_call_count);
+}
+
+TEST_F(LoggingTest, CheckStatements) {
+  SetLogAssertHandler(&LogSink);
+
+  EXPECT_EQ(0, log_sink_call_count);
+
+  bool reached = false;
+  if (false)
+    CHECK(false);
+  else
+    CHECK(reached = true);
+  ASSERT_TRUE(reached);
+
+  reached = false;
+  if (true)
+    CHECK(reached = true);
+  else
+    CHECK(false);
+  ASSERT_TRUE(reached);
+
+  reached = false;
+  switch (2) {
+    case 1:
+      CHECK(false);
+    case 2:
+      CHECK(reached=true);
+    default:
+      break;
+  }
+  ASSERT_TRUE(reached);
+
+  // Cases with failing checks.
+  EXPECT_EQ(0, log_sink_call_count);
+
+  reached = false;
+  if (false)
+    CHECK(reached = true);
+  else
+    CHECK(false);
+  ASSERT_FALSE(reached);
+  ASSERT_EQ(1, log_sink_call_count);
+
+  reached = false;
+  if (true)
+    CHECK(false);
+  else
+    CHECK(reached = true);
+  ASSERT_FALSE(reached);
+  ASSERT_EQ(2, log_sink_call_count);
+
+  reached = false;
+  switch (2) {
+    case 1:
+      CHECK(reached = true);
+    case 2:
+      CHECK(false);
+    default:
+      break;
+  }
+  ASSERT_FALSE(reached);
+  ASSERT_EQ(3, log_sink_call_count);
+}
+
+TEST_F(LoggingTest, DCheckEqStatements) {
+  SetLogAssertHandler(&LogSink);
+
+  EXPECT_EQ(0, log_sink_call_count);
+
+  bool reached = false;
+  if (false)
+    DCHECK_EQ(false, true);
+  else
+    DCHECK_EQ(true, reached = true);
+  ASSERT_EQ(DCHECK_IS_ON(), reached);
+
+  reached = false;
+  if (true)
+    DCHECK_EQ(true, reached = true);
+  else
+    DCHECK_EQ(false, true);
+  ASSERT_EQ(DCHECK_IS_ON(), reached);
+
+  reached = false;
+  switch (2) {
+    case 1:
+      DCHECK_EQ(false, true);
+    case 2:
+      DCHECK_EQ(true, reached=true);
+    default:
+      break;
+  }
+  ASSERT_EQ(DCHECK_IS_ON(), reached);
+
+  // Cases with failing checks.
+  EXPECT_EQ(0, log_sink_call_count);
+
+  reached = false;
+  if (false)
+    DCHECK_EQ(true, reached = true);
+  else
+    DCHECK_EQ(false, true);
+  ASSERT_FALSE(reached);
+  ASSERT_EQ(DCHECK_IS_ON() ? 1 : 0, log_sink_call_count);
+
+  reached = false;
+  if (true)
+    DCHECK_EQ(false, true);
+  else
+    DCHECK_EQ(true, reached = true);
+  ASSERT_FALSE(reached);
+  ASSERT_EQ(DCHECK_IS_ON() ? 2 : 0, log_sink_call_count);
+
+  reached = false;
+  switch (2) {
+    case 1:
+      DCHECK_EQ(true, reached = true);
+    case 2:
+      DCHECK_EQ(false, true);
+    default:
+      break;
+  }
+  ASSERT_FALSE(reached);
+  ASSERT_EQ(DCHECK_IS_ON() ? 3 : 0, log_sink_call_count);
+}
+
+TEST_F(LoggingTest, CheckEqStatements) {
+  SetLogAssertHandler(&LogSink);
+
+  EXPECT_EQ(0, log_sink_call_count);
+
+  bool reached = false;
+  if (false)
+    CHECK_EQ(false, true);
+  else
+    CHECK_EQ(true, reached = true);
+  ASSERT_TRUE(reached);
+
+  reached = false;
+  if (true)
+    CHECK_EQ(true, reached = true);
+  else
+    CHECK_EQ(false, true);
+  ASSERT_TRUE(reached);
+
+  reached = false;
+  switch (2) {
+    case 1:
+      CHECK_EQ(false, true);
+    case 2:
+      CHECK_EQ(true, reached = true);
+    default:
+      break;
+  }
+  ASSERT_TRUE(reached);
+
+  // Cases with failing checks.
+  EXPECT_EQ(0, log_sink_call_count);
+
+  reached = false;
+  if (false)
+    CHECK_EQ(true, reached = true);
+  else
+    CHECK_EQ(false, true);
+  ASSERT_FALSE(reached);
+  ASSERT_EQ(1, log_sink_call_count);
+
+  reached = false;
+  if (true)
+    CHECK_EQ(false, true);
+  else
+    CHECK_EQ(true, reached = true);
+  ASSERT_FALSE(reached);
+  ASSERT_EQ(2, log_sink_call_count);
+
+  reached = false;
+  switch (2) {
+    case 1:
+      CHECK_EQ(true, reached = true);
+    case 2:
+      CHECK_EQ(false, true);
+    default:
+      break;
+  }
+  ASSERT_FALSE(reached);
+  ASSERT_EQ(3, log_sink_call_count);
+}
+
+TEST_F(LoggingTest, LogStatements) {
+  bool reached = false;
+  if (true)
+    LOG(INFO) << "Hello: " << (reached = true);
+  else
+    LOG(FATAL) << "Goodbye.";
+  ASSERT_TRUE(reached);
+
+  reached = false;
+  if (false)
+    LOG(FATAL) << "Goodbye.";
+  else
+    LOG(INFO) << "Hello: " << (reached = true);
+  ASSERT_TRUE(reached);
+
+  reached = false;
+  if (true)
+    LOG_IF(INFO, reached = true) << "Hello.";
+  else
+    LOG_IF(FATAL, true) << "Goodbye.";
+  ASSERT_TRUE(reached);
+
+  reached = false;
+  if (false)
+    LOG_IF(FATAL, true) << "Goodbye.";
+  else
+    LOG_IF(INFO, reached = true) << "Hello.";
+  ASSERT_TRUE(reached);
+
+  reached = false;
+  if (false)
+    LOG_IF(FATAL, true) << "Goodbye.";
+  else
+    LOG_IF(INFO, false) << "Derp: " << (reached = true);
+  ASSERT_FALSE(reached);
+
+  reached = false;
+  if (true)
+    DLOG(INFO) << "Hello: " << (reached = true);
+  else
+    DLOG(FATAL) << "Goodbye.";
+  ASSERT_EQ(DLOG_IS_ON(INFO), reached);
+
+  reached = false;
+  if (false)
+    DLOG(FATAL) << "Goodbye.";
+  else
+    DLOG(INFO) << "Hello: " << (reached = true);
+  ASSERT_EQ(DLOG_IS_ON(INFO), reached);
+
+  reached = false;
+  if (true)
+    DLOG_IF(INFO, true) << "Hello: " << (reached = true);
+  else
+    DLOG_IF(FATAL, true) << "Goodbye.";
+  ASSERT_EQ(DLOG_IS_ON(INFO), reached);
+
+  reached = false;
+  if (false)
+    DLOG_IF(FATAL, true) << "Goodbye.";
+  else
+    DLOG_IF(INFO, true) << "Hello: " << (reached = true);
+  ASSERT_EQ(DLOG_IS_ON(INFO), reached);
+
+  reached = false;
+  if (false)
+    DLOG_IF(FATAL, true) << "Goodbye.";
+  else
+    DLOG_IF(INFO, false) << "Derp: " << (reached = true);
+  ASSERT_FALSE(reached);
+}
+
 // Test that defining an operator<< for a type in a namespace doesn't prevent
 // other code in that namespace from calling the operator<<(ostream, wstring)
 // defined by logging.h. This can fail if operator<<(ostream, wstring) can't be
 // found by ADL, since defining another operator<< prevents name lookup from
 // looking in the global namespace.
 namespace nested_test {
   class Streamable {};
   ALLOW_UNUSED_TYPE std::ostream& operator<<(std::ostream& out,
                                              const Streamable&) {
     return out << "Streamable";
   }
   TEST_F(LoggingTest, StreamingWstringFindsCorrectOperator) {
     std::wstring wstr = L"Hello World";
     std::ostringstream ostr;
     ostr << wstr;
     EXPECT_EQ("Hello World", ostr.str());
   }
 }  // namespace nested_test
 
 }  // namespace
 
 }  // namespace logging