Skip to content

Commit 684afc1

Browse files
Googlercopybara-github
authored andcommitted
Adjust newlines between the description and diff to improve matcher output consistency.
There are two slight inconsistencies that make output a little less consistent than ideal: * When constructing the summarize_diff message, some branches return the empty string, another a string with a leading newline, and yet another a string without a leading newline. * The explanation+diff formatting behavior between str_matcher and eq_matcher was not consistent: str_matcher added a newline between them, and eq_matcher did not, so depending on diff output, messages could be glued without a separator, making it harder to read. We adjust the formatting code to try to improve consistency. We drop the leading newline from summarize_diff, and make it the responsibility of callers to conditionally add or omit newlines to separate (possibly empty) diff output from the leading description. As a result, `eq_matcher` output aligns more closely with `str_matcher`. PiperOrigin-RevId: 696923930
1 parent 43a95ea commit 684afc1

File tree

4 files changed

+20
-8
lines changed

4 files changed

+20
-8
lines changed

googletest/src/matcher_support/summarize_diff.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ pub(crate) fn create_diff(
4141
match edit_distance::edit_list(actual_debug.lines(), expected_debug.lines(), diff_mode) {
4242
edit_distance::Difference::Equal => "No difference found between debug strings.".into(),
4343
edit_distance::Difference::Editable(edit_list) => {
44-
format!("\n{}{}", summary_header(), edit_list.into_iter().collect::<BufferedSummary>(),)
44+
format!("{}{}", summary_header(), edit_list.into_iter().collect::<BufferedSummary>(),)
4545
.into()
4646
}
4747
edit_distance::Difference::Unrelated => "".into(),
@@ -73,7 +73,7 @@ pub(crate) fn create_diff_reversed(
7373
edit_distance::Difference::Equal => "No difference found between debug strings.".into(),
7474
edit_distance::Difference::Editable(mut edit_list) => {
7575
edit_list.reverse();
76-
format!("\n{}{}", summary_header(), edit_list.into_iter().collect::<BufferedSummary>(),)
76+
format!("{}{}", summary_header(), edit_list.into_iter().collect::<BufferedSummary>(),)
7777
.into()
7878
}
7979
edit_distance::Difference::Unrelated => "".into(),
@@ -470,7 +470,6 @@ mod tests {
470470
create_diff(expected, actual, Mode::Exact),
471471
eq(indoc!(
472472
"
473-
474473
Difference(-actual / +expected):
475474
prefix
476475
-Actual#1
@@ -494,7 +493,6 @@ mod tests {
494493
create_diff(&build_text(1..50), &build_text(1..51), Mode::Exact),
495494
eq(indoc! {
496495
"
497-
498496
Difference(-actual / +expected):
499497
1
500498
2
@@ -514,7 +512,6 @@ mod tests {
514512
create_diff(&build_text(1..50), &build_text(1..51), Mode::Exact),
515513
eq(indoc! {
516514
"
517-
518515
Difference(-\x1B[1;31mactual\x1B[0m / +\x1B[1;32mexpected\x1B[0m):
519516
1
520517
2
@@ -546,7 +543,6 @@ mod tests {
546543
create_diff(actual, expected, Mode::Exact),
547544
eq(indoc! {
548545
"
549-
550546
Difference(-\x1B[1;31mactual\x1B[0m / +\x1B[1;32mexpected\x1B[0m):
551547
-\x1B[31mThere is a ho\x1B[0m\x1B[1;31mm\x1B[0m\x1B[31me in N\x1B[0m\x1B[1;31mouv\x1B[0m\x1B[31me\x1B[0m\x1B[1;31mlle\x1B[0m\x1B[31m Orleans\x1B[0m
552548
+\x1B[32mThere is a ho\x1B[0m\x1B[1;32mus\x1B[0m\x1B[32me \x1B[0m\x1B[1;32mway down \x1B[0m\x1B[32min Ne\x1B[0m\x1B[1;32mw\x1B[0m\x1B[32m Orleans\x1B[0m

googletest/src/matchers/display_matcher.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ mod tests {
111111
"
112112
Actual: \"123\\n234\",
113113
which displays as \"123\\n234\" which isn't equal to \"123\\n345\"
114+
114115
Difference(-actual / +expected):
115116
123
116117
-234

googletest/src/matchers/eq_matcher.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,11 @@ impl<T: Debug, A: Debug + Copy + PartialEq<T>> Matcher<A> for EqMatcher<T> {
116116
create_diff(&actual_debug, &expected_debug, edit_distance::Mode::Exact)
117117
};
118118

119-
format!("which {description}{diff}").into()
119+
if diff.is_empty() {
120+
format!("which {description}").into()
121+
} else {
122+
format!("which {description}\n\n{diff}").into()
123+
}
120124
}
121125
}
122126

@@ -176,6 +180,7 @@ mod tests {
176180
"
177181
Actual: Strukt { int: 123, string: \"something\" },
178182
which isn't equal to Strukt { int: 321, string: \"someone\" }
183+
179184
Difference(-actual / +expected):
180185
Strukt {
181186
- int: 123,
@@ -198,6 +203,7 @@ mod tests {
198203
Expected: is equal to [1, 3, 4]
199204
Actual: [1, 2, 3],
200205
which isn't equal to [1, 3, 4]
206+
201207
Difference(-actual / +expected):
202208
[
203209
1,
@@ -220,6 +226,7 @@ mod tests {
220226
Expected: is equal to [1, 3, 5]
221227
Actual: [1, 2, 3, 4, 5],
222228
which isn't equal to [1, 3, 5]
229+
223230
Difference(-actual / +expected):
224231
[
225232
1,
@@ -241,6 +248,7 @@ mod tests {
241248
"
242249
],
243250
which isn't equal to [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51]
251+
244252
Difference(-actual / +expected):
245253
[
246254
- 1,
@@ -265,6 +273,7 @@ mod tests {
265273
"
266274
Actual: [1, 2, 3, 4, 5, 6, 7],
267275
which isn't equal to [3, 4, 5, 6, 7, 8, 9]
276+
268277
Difference(-actual / +expected):
269278
[
270279
- 1,
@@ -289,6 +298,7 @@ mod tests {
289298
"
290299
],
291300
which isn't equal to [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51]
301+
292302
Difference(-actual / +expected):
293303
[
294304
1,
@@ -310,6 +320,7 @@ mod tests {
310320
"
311321
],
312322
which isn't equal to [3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51]
323+
313324
Difference(-actual / +expected):
314325
[
315326
- 1,

googletest/src/matchers/str_matcher.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,11 @@ impl Configuration {
533533
MatchMode::EndsWith => create_diff_reversed(actual, expected, self.mode.to_diff_mode()),
534534
};
535535

536-
format!("{default_explanation}\n{diff}").into()
536+
if diff.is_empty() {
537+
format!("{default_explanation}").into()
538+
} else {
539+
format!("{default_explanation}\n\n{diff}").into()
540+
}
537541
}
538542

539543
fn ignoring_leading_whitespace(self) -> Self {

0 commit comments

Comments
 (0)