Improves gtest's failure messages. In particulars, char pointers and

char arrays are not escapped properly.

5 files changed, 330 insertions, 36 deletions

diff --git a/test/gtest-port_test.cc b/test/gtest-port_test.cc
index 75471c39..dfbf029f 100644
--- a/test/gtest-port_test.cc
+++ b/test/gtest-port_test.cc
@@ -61,6 +61,43 @@ using std::pair;
 namespace testing {
 namespace internal {
 
+TEST(IsXDigitTest, WorksForNarrowAscii) {
+  EXPECT_TRUE(IsXDigit('0'));
+  EXPECT_TRUE(IsXDigit('9'));
+  EXPECT_TRUE(IsXDigit('A'));
+  EXPECT_TRUE(IsXDigit('F'));
+  EXPECT_TRUE(IsXDigit('a'));
+  EXPECT_TRUE(IsXDigit('f'));
+
+  EXPECT_FALSE(IsXDigit('-'));
+  EXPECT_FALSE(IsXDigit('g'));
+  EXPECT_FALSE(IsXDigit('G'));
+}
+
+TEST(IsXDigitTest, ReturnsFalseForNarrowNonAscii) {
+  EXPECT_FALSE(IsXDigit(static_cast<char>(0x80)));
+  EXPECT_FALSE(IsXDigit(static_cast<char>('0' | 0x80)));
+}
+
+TEST(IsXDigitTest, WorksForWideAscii) {
+  EXPECT_TRUE(IsXDigit(L'0'));
+  EXPECT_TRUE(IsXDigit(L'9'));
+  EXPECT_TRUE(IsXDigit(L'A'));
+  EXPECT_TRUE(IsXDigit(L'F'));
+  EXPECT_TRUE(IsXDigit(L'a'));
+  EXPECT_TRUE(IsXDigit(L'f'));
+
+  EXPECT_FALSE(IsXDigit(L'-'));
+  EXPECT_FALSE(IsXDigit(L'g'));
+  EXPECT_FALSE(IsXDigit(L'G'));
+}
+
+TEST(IsXDigitTest, ReturnsFalseForWideNonAscii) {
+  EXPECT_FALSE(IsXDigit(static_cast<wchar_t>(0x80)));
+  EXPECT_FALSE(IsXDigit(static_cast<wchar_t>(L'0' | 0x80)));
+  EXPECT_FALSE(IsXDigit(static_cast<wchar_t>(L'0' | 0x100)));
+}
+
 class Base {
  public:
   // Copy constructor and assignment operator do exactly what we need, so we
diff --git a/test/gtest-printers_test.cc b/test/gtest-printers_test.cc
index 58d96225..45610f8f 100644
--- a/test/gtest-printers_test.cc
+++ b/test/gtest-printers_test.cc
@@ -197,14 +197,15 @@ using ::std::pair;
 using ::std::set;
 using ::std::vector;
 using ::testing::PrintToString;
+using ::testing::internal::FormatForComparisonFailureMessage;
 using ::testing::internal::ImplicitCast_;
 using ::testing::internal::NativeArray;
 using ::testing::internal::RE;
 using ::testing::internal::Strings;
-using ::testing::internal::UniversalTersePrint;
 using ::testing::internal::UniversalPrint;
-using ::testing::internal::UniversalTersePrintTupleFieldsToStrings;
 using ::testing::internal::UniversalPrinter;
+using ::testing::internal::UniversalTersePrint;
+using ::testing::internal::UniversalTersePrintTupleFieldsToStrings;
 using ::testing::internal::kReference;
 using ::testing::internal::string;
 
@@ -613,17 +614,30 @@ TEST(PrintArrayTest, ConstArray) {
   EXPECT_EQ("{ false }", PrintArrayHelper(a));
 }
 
-// Char array.
-TEST(PrintArrayTest, CharArray) {
+// char array without terminating NUL.
+TEST(PrintArrayTest, CharArrayWithNoTerminatingNul) {
+  // Array a contains '\0' in the middle and doesn't end with '\0'.
+  char a[] = { 'H', '\0', 'i' };
+  EXPECT_EQ("\"H\\0i\" (no terminating NUL)", PrintArrayHelper(a));
+}
+
+// const char array with terminating NUL.
+TEST(PrintArrayTest, ConstCharArrayWithTerminatingNul) {
+  const char a[] = "\0Hi";
+  EXPECT_EQ("\"\\0Hi\"", PrintArrayHelper(a));
+}
+
+// const wchar_t array without terminating NUL.
+TEST(PrintArrayTest, WCharArrayWithNoTerminatingNul) {
   // Array a contains '\0' in the middle and doesn't end with '\0'.
-  char a[3] = { 'H', '\0', 'i' };
-  EXPECT_EQ("\"H\\0i\"", PrintArrayHelper(a));
+  const wchar_t a[] = { L'H', L'\0', L'i' };
+  EXPECT_EQ("L\"H\\0i\" (no terminating NUL)", PrintArrayHelper(a));
 }
 
-// Const char array.
-TEST(PrintArrayTest, ConstCharArray) {
-  const char a[4] = "\0Hi";
-  EXPECT_EQ("\"\\0Hi\\0\"", PrintArrayHelper(a));
+// wchar_t array with terminating NUL.
+TEST(PrintArrayTest, WConstCharArrayWithTerminatingNul) {
+  const wchar_t a[] = L"\0Hi";
+  EXPECT_EQ("L\"\\0Hi\"", PrintArrayHelper(a));
 }
 
 // Array of objects.
@@ -1186,6 +1200,207 @@ TEST(PrintReferenceTest, HandlesMemberVariablePointer) {
       "@" + PrintPointer(&p) + " " + Print(sizeof(p)) + "-byte object "));
 }
 
+// Tests that FormatForComparisonFailureMessage(), which is used to print
+// an operand in a comparison assertion (e.g. ASSERT_EQ) when the assertion
+// fails, formats the operand in the desired way.
+
+// scalar
+TEST(FormatForComparisonFailureMessageTest, WorksForScalar) {
+  EXPECT_STREQ("123",
+               FormatForComparisonFailureMessage(123, 124).c_str());
+}
+
+// non-char pointer
+TEST(FormatForComparisonFailureMessageTest, WorksForNonCharPointer) {
+  int n = 0;
+  EXPECT_EQ(PrintPointer(&n),
+            FormatForComparisonFailureMessage(&n, &n).c_str());
+}
+
+// non-char array
+TEST(FormatForComparisonFailureMessageTest, FormatsNonCharArrayAsPointer) {
+  // In expression 'array == x', 'array' is compared by pointer.
+  // Therefore we want to print an array operand as a pointer.
+  int n[] = { 1, 2, 3 };
+  EXPECT_EQ(PrintPointer(n),
+            FormatForComparisonFailureMessage(n, n).c_str());
+}
+
+// Tests formatting a char pointer when it's compared with another pointer.
+// In this case we want to print it as a raw pointer, as the comparision is by
+// pointer.
+
+// char pointer vs pointer
+TEST(FormatForComparisonFailureMessageTest, WorksForCharPointerVsPointer) {
+  // In expression 'p == x', where 'p' and 'x' are (const or not) char
+  // pointers, the operands are compared by pointer.  Therefore we
+  // want to print 'p' as a pointer instead of a C string (we don't
+  // even know if it's supposed to point to a valid C string).
+
+  // const char*
+  const char* s = "hello";
+  EXPECT_EQ(PrintPointer(s),
+            FormatForComparisonFailureMessage(s, s).c_str());
+
+  // char*
+  char ch = 'a';
+  EXPECT_EQ(PrintPointer(&ch),
+            FormatForComparisonFailureMessage(&ch, &ch).c_str());
+}
+
+// wchar_t pointer vs pointer
+TEST(FormatForComparisonFailureMessageTest, WorksForWCharPointerVsPointer) {
+  // In expression 'p == x', where 'p' and 'x' are (const or not) char
+  // pointers, the operands are compared by pointer.  Therefore we
+  // want to print 'p' as a pointer instead of a wide C string (we don't
+  // even know if it's supposed to point to a valid wide C string).
+
+  // const wchar_t*
+  const wchar_t* s = L"hello";
+  EXPECT_EQ(PrintPointer(s),
+            FormatForComparisonFailureMessage(s, s).c_str());
+
+  // wchar_t*
+  wchar_t ch = L'a';
+  EXPECT_EQ(PrintPointer(&ch),
+            FormatForComparisonFailureMessage(&ch, &ch).c_str());
+}
+
+// Tests formatting a char pointer when it's compared to a string object.
+// In this case we want to print the char pointer as a C string.
+
+#if GTEST_HAS_GLOBAL_STRING
+// char pointer vs ::string
+TEST(FormatForComparisonFailureMessageTest, WorksForCharPointerVsString) {
+  const char* s = "hello \"world";
+  EXPECT_STREQ("\"hello \\\"world\"",  // The string content should be escaped.
+               FormatForComparisonFailureMessage(s, ::string()).c_str());
+
+  // char*
+  char str[] = "hi\1";
+  char* p = str;
+  EXPECT_STREQ("\"hi\\x1\"",  // The string content should be escaped.
+               FormatForComparisonFailureMessage(p, ::string()).c_str());
+}
+#endif
+
+// char pointer vs std::string
+TEST(FormatForComparisonFailureMessageTest, WorksForCharPointerVsStdString) {
+  const char* s = "hello \"world";
+  EXPECT_STREQ("\"hello \\\"world\"",  // The string content should be escaped.
+               FormatForComparisonFailureMessage(s, ::std::string()).c_str());
+
+  // char*
+  char str[] = "hi\1";
+  char* p = str;
+  EXPECT_STREQ("\"hi\\x1\"",  // The string content should be escaped.
+               FormatForComparisonFailureMessage(p, ::std::string()).c_str());
+}
+
+#if GTEST_HAS_GLOBAL_WSTRING
+// wchar_t pointer vs ::wstring
+TEST(FormatForComparisonFailureMessageTest, WorksForWCharPointerVsWString) {
+  const wchar_t* s = L"hi \"world";
+  EXPECT_STREQ("L\"hi \\\"world\"",  // The string content should be escaped.
+               FormatForComparisonFailureMessage(s, ::wstring()).c_str());
+
+  // wchar_t*
+  wchar_t str[] = L"hi\1";
+  wchar_t* p = str;
+  EXPECT_STREQ("L\"hi\\x1\"",  // The string content should be escaped.
+               FormatForComparisonFailureMessage(p, ::wstring()).c_str());
+}
+#endif
+
+#if GTEST_HAS_STD_WSTRING
+// wchar_t pointer vs std::wstring
+TEST(FormatForComparisonFailureMessageTest, WorksForWCharPointerVsStdWString) {
+  const wchar_t* s = L"hi \"world";
+  EXPECT_STREQ("L\"hi \\\"world\"",  // The string content should be escaped.
+               FormatForComparisonFailureMessage(s, ::std::wstring()).c_str());
+
+  // wchar_t*
+  wchar_t str[] = L"hi\1";
+  wchar_t* p = str;
+  EXPECT_STREQ("L\"hi\\x1\"",  // The string content should be escaped.
+               FormatForComparisonFailureMessage(p, ::std::wstring()).c_str());
+}
+#endif
+
+// Tests formatting a char array when it's compared with a pointer or array.
+// In this case we want to print the array as a row pointer, as the comparison
+// is by pointer.
+
+// char array vs pointer
+TEST(FormatForComparisonFailureMessageTest, WorksForCharArrayVsPointer) {
+  char str[] = "hi \"world\"";
+  char* p = NULL;
+  EXPECT_EQ(PrintPointer(str),
+            FormatForComparisonFailureMessage(str, p).c_str());
+}
+
+// char array vs char array
+TEST(FormatForComparisonFailureMessageTest, WorksForCharArrayVsCharArray) {
+  const char str[] = "hi \"world\"";
+  EXPECT_EQ(PrintPointer(str),
+            FormatForComparisonFailureMessage(str, str).c_str());
+}
+
+// wchar_t array vs pointer
+TEST(FormatForComparisonFailureMessageTest, WorksForWCharArrayVsPointer) {
+  wchar_t str[] = L"hi \"world\"";
+  wchar_t* p = NULL;
+  EXPECT_EQ(PrintPointer(str),
+            FormatForComparisonFailureMessage(str, p).c_str());
+}
+
+// wchar_t array vs wchar_t array
+TEST(FormatForComparisonFailureMessageTest, WorksForWCharArrayVsWCharArray) {
+  const wchar_t str[] = L"hi \"world\"";
+  EXPECT_EQ(PrintPointer(str),
+            FormatForComparisonFailureMessage(str, str).c_str());
+}
+
+// Tests formatting a char array when it's compared with a string object.
+// In this case we want to print the array as a C string.
+
+#if GTEST_HAS_GLOBAL_STRING
+// char array vs string
+TEST(FormatForComparisonFailureMessageTest, WorksForCharArrayVsString) {
+  const char str[] = "hi \"w\0rld\"";
+  EXPECT_STREQ("\"hi \\\"w\"",  // The content should be escaped.
+                                // Embedded NUL terminates the string.
+               FormatForComparisonFailureMessage(str, ::string()).c_str());
+}
+#endif
+
+// char array vs std::string
+TEST(FormatForComparisonFailureMessageTest, WorksForCharArrayVsStdString) {
+  const char str[] = "hi \"world\"";
+  EXPECT_STREQ("\"hi \\\"world\\\"\"",  // The content should be escaped.
+               FormatForComparisonFailureMessage(str, ::std::string()).c_str());
+}
+
+#if GTEST_HAS_GLOBAL_WSTRING
+// wchar_t array vs wstring
+TEST(FormatForComparisonFailureMessageTest, WorksForWCharArrayVsWString) {
+  const wchar_t str[] = L"hi \"world\"";
+  EXPECT_STREQ("L\"hi \\\"world\\\"\"",  // The content should be escaped.
+               FormatForComparisonFailureMessage(str, ::wstring()).c_str());
+}
+#endif
+
+#if GTEST_HAS_STD_WSTRING
+// wchar_t array vs std::wstring
+TEST(FormatForComparisonFailureMessageTest, WorksForWCharArrayVsStdWString) {
+  const wchar_t str[] = L"hi \"w\0rld\"";
+  EXPECT_STREQ(
+      "L\"hi \\\"w\"",  // The content should be escaped.
+                        // Embedded NUL terminates the string.
+      FormatForComparisonFailureMessage(str, ::std::wstring()).c_str());
+}
+#endif
+
 // Useful for testing PrintToString().  We cannot use EXPECT_EQ()
 // there as its implementation uses PrintToString().  The caller must
 // ensure that 'value' has no side effect.
@@ -1208,11 +1423,35 @@ TEST(PrintToStringTest, WorksForPointerToNonConstChar) {
   EXPECT_PRINT_TO_STRING_(p, "\"hello\"");
 }
 
+TEST(PrintToStringTest, EscapesForPointerToConstChar) {
+  const char* p = "hello\n";
+  EXPECT_PRINT_TO_STRING_(p, "\"hello\\n\"");
+}
+
+TEST(PrintToStringTest, EscapesForPointerToNonConstChar) {
+  char s[] = "hello\1";
+  char* p = s;
+  EXPECT_PRINT_TO_STRING_(p, "\"hello\\x1\"");
+}
+
 TEST(PrintToStringTest, WorksForArray) {
   int n[3] = { 1, 2, 3 };
   EXPECT_PRINT_TO_STRING_(n, "{ 1, 2, 3 }");
 }
 
+TEST(PrintToStringTest, WorksForCharArray) {
+  char s[] = "hello";
+  EXPECT_PRINT_TO_STRING_(s, "\"hello\"");
+}
+
+TEST(PrintToStringTest, WorksForCharArrayWithEmbeddedNul) {
+  const char str_with_nul[] = "hello\0 world";
+  EXPECT_PRINT_TO_STRING_(str_with_nul, "\"hello\\0 world\"");
+
+  char mutable_str_with_nul[] = "hello\0 world";
+  EXPECT_PRINT_TO_STRING_(mutable_str_with_nul, "\"hello\\0 world\"");
+}
+
 #undef EXPECT_PRINT_TO_STRING_
 
 TEST(UniversalTersePrintTest, WorksForNonReference) {
@@ -1275,6 +1514,17 @@ TEST(UniversalPrintTest, WorksForCString) {
   EXPECT_EQ("NULL", ss3.str());
 }
 
+TEST(UniversalPrintTest, WorksForCharArray) {
+  const char str[] = "\"Line\0 1\"\nLine 2";
+  ::std::stringstream ss1;
+  UniversalPrint(str, &ss1);
+  EXPECT_EQ("\"\\\"Line\\0 1\\\"\\nLine 2\"", ss1.str());
+
+  const char mutable_str[] = "\"Line\0 1\"\nLine 2";
+  ::std::stringstream ss2;
+  UniversalPrint(mutable_str, &ss2);
+  EXPECT_EQ("\"\\\"Line\\0 1\\\"\\nLine 2\"", ss2.str());
+}
 
 #if GTEST_HAS_TR1_TUPLE
 
diff --git a/test/gtest_output_test_.cc b/test/gtest_output_test_.cc
index 1b08b65b..da8b7449 100644
--- a/test/gtest_output_test_.cc
+++ b/test/gtest_output_test_.cc
@@ -27,8 +27,11 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 //
-// A unit test for Google Test itself.  This verifies that the basic
-// constructs of Google Test work.
+// The purpose of this file is to generate Google Test output under
+// various conditions.  The output will then be verified by
+// gtest_output_test.py to ensure that Google Test generates the
+// desired messages.  Therefore, most tests in this file are MEANT TO
+// FAIL.
 //
 // Author: wan@google.com (Zhanyong Wan)
 
@@ -101,6 +104,16 @@ INSTANTIATE_TEST_CASE_P(PrintingFailingParams,
                         FailingParamTest,
                         testing::Values(2));
 
+static const char kGoldenString[] = "\"Line\0 1\"\nLine 2";
+
+TEST(NonfatalFailureTest, EscapesStringOperands) {
+  std::string actual = "actual \"string\"";
+  EXPECT_EQ(kGoldenString, actual);
+
+  const char* golden = kGoldenString;
+  EXPECT_EQ(golden, actual);
+}
+
 // Tests catching a fatal failure in a subroutine.
 TEST(FatalFailureTest, FatalFailureInSubroutine) {
   printf("(expecting a failure that x should be 1)\n");
diff --git a/test/gtest_output_test_golden_lin.txt b/test/gtest_output_test_golden_lin.txt
index a1d342d9..0e26d63d 100644
--- a/test/gtest_output_test_golden_lin.txt
+++ b/test/gtest_output_test_golden_lin.txt
@@ -7,7 +7,7 @@ Expected: true
 gtest_output_test_.cc:#: Failure
 Value of: 3
 Expected: 2
-[==========] Running 62 tests from 27 test cases.
+[==========] Running 63 tests from 28 test cases.
 [----------] Global test environment set-up.
 FooEnvironment::SetUp() called.
 BarEnvironment::SetUp() called.
@@ -31,6 +31,19 @@ BarEnvironment::SetUp() called.
 [       OK ] PassingTest.PassingTest1
 [ RUN      ] PassingTest.PassingTest2
 [       OK ] PassingTest.PassingTest2
+[----------] 1 test from NonfatalFailureTest
+[ RUN      ] NonfatalFailureTest.EscapesStringOperands
+gtest_output_test_.cc:#: Failure
+Value of: actual
+  Actual: "actual \"string\""
+Expected: kGoldenString
+Which is: "\"Line"
+gtest_output_test_.cc:#: Failure
+Value of: actual
+  Actual: "actual \"string\""
+Expected: golden
+Which is: "\"Line"
+[  FAILED  ] NonfatalFailureTest.EscapesStringOperands
 [----------] 3 tests from FatalFailureTest
 [ RUN      ] FatalFailureTest.FatalFailureInSubroutine
 (expecting a failure that x should be 1)
@@ -586,9 +599,10 @@ FooEnvironment::TearDown() called.
 gtest_output_test_.cc:#: Failure
 Failed
 Expected fatal failure.
-[==========] 62 tests from 27 test cases ran.
+[==========] 63 tests from 28 test cases ran.
 [  PASSED  ] 21 tests.
-[  FAILED  ] 41 tests, listed below:
+[  FAILED  ] 42 tests, listed below:
+[  FAILED  ] NonfatalFailureTest.EscapesStringOperands
 [  FAILED  ] FatalFailureTest.FatalFailureInSubroutine
 [  FAILED  ] FatalFailureTest.FatalFailureInNestedSubroutine
 [  FAILED  ] FatalFailureTest.NonfatalFailureInSubroutine
@@ -631,7 +645,7 @@ Expected fatal failure.
 [  FAILED  ] ScopedFakeTestPartResultReporterTest.InterceptOnlyCurrentThread
 [  FAILED  ] PrintingFailingParams/FailingParamTest.Fails/0, where GetParam() = 2
 
-41 FAILED TESTS
+42 FAILED TESTS
   YOU HAVE 1 DISABLED TEST
 
 Note: Google Test filter = FatalFailureTest.*:LoggingTest.*
diff --git a/test/gtest_unittest.cc b/test/gtest_unittest.cc
index e61f7919..31a45656 100644
--- a/test/gtest_unittest.cc
+++ b/test/gtest_unittest.cc
@@ -1065,16 +1065,6 @@ TEST(StringTest, ConvertsToGlobalString) {
 
 #endif  // GTEST_HAS_GLOBAL_STRING
 
-// Tests String::ShowCStringQuoted().
-TEST(StringTest, ShowCStringQuoted) {
-  EXPECT_STREQ("(null)",
-               String::ShowCStringQuoted(NULL).c_str());
-  EXPECT_STREQ("\"\"",
-               String::ShowCStringQuoted("").c_str());
-  EXPECT_STREQ("\"foo\"",
-               String::ShowCStringQuoted("foo").c_str());
-}
-
 // Tests String::empty().
 TEST(StringTest, Empty) {
   EXPECT_TRUE(String("").empty());
@@ -1305,16 +1295,6 @@ TEST(StringTest, ShowWideCString) {
   EXPECT_STREQ("foo", String::ShowWideCString(L"foo").c_str());
 }
 
-// Tests String::ShowWideCStringQuoted().
-TEST(StringTest, ShowWideCStringQuoted) {
-  EXPECT_STREQ("(null)",
-               String::ShowWideCStringQuoted(NULL).c_str());
-  EXPECT_STREQ("L\"\"",
-               String::ShowWideCStringQuoted(L"").c_str());
-  EXPECT_STREQ("L\"foo\"",
-               String::ShowWideCStringQuoted(L"foo").c_str());
-}
-
 # if GTEST_OS_WINDOWS_MOBILE
 TEST(StringTest, AnsiAndUtf16Null) {
   EXPECT_EQ(NULL, String::AnsiToUtf16(NULL));