Fix bad date NPE #143
Fix bad date NPE #143
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #143 +/- ##
======================================
Coverage ? 55.76%
Complexity ? 749
======================================
Files ? 144
Lines ? 7212
Branches ? 835
======================================
Hits ? 4022
Misses ? 2852
Partials ? 338
Continue to review full report at Codecov.
|
| @@ -181,6 +181,8 @@ public void complete(ValidationResult validationResult) { | |||
| LocalDate firstDate = LocalDate.MAX; | |||
| LocalDate lastDate = LocalDate.MIN; | |||
| for (LocalDate date : dateInfoForDate.keySet()) { | |||
| // If the date is invalid, skip. | |||
| if (date == null) continue; | |||
There was a problem hiding this comment.
How is this error happening? Shouldn't the computeIfAbsent method for adding keys and values never yield a null key? It'd be very helpful to add a failing test case to illustrate what is causing the problem here.
There was a problem hiding this comment.
@evansiroky, there are now tests that load feeds with bad date values. When you run canLoadFeedWithBadDates, you'll see that it does encounter a null key.
|
Reassigning to Landon. Would like an answer to #143 (comment) so I can do a better review. |
src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java
Outdated
Show resolved
Hide resolved
|
Assigning back to you @landonreed regarding #143 (comment). |
Conflicts: src/test/java/com/conveyal/gtfs/GTFSTest.java
|
Thanks for catching that, @evansiroky. I addressed that comment and fixed some merge conflicts. Let me know if it looks good. |
| Multimap<String, ValuePair> fieldsWithMismatches = ArrayListMultimap.create(); | ||
| // Check that no validators failed during validation. | ||
| assertThat( | ||
| "No validators failed during GTFS import.", |
There was a problem hiding this comment.
This string should be reworded as it is a failure message to display in the event that the assertion fails.
|
🎉 This PR is included in version 4.2.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Ensure that only non-null dates are processed by the
ServiceValidatorso that we don't cause exceptions when finding first/last dates.