Put "value of," "object was," and any user messages on their own line…

…s, separate from the body. This inches us closer to the format we'll have after we shift to a field-style format and API. In particular, it will let us reimplement failWithRawMessage(...) as a call to failWithFields(fieldWithoutValue(...)) and migrate callers. It also breaks up long lines like those plenty of existing subjects produce. RELNOTES=Put any user messages on their own lines, separate from the main failure message. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=189630013
google · Mar 20, 2018 · 6d060b8 · 6d060b8
1 parent cd74a13
commit 6d060b8
Show file tree

Hide file tree

Showing 11 changed files with 85 additions and 92 deletions.
diff --git a/core/src/main/java/com/google/common/truth/FailureMetadata.java b/core/src/main/java/com/google/common/truth/FailureMetadata.java
@@ -209,25 +209,18 @@ private void doFail(AssertionError failure) {
   private String addToMessage(String body) {
     ImmutableList<?> messages = allPrefixMessages();
     StringBuilder result = new StringBuilder(body.length());
-    Joiner.on(": ").appendTo(result, messages);
+    Joiner.on("\n").appendTo(result, messages);
     if (!messages.isEmpty()) {
-      if (body.isEmpty()) {
-        /*
-         * The only likely case of an empty body is with failComparing(). In that case, we still
-         * want a colon because ComparisonFailure will construct a message of the form
-         * "<ourString> <theirString>." For consistency with the normal behavior of withMessage,
-         * we want a colon between the parts.
-         *
-         * That actually makes it sound like we'd want to *always* include the trailing colon in
-         * the case of failComparing(), but I don't want to bite that off now in case it requires
-         * updating more existing Subjects' tests.
-         */
-        result.append(":");
-      } else {
-        result.append(": ");
-      }
+      result.append("\n");
     }
     result.append(body);
+    /*
+     * JUnit's ComparisonFailure will append " expected: <...> but was: <...> to the end of this.
+     * Note the space at the beginning. That means that, if the body is empty, the "expected... but
+     * was" text will come at the beginning of a line... indented by one space :\ The right fix for
+     * this is to stop depending on JUnit's ComparisonFailure formatting. That's coming. For now,
+     * the space is an annoyance.
+     */
     return result.toString();
   }
 

diff --git a/core/src/main/java/com/google/common/truth/Truth.java b/core/src/main/java/com/google/common/truth/Truth.java
@@ -314,7 +314,7 @@ public String toString() {
   @Nullable
   static String appendSuffixIfNotNull(String message, String suffix) {
     if (suffix != null) {
-      message += ": " + suffix;
+      message += "\n" + suffix;
     }
     return message;
   }

diff --git a/core/src/test/java/com/google/common/truth/ChainingTest.java b/core/src/test/java/com/google/common/truth/ChainingTest.java
@@ -101,7 +101,7 @@ public void oneLevelNamed() {
     expectFailureWhenTestingThat("root")
         .delegatingToNamed("child", "child")
         .isThePresentKingOfFrance();
-    assertNoCause("value of: myObject.child: message: myObject was: root");
+    assertNoCause("value of: myObject.child\nmessage\nmyObject was: root");
   }
 
   @Test
@@ -110,7 +110,7 @@ public void twoLevelsNamed() {
         .delegatingToNamed("child", "child")
         .delegatingToNamed("grandchild", "grandchild")
         .isThePresentKingOfFrance();
-    assertNoCause("value of: myObject.child.grandchild: message: myObject was: root");
+    assertNoCause("value of: myObject.child.grandchild\nmessage\nmyObject was: root");
   }
 
   @Test
@@ -119,7 +119,7 @@ public void twoLevelsOnlyFirstNamed() {
         .delegatingToNamed("child", "child")
         .delegatingTo("grandchild")
         .isThePresentKingOfFrance();
-    assertNoCause("message: myObject was: root");
+    assertNoCause("message\nmyObject was: root");
   }
 
   @Test
@@ -128,15 +128,15 @@ public void twoLevelsOnlySecondNamed() {
         .delegatingTo("child")
         .delegatingToNamed("grandchild", "grandchild")
         .isThePresentKingOfFrance();
-    assertNoCause("value of: myObject.grandchild: message: myObject was: root");
+    assertNoCause("value of: myObject.grandchild\nmessage\nmyObject was: root");
   }
 
   @Test
   public void oneLevelNamedNoNeedToDisplayBoth() {
     expectFailureWhenTestingThat("root")
         .delegatingToNamedNoNeedToDisplayBoth("child", "child")
         .isThePresentKingOfFrance();
-    assertNoCause("value of: myObject.child: message");
+    assertNoCause("value of: myObject.child\nmessage");
   }
 
   @Test
@@ -145,7 +145,7 @@ public void twoLevelsNamedNoNeedToDisplayBoth() {
         .delegatingToNamedNoNeedToDisplayBoth("child", "child")
         .delegatingToNamedNoNeedToDisplayBoth("grandchild", "grandchild")
         .isThePresentKingOfFrance();
-    assertNoCause("value of: myObject.child.grandchild: message");
+    assertNoCause("value of: myObject.child.grandchild\nmessage");
   }
 
   @Test
@@ -163,7 +163,7 @@ public void twoLevelsOnlySecondNamedNoNeedToDisplayBoth() {
         .delegatingTo("child")
         .delegatingToNamedNoNeedToDisplayBoth("grandchild", "grandchild")
         .isThePresentKingOfFrance();
-    assertNoCause("value of: myObject.grandchild: message");
+    assertNoCause("value of: myObject.grandchild\nmessage");
   }
 
   @Test
@@ -172,7 +172,7 @@ public void twoLevelsNamedOnlyFirstNoNeedToDisplayBoth() {
         .delegatingToNamedNoNeedToDisplayBoth("child", "child")
         .delegatingToNamed("grandchild", "grandchild")
         .isThePresentKingOfFrance();
-    assertNoCause("value of: myObject.child.grandchild: message: myObject was: root");
+    assertNoCause("value of: myObject.child.grandchild\nmessage\nmyObject was: root");
   }
 
   @Test
@@ -181,14 +181,14 @@ public void twoLevelsNamedOnlySecondNoNeedToDisplayBoth() {
         .delegatingToNamed("child", "child")
         .delegatingToNamedNoNeedToDisplayBoth("grandchild", "grandchild")
         .isThePresentKingOfFrance();
-    assertNoCause("value of: myObject.child.grandchild: message: myObject was: root");
+    assertNoCause("value of: myObject.child.grandchild\nmessage\nmyObject was: root");
   }
 
   @Test
   public void namedAndComparisonFailure() {
     expectFailureWhenTestingThat("root").delegatingToNamed("child", "child").isEqualToString("z");
     assertNoCause(
-        "value of: myObject.child: message expected:<[child]> but was:<[z]>: myObject was: root");
+        "value of: myObject.child\nmessage expected:<[child]> but was:<[z]>\nmyObject was: root");
   }
 
   @Test
@@ -200,7 +200,7 @@ public void namedAndMessage() {
         .that("root")
         .delegatingToNamed("child", "child")
         .isThePresentKingOfFrance();
-    assertNoCause("prefix: value of: myObject.child: message: myObject was: root");
+    assertNoCause("prefix\nvalue of: myObject.child\nmessage\nmyObject was: root");
   }
 
   @Test
@@ -212,7 +212,7 @@ public void checkFail() {
   @Test
   public void checkFailWithName() {
     expectFailureWhenTestingThat("root").doCheckFail("child");
-    assertNoCause("value of: myObject.child: message: myObject was: root");
+    assertNoCause("value of: myObject.child\nmessage\nmyObject was: root");
   }
 
   @Test

diff --git a/core/src/test/java/com/google/common/truth/CustomFailureMessageTest.java b/core/src/test/java/com/google/common/truth/CustomFailureMessageTest.java
@@ -41,7 +41,7 @@ public void assertWithMessageIsPrepended() {
     } catch (AssertionError expected) {
       assertThat(expected)
           .hasMessageThat()
-          .isEqualTo("Invalid month: Not true that <13> is in <" + range + ">");
+          .isEqualTo("Invalid month\nNot true that <13> is in <" + range + ">");
       return;
     }
     fail("Should have thrown");
@@ -55,7 +55,7 @@ public void assertWithMessageIsPrependedWithNamed() {
     } catch (AssertionError expected) {
       assertThat(expected)
           .hasMessageThat()
-          .isEqualTo("Invalid month: Not true that Septober (<13>) is in <" + range + ">");
+          .isEqualTo("Invalid month\nNot true that Septober (<13>) is in <" + range + ">");
       return;
     }
     fail("Should have thrown");
@@ -69,7 +69,7 @@ public void assertWithMessageThat() {
       assertThat(expected)
           .hasMessageThat()
           .isEqualTo(
-              "This is a custom message: The subject was expected to be true, but was false");
+              "This is a custom message\nThe subject was expected to be true, but was false");
       return;
     }
     fail("Should have thrown");
@@ -83,7 +83,7 @@ public void customMessageIsPrepended() {
     } catch (AssertionError expected) {
       assertThat(expected)
           .hasMessageThat()
-          .isEqualTo("Invalid month: Not true that <13> is in <" + range + ">");
+          .isEqualTo("Invalid month\nNot true that <13> is in <" + range + ">");
       return;
     }
     fail("Should have thrown");
@@ -97,7 +97,7 @@ public void customMessageIsPrependedWithNamed() {
     } catch (AssertionError expected) {
       assertThat(expected)
           .hasMessageThat()
-          .isEqualTo("Invalid month: Not true that Septober (<13>) is in <" + range + ">");
+          .isEqualTo("Invalid month\nNot true that Septober (<13>) is in <" + range + ">");
       return;
     }
     fail("Should have thrown");
@@ -111,7 +111,7 @@ public void customMessage() {
       assertThat(expected)
           .hasMessageThat()
           .isEqualTo(
-              "This is a custom message: The subject was expected to be true, but was false");
+              "This is a custom message\nThe subject was expected to be true, but was false");
       return;
     }
     fail("Should have thrown");
@@ -142,7 +142,7 @@ public void assertWithMessageIsPrepended_withPlaceholders() {
     } catch (AssertionError expected) {
       assertThat(expected)
           .hasMessageThat()
-          .isEqualTo("Invalid month: Not true that <13> is in <" + range + ">");
+          .isEqualTo("Invalid month\nNot true that <13> is in <" + range + ">");
       return;
     }
     fail("Should have thrown");
@@ -156,7 +156,7 @@ public void assertWithMessageIsPrependedWithNamed_withPlaceholders() {
     } catch (AssertionError expected) {
       assertThat(expected)
           .hasMessageThat()
-          .isEqualTo("Invalid month: Not true that Septober (<13>) is in <" + range + ">");
+          .isEqualTo("Invalid month\nNot true that Septober (<13>) is in <" + range + ">");
       return;
     }
     fail("Should have thrown");
@@ -170,7 +170,7 @@ public void assertWithMessageThat_withPlaceholders() {
       assertThat(expected)
           .hasMessageThat()
           .isEqualTo(
-              "This is a custom message: The subject was expected to be true, but was false");
+              "This is a custom message\nThe subject was expected to be true, but was false");
       return;
     }
     fail("Should have thrown");
@@ -184,7 +184,7 @@ public void customMessageIsPrepended_withPlaceholders() {
     } catch (AssertionError expected) {
       assertThat(expected)
           .hasMessageThat()
-          .isEqualTo("Invalid month: Not true that <13> is in <" + range + ">");
+          .isEqualTo("Invalid month\nNot true that <13> is in <" + range + ">");
       return;
     }
     fail("Should have thrown");
@@ -198,7 +198,7 @@ public void customMessageIsPrependedWithNamed_withPlaceholders() {
     } catch (AssertionError expected) {
       assertThat(expected)
           .hasMessageThat()
-          .isEqualTo("Invalid month: Not true that Septober (<13>) is in <" + range + ">");
+          .isEqualTo("Invalid month\nNot true that Septober (<13>) is in <" + range + ">");
       return;
     }
     fail("Should have thrown");
@@ -212,7 +212,7 @@ public void customMessage_withPlaceholders() {
       assertThat(expected)
           .hasMessageThat()
           .isEqualTo(
-              "This is a custom message: The subject was expected to be true, but was false");
+              "This is a custom message\nThe subject was expected to be true, but was false");
       return;
     }
     fail("Should have thrown");

diff --git a/core/src/test/java/com/google/common/truth/IterableSubjectTest.java b/core/src/test/java/com/google/common/truth/IterableSubjectTest.java
@@ -117,7 +117,7 @@ public void failureMessageIterableContainsFailure() {
     expectFailure.whenTesting().withMessage("custom msg").that(asList(1, 2, 3)).contains(5);
     assertThat(expectFailure.getFailure())
         .hasMessageThat()
-        .isEqualTo("custom msg: <[1, 2, 3]> should have contained <5>");
+        .isEqualTo("custom msg\n<[1, 2, 3]> should have contained <5>");
   }
 
   @Test

diff --git a/core/src/test/java/com/google/common/truth/MultimapSubjectTest.java b/core/src/test/java/com/google/common/truth/MultimapSubjectTest.java
@@ -176,9 +176,9 @@ public void multimapNamedValuesForKey() {
     assertThat(expectFailure.getFailure())
         .hasMessageThat()
         .isEqualTo(
-            "value of: multymap.valuesForKey(1): "
+            "value of: multymap.valuesForKey(1)\n"
                 + "Not true that <[5]> contains exactly <[4]>. "
-                + "It is missing <[4]> and has unexpected items <[5]>: "
+                + "It is missing <[4]> and has unexpected items <[5]>\n"
                 + "multimap was: multymap ({1=[5]})");
   }
 
@@ -189,9 +189,9 @@ public void valuesForKeyNamed() {
     assertThat(expectFailure.getFailure())
         .hasMessageThat()
         .isEqualTo(
-            "value of: multimap.valuesForKey(1): "
+            "value of: multimap.valuesForKey(1)\n"
                 + "Not true that valuez (<[5]>) contains exactly <[4]>. "
-                + "It is missing <[4]> and has unexpected items <[5]>: "
+                + "It is missing <[4]> and has unexpected items <[5]>\n"
                 + "multimap was: {1=[5]}");
   }