Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Diffing tables doesn't work when delta span multiple lines #465

Closed
wants to merge 1 commit into from
Closed

Diffing tables doesn't work when delta span multiple lines #465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 25 additions & 13 deletions core/src/main/java/cucumber/runtime/table/TableDiffer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ public void calculateDiffs() throws TableDiffException {
}
}

private Map<Integer, Delta> createDeltasByLine(List<Delta> deltas) {
Map<Integer, Delta> deltasByLine = new HashMap<Integer, Delta>();
for (Delta delta : deltas) {
deltasByLine.put(delta.getOriginal().getPosition(), delta);
}
return deltasByLine;
}

private DataTable createTableDiff(Map<Integer, Delta> deltasByLine) {
List<DataTableRow> diffTableRows = new ArrayList<DataTableRow>();
List<List<String>> rows = orig.raw();
Expand All @@ -47,6 +55,12 @@ private DataTable createTableDiff(Map<Integer, Delta> deltasByLine) {
diffTableRows.add(orig.getGherkinRows().get(i));
} else {
addRowsToTableDiff(diffTableRows, delta);
// skipping lines involved in a delta
if (delta.getType() == Delta.TYPE.CHANGE || delta.getType() == Delta.TYPE.DELETE) {
i += delta.getOriginal().getLines().size() - 1;
} else {
diffTableRows.add(orig.getGherkinRows().get(i));
}
}
}
// Can have new lines at end
Expand All @@ -58,23 +72,21 @@ private DataTable createTableDiff(Map<Integer, Delta> deltasByLine) {
}

private void addRowsToTableDiff(List<DataTableRow> diffTableRows, Delta delta) {
if (delta.getType() == Delta.TYPE.CHANGE || delta.getType() == Delta.TYPE.DELETE) {
List<DiffableRow> deletedLines = (List<DiffableRow>) delta.getOriginal().getLines();
for (DiffableRow row : deletedLines) {
diffTableRows.add(new DataTableRow(row.row.getComments(), row.row.getCells(), row.row.getLine(), Row.DiffType.DELETE));
}
markChangedAndDeletedRowsInOriginalAsMissing(diffTableRows, delta);
markChangedAndInsertedRowsInRevisedAsNew(diffTableRows, delta);
}

private void markChangedAndDeletedRowsInOriginalAsMissing(List<DataTableRow> diffTableRows, Delta delta) {
List<DiffableRow> deletedLines = (List<DiffableRow>) delta.getOriginal().getLines();
for (DiffableRow row : deletedLines) {
diffTableRows.add(new DataTableRow(row.row.getComments(), row.row.getCells(), row.row.getLine(), Row.DiffType.DELETE));
}
}

private void markChangedAndInsertedRowsInRevisedAsNew(List<DataTableRow> diffTableRows, Delta delta) {
List<DiffableRow> insertedLines = (List<DiffableRow>) delta.getRevised().getLines();
for (DiffableRow row : insertedLines) {
diffTableRows.add(new DataTableRow(row.row.getComments(), row.row.getCells(), row.row.getLine(), Row.DiffType.INSERT));
}
}

private Map<Integer, Delta> createDeltasByLine(List<Delta> deltas) {
Map<Integer, Delta> deltasByLine = new HashMap<Integer, Delta>();
for (Delta delta : deltas) {
deltasByLine.put(delta.getOriginal().getPosition(), delta);
}
return deltasByLine;
}
}
89 changes: 83 additions & 6 deletions core/src/test/java/cucumber/runtime/table/TableDifferTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,34 @@ private DataTable table() {
return TableParser.parse(source, null);
}

private DataTable otherTableWithTwoConsecutiveRowsDeleted() {
String source = "" +
"| Aslak | aslak@email.com | 123 |\n" +
"| Ni | ni@email.com | 654 |\n";
return TableParser.parse(source, null);

}

private DataTable otherTableWithTwoConsecutiveRowsChanged() {
String source = "" +
"| Aslak | aslak@email.com | 123 |\n" +
"| Joe | joe@NOSPAM.com | 234 |\n" +
"| Bryan | bryan@NOSPAM.org | 456 |\n" +
"| Ni | ni@email.com | 654 |\n";
return TableParser.parse(source, null);
}

private DataTable otherTableWithTwoConsecutiveRowsInserted() {
String source = "" +
"| Aslak | aslak@email.com | 123 |\n" +
"| Joe | joe@email.com | 234 |\n" +
"| Doe | joe@email.com | 234 |\n" +
"| Foo | schnickens@email.net | 789 |\n" +
"| Bryan | bryan@email.org | 456 |\n" +
"| Ni | ni@email.com | 654 |\n";
return TableParser.parse(source, null);
}

private DataTable otherTableWithDeletedAndInserted() {
String source = "" +
"| Aslak | aslak@email.com | 123 |\n" +
Expand Down Expand Up @@ -59,6 +87,7 @@ public void shouldFindDifferences() {
}
}


@Test(expected = TableDiffException.class)
public void shouldFindNewLinesAtEnd() {
try {
Expand Down Expand Up @@ -105,21 +134,68 @@ public void shouldFindNewLinesAtEndWhenUsingDiff() {
public void should_not_fail_with_out_of_memory() {
DataTable expected = TableParser.parse("" +
"| I'm going to work |\n", null);

List<List<String>> actual = new ArrayList<List<String>>();

actual.add(asList("I just woke up"));
actual.add(asList("I'm going to work"));
expected.diff(actual);
}

@Test(expected = TableDiffException.class)
public void should_diff_when_consecutive_deleted_lines() {
try {
List<List<String>> other = otherTableWithTwoConsecutiveRowsDeleted().raw();
table().diff(other);
} catch (TableDiffException e) {
String expected = "" +
"Tables were not identical:\n" +
" | Aslak | aslak@email.com | 123 |\n" +
" - | Joe | joe@email.com | 234 |\n" +
" - | Bryan | bryan@email.org | 456 |\n" +
" | Ni | ni@email.com | 654 |\n";
assertEquals(expected, e.getMessage());
throw e;
}

}

@Test(expected = TableDiffException.class)
public void should_diff_when_consecutive_changed_lines() {
try {
List<List<String>> other = otherTableWithTwoConsecutiveRowsChanged().raw();
table().diff(other);
} catch (TableDiffException e) {
String expected = "" +
"Tables were not identical:\n" +
" | Aslak | aslak@email.com | 123 |\n" +
" - | Joe | joe@email.com | 234 |\n" +
" - | Bryan | bryan@email.org | 456 |\n" +
" + | Joe | joe@NOSPAM.com | 234 |\n" +
" + | Bryan | bryan@NOSPAM.org | 456 |\n" +
" | Ni | ni@email.com | 654 |\n";
assertEquals(expected, e.getMessage());
throw e;
}

}

@Test(expected = TableDiffException.class)
public void should_diff_when_consecutive_inserted_lines() {
try {
expected.diff(actual);
List<List<String>> other = otherTableWithTwoConsecutiveRowsInserted().raw();
table().diff(other);
} catch (TableDiffException e) {
String expectedDiff = "" +
String expected = "" +
"Tables were not identical:\n" +
" + | I just woke up |\n";
assertEquals(expectedDiff, e.getMessage());
" | Aslak | aslak@email.com | 123 |\n" +
" | Joe | joe@email.com | 234 |\n" +
" + | Doe | joe@email.com | 234 |\n" +
" + | Foo | schnickens@email.net | 789 |\n" +
" | Bryan | bryan@email.org | 456 |\n" +
" | Ni | ni@email.com | 654 |\n";
assertEquals(expected, e.getMessage());
throw e;
}

}

public static class TestPojo {
Expand All @@ -134,6 +210,7 @@ public TestPojo(Integer id, String givenName, int decisionCriteria) {
}
}


@Test
public void diff_with_list_of_pojos_and_camelcase_header_mapping() {
String source = "" +
Expand Down