Skip to content

Commit 6d060b8

Browse files
cpovirkronshapiro
cpovirk
authored and
ronshapiro
committed
Put "value of," "object was," and any user messages on their own lines, 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
1 parent cd74a13 commit 6d060b8

11 files changed

+85
-92
lines changed

‎core/src/main/java/com/google/common/truth/FailureMetadata.java‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -209,25 +209,18 @@ private void doFail(AssertionError failure) {
209209
private String addToMessage(String body) {
210210
ImmutableList<?> messages = allPrefixMessages();
211211
StringBuilder result = new StringBuilder(body.length());
212-
Joiner.on(": ").appendTo(result, messages);
212+
Joiner.on("\n").appendTo(result, messages);
213213
if (!messages.isEmpty()) {
214-
if (body.isEmpty()) {
215-
/*
216-
* The only likely case of an empty body is with failComparing(). In that case, we still
217-
* want a colon because ComparisonFailure will construct a message of the form
218-
* "<ourString> <theirString>." For consistency with the normal behavior of withMessage,
219-
* we want a colon between the parts.
220-
*
221-
* That actually makes it sound like we'd want to *always* include the trailing colon in
222-
* the case of failComparing(), but I don't want to bite that off now in case it requires
223-
* updating more existing Subjects' tests.
224-
*/
225-
result.append(":");
226-
} else {
227-
result.append(": ");
228-
}
214+
result.append("\n");
229215
}
230216
result.append(body);
217+
/*
218+
* JUnit's ComparisonFailure will append " expected: <...> but was: <...> to the end of this.
219+
* Note the space at the beginning. That means that, if the body is empty, the "expected... but
220+
* was" text will come at the beginning of a line... indented by one space :\ The right fix for
221+
* this is to stop depending on JUnit's ComparisonFailure formatting. That's coming. For now,
222+
* the space is an annoyance.
223+
*/
231224
return result.toString();
232225
}
233226

‎core/src/main/java/com/google/common/truth/Truth.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ public String toString() {
314314
@Nullable
315315
static String appendSuffixIfNotNull(String message, String suffix) {
316316
if (suffix != null) {
317-
message += ": " + suffix;
317+
message += "\n" + suffix;
318318
}
319319
return message;
320320
}

‎core/src/test/java/com/google/common/truth/ChainingTest.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public void oneLevelNamed() {
101101
expectFailureWhenTestingThat("root")
102102
.delegatingToNamed("child", "child")
103103
.isThePresentKingOfFrance();
104-
assertNoCause("value of: myObject.child: message: myObject was: root");
104+
assertNoCause("value of: myObject.child\nmessage\nmyObject was: root");
105105
}
106106

107107
@Test
@@ -110,7 +110,7 @@ public void twoLevelsNamed() {
110110
.delegatingToNamed("child", "child")
111111
.delegatingToNamed("grandchild", "grandchild")
112112
.isThePresentKingOfFrance();
113-
assertNoCause("value of: myObject.child.grandchild: message: myObject was: root");
113+
assertNoCause("value of: myObject.child.grandchild\nmessage\nmyObject was: root");
114114
}
115115

116116
@Test
@@ -119,7 +119,7 @@ public void twoLevelsOnlyFirstNamed() {
119119
.delegatingToNamed("child", "child")
120120
.delegatingTo("grandchild")
121121
.isThePresentKingOfFrance();
122-
assertNoCause("message: myObject was: root");
122+
assertNoCause("message\nmyObject was: root");
123123
}
124124

125125
@Test
@@ -128,15 +128,15 @@ public void twoLevelsOnlySecondNamed() {
128128
.delegatingTo("child")
129129
.delegatingToNamed("grandchild", "grandchild")
130130
.isThePresentKingOfFrance();
131-
assertNoCause("value of: myObject.grandchild: message: myObject was: root");
131+
assertNoCause("value of: myObject.grandchild\nmessage\nmyObject was: root");
132132
}
133133

134134
@Test
135135
public void oneLevelNamedNoNeedToDisplayBoth() {
136136
expectFailureWhenTestingThat("root")
137137
.delegatingToNamedNoNeedToDisplayBoth("child", "child")
138138
.isThePresentKingOfFrance();
139-
assertNoCause("value of: myObject.child: message");
139+
assertNoCause("value of: myObject.child\nmessage");
140140
}
141141

142142
@Test
@@ -145,7 +145,7 @@ public void twoLevelsNamedNoNeedToDisplayBoth() {
145145
.delegatingToNamedNoNeedToDisplayBoth("child", "child")
146146
.delegatingToNamedNoNeedToDisplayBoth("grandchild", "grandchild")
147147
.isThePresentKingOfFrance();
148-
assertNoCause("value of: myObject.child.grandchild: message");
148+
assertNoCause("value of: myObject.child.grandchild\nmessage");
149149
}
150150

151151
@Test
@@ -163,7 +163,7 @@ public void twoLevelsOnlySecondNamedNoNeedToDisplayBoth() {
163163
.delegatingTo("child")
164164
.delegatingToNamedNoNeedToDisplayBoth("grandchild", "grandchild")
165165
.isThePresentKingOfFrance();
166-
assertNoCause("value of: myObject.grandchild: message");
166+
assertNoCause("value of: myObject.grandchild\nmessage");
167167
}
168168

169169
@Test
@@ -172,7 +172,7 @@ public void twoLevelsNamedOnlyFirstNoNeedToDisplayBoth() {
172172
.delegatingToNamedNoNeedToDisplayBoth("child", "child")
173173
.delegatingToNamed("grandchild", "grandchild")
174174
.isThePresentKingOfFrance();
175-
assertNoCause("value of: myObject.child.grandchild: message: myObject was: root");
175+
assertNoCause("value of: myObject.child.grandchild\nmessage\nmyObject was: root");
176176
}
177177

178178
@Test
@@ -181,14 +181,14 @@ public void twoLevelsNamedOnlySecondNoNeedToDisplayBoth() {
181181
.delegatingToNamed("child", "child")
182182
.delegatingToNamedNoNeedToDisplayBoth("grandchild", "grandchild")
183183
.isThePresentKingOfFrance();
184-
assertNoCause("value of: myObject.child.grandchild: message: myObject was: root");
184+
assertNoCause("value of: myObject.child.grandchild\nmessage\nmyObject was: root");
185185
}
186186

187187
@Test
188188
public void namedAndComparisonFailure() {
189189
expectFailureWhenTestingThat("root").delegatingToNamed("child", "child").isEqualToString("z");
190190
assertNoCause(
191-
"value of: myObject.child: message expected:<[child]> but was:<[z]>: myObject was: root");
191+
"value of: myObject.child\nmessage expected:<[child]> but was:<[z]>\nmyObject was: root");
192192
}
193193

194194
@Test
@@ -200,7 +200,7 @@ public void namedAndMessage() {
200200
.that("root")
201201
.delegatingToNamed("child", "child")
202202
.isThePresentKingOfFrance();
203-
assertNoCause("prefix: value of: myObject.child: message: myObject was: root");
203+
assertNoCause("prefix\nvalue of: myObject.child\nmessage\nmyObject was: root");
204204
}
205205

206206
@Test
@@ -212,7 +212,7 @@ public void checkFail() {
212212
@Test
213213
public void checkFailWithName() {
214214
expectFailureWhenTestingThat("root").doCheckFail("child");
215-
assertNoCause("value of: myObject.child: message: myObject was: root");
215+
assertNoCause("value of: myObject.child\nmessage\nmyObject was: root");
216216
}
217217

218218
@Test

‎core/src/test/java/com/google/common/truth/CustomFailureMessageTest.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public void assertWithMessageIsPrepended() {
4141
} catch (AssertionError expected) {
4242
assertThat(expected)
4343
.hasMessageThat()
44-
.isEqualTo("Invalid month: Not true that <13> is in <" + range + ">");
44+
.isEqualTo("Invalid month\nNot true that <13> is in <" + range + ">");
4545
return;
4646
}
4747
fail("Should have thrown");
@@ -55,7 +55,7 @@ public void assertWithMessageIsPrependedWithNamed() {
5555
} catch (AssertionError expected) {
5656
assertThat(expected)
5757
.hasMessageThat()
58-
.isEqualTo("Invalid month: Not true that Septober (<13>) is in <" + range + ">");
58+
.isEqualTo("Invalid month\nNot true that Septober (<13>) is in <" + range + ">");
5959
return;
6060
}
6161
fail("Should have thrown");
@@ -69,7 +69,7 @@ public void assertWithMessageThat() {
6969
assertThat(expected)
7070
.hasMessageThat()
7171
.isEqualTo(
72-
"This is a custom message: The subject was expected to be true, but was false");
72+
"This is a custom message\nThe subject was expected to be true, but was false");
7373
return;
7474
}
7575
fail("Should have thrown");
@@ -83,7 +83,7 @@ public void customMessageIsPrepended() {
8383
} catch (AssertionError expected) {
8484
assertThat(expected)
8585
.hasMessageThat()
86-
.isEqualTo("Invalid month: Not true that <13> is in <" + range + ">");
86+
.isEqualTo("Invalid month\nNot true that <13> is in <" + range + ">");
8787
return;
8888
}
8989
fail("Should have thrown");
@@ -97,7 +97,7 @@ public void customMessageIsPrependedWithNamed() {
9797
} catch (AssertionError expected) {
9898
assertThat(expected)
9999
.hasMessageThat()
100-
.isEqualTo("Invalid month: Not true that Septober (<13>) is in <" + range + ">");
100+
.isEqualTo("Invalid month\nNot true that Septober (<13>) is in <" + range + ">");
101101
return;
102102
}
103103
fail("Should have thrown");
@@ -111,7 +111,7 @@ public void customMessage() {
111111
assertThat(expected)
112112
.hasMessageThat()
113113
.isEqualTo(
114-
"This is a custom message: The subject was expected to be true, but was false");
114+
"This is a custom message\nThe subject was expected to be true, but was false");
115115
return;
116116
}
117117
fail("Should have thrown");
@@ -142,7 +142,7 @@ public void assertWithMessageIsPrepended_withPlaceholders() {
142142
} catch (AssertionError expected) {
143143
assertThat(expected)
144144
.hasMessageThat()
145-
.isEqualTo("Invalid month: Not true that <13> is in <" + range + ">");
145+
.isEqualTo("Invalid month\nNot true that <13> is in <" + range + ">");
146146
return;
147147
}
148148
fail("Should have thrown");
@@ -156,7 +156,7 @@ public void assertWithMessageIsPrependedWithNamed_withPlaceholders() {
156156
} catch (AssertionError expected) {
157157
assertThat(expected)
158158
.hasMessageThat()
159-
.isEqualTo("Invalid month: Not true that Septober (<13>) is in <" + range + ">");
159+
.isEqualTo("Invalid month\nNot true that Septober (<13>) is in <" + range + ">");
160160
return;
161161
}
162162
fail("Should have thrown");
@@ -170,7 +170,7 @@ public void assertWithMessageThat_withPlaceholders() {
170170
assertThat(expected)
171171
.hasMessageThat()
172172
.isEqualTo(
173-
"This is a custom message: The subject was expected to be true, but was false");
173+
"This is a custom message\nThe subject was expected to be true, but was false");
174174
return;
175175
}
176176
fail("Should have thrown");
@@ -184,7 +184,7 @@ public void customMessageIsPrepended_withPlaceholders() {
184184
} catch (AssertionError expected) {
185185
assertThat(expected)
186186
.hasMessageThat()
187-
.isEqualTo("Invalid month: Not true that <13> is in <" + range + ">");
187+
.isEqualTo("Invalid month\nNot true that <13> is in <" + range + ">");
188188
return;
189189
}
190190
fail("Should have thrown");
@@ -198,7 +198,7 @@ public void customMessageIsPrependedWithNamed_withPlaceholders() {
198198
} catch (AssertionError expected) {
199199
assertThat(expected)
200200
.hasMessageThat()
201-
.isEqualTo("Invalid month: Not true that Septober (<13>) is in <" + range + ">");
201+
.isEqualTo("Invalid month\nNot true that Septober (<13>) is in <" + range + ">");
202202
return;
203203
}
204204
fail("Should have thrown");
@@ -212,7 +212,7 @@ public void customMessage_withPlaceholders() {
212212
assertThat(expected)
213213
.hasMessageThat()
214214
.isEqualTo(
215-
"This is a custom message: The subject was expected to be true, but was false");
215+
"This is a custom message\nThe subject was expected to be true, but was false");
216216
return;
217217
}
218218
fail("Should have thrown");

‎core/src/test/java/com/google/common/truth/IterableSubjectTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public void failureMessageIterableContainsFailure() {
117117
expectFailure.whenTesting().withMessage("custom msg").that(asList(1, 2, 3)).contains(5);
118118
assertThat(expectFailure.getFailure())
119119
.hasMessageThat()
120-
.isEqualTo("custom msg: <[1, 2, 3]> should have contained <5>");
120+
.isEqualTo("custom msg\n<[1, 2, 3]> should have contained <5>");
121121
}
122122

123123
@Test

‎core/src/test/java/com/google/common/truth/MultimapSubjectTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,9 @@ public void multimapNamedValuesForKey() {
176176
assertThat(expectFailure.getFailure())
177177
.hasMessageThat()
178178
.isEqualTo(
179-
"value of: multymap.valuesForKey(1): "
179+
"value of: multymap.valuesForKey(1)\n"
180180
+ "Not true that <[5]> contains exactly <[4]>. "
181-
+ "It is missing <[4]> and has unexpected items <[5]>: "
181+
+ "It is missing <[4]> and has unexpected items <[5]>\n"
182182
+ "multimap was: multymap ({1=[5]})");
183183
}
184184

@@ -189,9 +189,9 @@ public void valuesForKeyNamed() {
189189
assertThat(expectFailure.getFailure())
190190
.hasMessageThat()
191191
.isEqualTo(
192-
"value of: multimap.valuesForKey(1): "
192+
"value of: multimap.valuesForKey(1)\n"
193193
+ "Not true that valuez (<[5]>) contains exactly <[4]>. "
194-
+ "It is missing <[4]> and has unexpected items <[5]>: "
194+
+ "It is missing <[4]> and has unexpected items <[5]>\n"
195195
+ "multimap was: {1=[5]}");
196196
}
197197

0 commit comments

Comments
 (0)