Skip to content

Commit 44c1e2d

Browse files
hovinenbcopybara-github
authored andcommitted
Improve support for displaying the diff between actual and expected values in the contains_substring matcher.
In contrast with `starts_with` and `ends_with`, there is no anchor to identify where to start matching the substring with the actual value. Any method to find such an anchor in general would require using a maximum edit distance equal to the sum of the lengths of the inputs. The resulting algorithm would have quadratic running time in the input length, which is too expensive. For this reason, this implementation makes a "best effort" attempt using the existing maximum edit length and exact matching. The `edit_list` implementation has a new mode `Contains` which compresses the prefix and suffix in the actual value to `Edit::AdditionalLeft`. If the total number of lines in the actual value not in the substring already exceed the maximum edit distance, then the diff output will not be shown. The practical effect of this change is that the lines of the actual value preceding and succeeding the substring are now compressed to the string `<---- remaining lines omitted ---->`. PiperOrigin-RevId: 540904195
1 parent d15f5ad commit 44c1e2d

File tree

2 files changed

+121
-3
lines changed

2 files changed

+121
-3
lines changed

googletest/src/matcher_support/edit_distance.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ pub(crate) enum Edit<T> {
6161
}
6262

6363
/// Controls the termination condition of [`edit_list`].
64+
#[derive(Clone, Copy)]
6465
pub(crate) enum Mode {
6566
/// Indicates that the two arguments are intended to be equal.
6667
///
@@ -73,6 +74,15 @@ pub(crate) enum Mode {
7374
/// Any additional parts of `left` after the prefix `right` are omitted from
7475
/// the output.
7576
Prefix,
77+
78+
/// Similar to [`Mode::Prefix`], except it is also assumed that `left` has
79+
/// some number of initial lines which should not be in the output.
80+
///
81+
/// Any initial [`Edit::ExtraLeft`] entries are replaced with
82+
/// [`Edit::AdditionalLeft`] in the edit list. If the first entry which is
83+
/// not an [`Edit::Extraleft`] is [`Edit::ExtraRight`], then the last
84+
/// [`Edit::ExtraLeft`] is left in the output.
85+
Contains,
7686
}
7787

7888
/// Computes the edit list of `left` and `right`.
@@ -163,6 +173,9 @@ pub(crate) fn edit_list<T: PartialEq + Copy>(
163173
// If we have exhausted both inputs, we are done.
164174
if left_endpoint == left.len() && right_endpoint == right.len() {
165175
return if path.edits.iter().any(|v| !matches!(v, Edit::Both(_))) {
176+
if matches!(mode, Mode::Contains) {
177+
compress_prefix_and_suffix(&mut path.edits);
178+
}
166179
Difference::Editable(path.edits)
167180
} else {
168181
Difference::Equal
@@ -213,6 +226,30 @@ fn index_of_k(k: i32, k_min: i32) -> usize {
213226
((k - k_min) / 2) as usize
214227
}
215228

229+
fn compress_prefix_and_suffix<T>(edits: &mut Vec<Edit<T>>) {
230+
if let Some(mut first_non_extra_left_edit) =
231+
edits.iter().position(|e| !matches!(e, Edit::ExtraLeft(_)))
232+
{
233+
if first_non_extra_left_edit > 1
234+
&& matches!(edits[first_non_extra_left_edit], Edit::ExtraRight(_))
235+
{
236+
first_non_extra_left_edit -= 1;
237+
}
238+
edits.splice(..first_non_extra_left_edit, [Edit::AdditionalLeft]);
239+
}
240+
241+
if let Some(mut last_non_extra_left_edit) =
242+
edits.iter().rposition(|e| !matches!(e, Edit::ExtraLeft(_)))
243+
{
244+
if last_non_extra_left_edit < edits.len() - 1
245+
&& matches!(edits[last_non_extra_left_edit], Edit::ExtraRight(_))
246+
{
247+
last_non_extra_left_edit += 1;
248+
}
249+
edits.splice(last_non_extra_left_edit + 1.., [Edit::AdditionalLeft]);
250+
}
251+
}
252+
216253
#[derive(Clone)]
217254
struct Path<T: Clone> {
218255
left_endpoint: usize,

googletest/src/matchers/str_matcher.rs

Lines changed: 84 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -399,11 +399,10 @@ enum MatchMode {
399399

400400
impl MatchMode {
401401
fn to_diff_mode(&self) -> edit_distance::Mode {
402-
// TODO(b/286515736): Once supported, only MatchMode::Equals should map to
403-
// Mode::Exact.
404402
match self {
405403
MatchMode::StartsWith | MatchMode::EndsWith => edit_distance::Mode::Prefix,
406-
_ => edit_distance::Mode::Exact,
404+
MatchMode::Contains => edit_distance::Mode::Contains,
405+
MatchMode::Equals => edit_distance::Mode::Exact,
407406
}
408407
}
409408
}
@@ -542,6 +541,9 @@ impl Configuration {
542541

543542
let diff = match self.mode {
544543
MatchMode::Equals | MatchMode::StartsWith | MatchMode::Contains => {
544+
// TODO(b/287632452): Also consider improving the output in MatchMode::Contains
545+
// when the substring begins or ends in the middle of a line of the actual
546+
// value.
545547
create_diff(expected, actual, self.mode.to_diff_mode())
546548
}
547549
MatchMode::EndsWith => create_diff_reversed(expected, actual, self.mode.to_diff_mode()),
@@ -1082,6 +1084,85 @@ mod tests {
10821084
)
10831085
}
10841086

1087+
#[test]
1088+
fn match_explanation_for_contains_substring_ignores_outer_lines_in_actual_string() -> Result<()>
1089+
{
1090+
let result = verify_that!(
1091+
indoc!(
1092+
"
1093+
First line
1094+
Second line
1095+
Third line
1096+
Fourth line
1097+
Fifth line
1098+
"
1099+
),
1100+
contains_substring(indoc!(
1101+
"
1102+
Second line
1103+
Third lines
1104+
Fourth line
1105+
"
1106+
))
1107+
);
1108+
1109+
verify_that!(
1110+
result,
1111+
err(displays_as(contains_substring(indoc!(
1112+
"
1113+
Difference:
1114+
<---- remaining lines omitted ---->
1115+
Second line
1116+
-Third lines
1117+
+Third line
1118+
Fourth line
1119+
<---- remaining lines omitted ---->
1120+
"
1121+
))))
1122+
)
1123+
}
1124+
1125+
#[test]
1126+
fn match_explanation_for_contains_substring_shows_diff_when_first_and_last_line_are_incomplete()
1127+
-> Result<()> {
1128+
let result = verify_that!(
1129+
indoc!(
1130+
"
1131+
First line
1132+
Second line
1133+
Third line
1134+
Fourth line
1135+
Fifth line
1136+
"
1137+
),
1138+
contains_substring(indoc!(
1139+
"
1140+
line
1141+
Third line
1142+
Foorth line
1143+
Fifth"
1144+
))
1145+
);
1146+
1147+
verify_that!(
1148+
result,
1149+
err(displays_as(contains_substring(indoc!(
1150+
"
1151+
Difference:
1152+
<---- remaining lines omitted ---->
1153+
-line
1154+
+Second line
1155+
Third line
1156+
-Foorth line
1157+
+Fourth line
1158+
-Fifth
1159+
+Fifth line
1160+
<---- remaining lines omitted ---->
1161+
"
1162+
))))
1163+
)
1164+
}
1165+
10851166
#[test]
10861167
fn match_explanation_for_eq_does_not_ignore_trailing_lines_in_actual_string() -> Result<()> {
10871168
let result = verify_that!(

0 commit comments

Comments
 (0)