Skip to content

Commit d9329f0

Browse files
authored
fix: acceptance tests keep dropping errors for missing_required_column (#2007)
1 parent 4532574 commit d9329f0

File tree

2 files changed

+70
-9
lines changed

2 files changed

+70
-9
lines changed

output-comparator/src/main/java/org/mobilitydata/gtfsvalidator/outputcomparator/cli/Arguments.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323
/** Command-line arguments for output-comparator CLI. */
2424
public class Arguments {
2525

26+
public static final String REFERENCE_SYSTEM_ERRORS_NAME = "reference_errors.json";
27+
public static final String LATEST_SYSTEM_ERRORS_NAME = "latest_errors.json";
28+
2629
@Parameter(
2730
names = {"-d", "--report_directory"},
2831
description = "Directory where reports are stored.",
@@ -42,11 +45,21 @@ public class Arguments {
4245
description = "Name of the reference validation report.")
4346
private String referenceValidationReportName;
4447

48+
@Parameter(
49+
names = {"-rse", "--reference_system_errors_name"},
50+
description = "Name of the reference system errors file.")
51+
private String referenceSystemErrorsName;
52+
4553
@Parameter(
4654
names = {"-l", "--latest_report_name"},
4755
description = "Name of the latest validation report.")
4856
private String latestValidationReportName;
4957

58+
@Parameter(
59+
names = {"-lse", "--latest_system_errors_name"},
60+
description = "Name of the latest system errors file.")
61+
private String latestSystemErrorsName;
62+
5063
@Parameter(
5164
names = {"-p", "--percent_invalid_datasets_threshold"},
5265
description =
@@ -164,4 +177,22 @@ public Optional<String> getCommitSha() {
164177
public void setCommitSha(String commitSha) {
165178
this.commitSha = commitSha;
166179
}
180+
181+
public String getReferenceSystemErrorsName() {
182+
return referenceSystemErrorsName == null
183+
? REFERENCE_SYSTEM_ERRORS_NAME
184+
: referenceSystemErrorsName;
185+
}
186+
187+
public void setReferenceSystemErrorsName(String referenceSystemErrorsName) {
188+
this.referenceSystemErrorsName = referenceSystemErrorsName;
189+
}
190+
191+
public String getLatestSystemErrorsName() {
192+
return latestSystemErrorsName == null ? LATEST_SYSTEM_ERRORS_NAME : latestSystemErrorsName;
193+
}
194+
195+
public void setLatestSystemErrorsName(String latestSystemErrorsName) {
196+
this.latestSystemErrorsName = latestSystemErrorsName;
197+
}
167198
}

output-comparator/src/main/java/org/mobilitydata/gtfsvalidator/outputcomparator/cli/ValidationReportComparator.java

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ public Result compareValidationRuns(
7272
String sourceUrl = sourceUrlContainer.getUrlForSourceId(sourceId);
7373

7474
Path referenceReportPath = file.toPath().resolve(args.getReferenceValidationReportName());
75+
Path referenceErrorsPath = file.toPath().resolve(args.getReferenceSystemErrorsName());
7576
Path latestReportPath = file.toPath().resolve(args.getLatestValidationReportName());
77+
Path latestErrorsPath = file.toPath().resolve(args.getLatestSystemErrorsName());
7678
// in case a validation report does not exist for a sourceId we add the sourceId to
7779
// the list of corrupted sources
7880
if (!(referenceReportPath.toFile().exists() && latestReportPath.toFile().exists())) {
@@ -86,22 +88,31 @@ public Result compareValidationRuns(
8688
}
8789
ValidationReport referenceReport;
8890
ValidationReport latestReport;
89-
try {
90-
referenceReport = ValidationReport.fromPath(referenceReportPath);
91-
} catch (IOException ioException) {
92-
logger.atSevere().withCause(ioException).log("Error reading reference validation report");
91+
referenceReport = getValidationReport(referenceReportPath);
92+
if (referenceReport == null) {
9393
// in case a file is corrupted, add the sourceId to the list of corrupted sources
9494
corruptedSources.addCorruptedSource(sourceId, true, false, true, null);
9595
continue;
9696
}
97-
try {
98-
latestReport = ValidationReport.fromPath(latestReportPath);
99-
} catch (IOException ioException) {
100-
logger.atSevere().withCause(ioException).log("Error reading latest validation report");
97+
latestReport = getValidationReport(latestReportPath);
98+
if (latestReport == null) {
10199
// in case a file is corrupted, add the sourceId to the list of corrupted sources
102-
corruptedSources.addCorruptedSource(sourceId, true, true, true, false);
100+
corruptedSources.addCorruptedSource(sourceId, true, false, true, null);
101+
continue;
102+
}
103+
104+
// As an edge case, when the execution raise a system exception the report will contain
105+
// no notices but system notices are found in the system_errors.json file.
106+
// In this case, the system notices are not considered as corrupted sources.
107+
if (hasSystemErrors(referenceReport, referenceErrorsPath)) {
108+
corruptedSources.addCorruptedSource(sourceId, true, true, true, true);
109+
continue;
110+
}
111+
if (hasSystemErrors(latestReport, latestErrorsPath)) {
112+
corruptedSources.addCorruptedSource(sourceId, true, true, true, true);
103113
continue;
104114
}
115+
105116
newErrors.compareValidationReports(sourceId, sourceUrl, referenceReport, latestReport);
106117
droppedErrors.compareValidationReports(sourceId, sourceUrl, latestReport, referenceReport);
107118
newWarnings.compareValidationReports(sourceId, sourceUrl, referenceReport, latestReport);
@@ -140,6 +151,25 @@ public Result compareValidationRuns(
140151
return Result.create(report, reportSummaryString, failure);
141152
}
142153

154+
private boolean hasSystemErrors(ValidationReport validationReport, Path path) {
155+
var systemErrors = getValidationReport(path);
156+
return validationReport.getNotices().isEmpty()
157+
&& systemErrors != null
158+
&& !systemErrors.getNotices().isEmpty();
159+
}
160+
161+
private static ValidationReport getValidationReport(Path referenceReportPath) {
162+
ValidationReport referenceReport;
163+
try {
164+
referenceReport = ValidationReport.fromPath(referenceReportPath);
165+
} catch (IOException ioException) {
166+
logger.atSevere().withCause(ioException).log(
167+
"Error reading %s validation report", referenceReportPath);
168+
return null;
169+
}
170+
return referenceReport;
171+
}
172+
143173
@AutoValue
144174
abstract static class Result {
145175

0 commit comments

Comments
 (0)