Fix #14 Extends GTFS types support -- Add support for Time and Date types#76
Fix #14 Extends GTFS types support -- Add support for Time and Date types#769 commits merged intomasterfrom
Conversation
-modified type in .proto file -updated bindings with protoc output -took the occasion to correct package name
-remove test files associated with removed types -also some formatting
-use new types in aciipb spec schema -add time GTFS type support -make parser and validator classes injectable -rewrite tests with mockito framework -remove now useless test fake schema resource files
-convert date strings to LocalDate type -extend test -remove unsupported type notice
ghost
requested review from
ghost
self-assigned this
…erage into gtfs-types � Conflicts: � adapter/parser/src/main/java/org/mobilitydata/gtfsvalidator/parser/GtfsEntityParser.java � adapter/parser/src/test/java/org/mobilitydata/gtfsvalidator/parser/GtfsEntityParserTest.java � adapter/repository/in-memory-simple/src/main/java/org/mobilitydata/gtfsvalidator/db/InMemoryGtfsSpecRepository.java
adapter/parser/src/main/java/org/mobilitydata/gtfsvalidator/parser/GtfsEntityParser.java
Show resolved
Hide resolved
adapter/parser/src/main/java/org/mobilitydata/gtfsvalidator/parser/GtfsEntityParser.java
Outdated
Show resolved
Hide resolved
adapter/parser/src/main/java/org/mobilitydata/gtfsvalidator/parser/GtfsEntityParser.java
Outdated
Show resolved
Hide resolved
-non nullability of constructors parameters
barbeau
left a comment
barbeau
left a comment
There was a problem hiding this comment.
@Fabrice-V Looking good! A few comments in-line.
adapter/parser/src/main/java/org/mobilitydata/gtfsvalidator/parser/GtfsEntityParser.java
Outdated
Show resolved
Hide resolved
| assertEquals(TEST_FILE_TST, notice.getFilename()); | ||
| assertEquals("Invalid color:AZ-FTJ in field:int_hex_with_regex for entity with id:test_id", | ||
| notice.getDescription()); | ||
| verify(mockTimeValidator, times(1)).isValid(ArgumentMatchers.eq("01:02:03")); |
There was a problem hiding this comment.
Time is tricky in GTFS - the spec says:
Time - Time in the HH:MM:SS format (H:MM:SS is also accepted). The time is measured from "noon minus 12h" of the service day (effectively midnight except for days on which daylight savings time changes occur). For times occurring after midnight, enter the time as a value greater than 24:00:00 in HH:MM:SS local time for the day on which the trip schedule begins.
Example: 14:30:00 for 2:30PM or 25:35:00 for 1:35AM on the next day.
There need to be a few more assertions here to make sure we're covering all the valid times.
I suggest verifying that the following are correctly parsed as legit times:
- "22:15:09"
- "25:05:55"
- "5:15:35"
There was a problem hiding this comment.
Ok, in those unit tests the isValid return value is mocked and the parameter check is just here to verify it is passed unchanged. That way we don't test the validator library code at the same time. Do you think we shall have integration tests as well?
| assertEquals("E016", notice.getId()); | ||
| assertEquals("Invalid time", notice.getTitle()); | ||
| assertEquals(TEST_FILE_TST, notice.getFilename()); | ||
| assertEquals("Invalid time:001:2:00003 in field:time_with_regex for entity with id:test_id", |
There was a problem hiding this comment.
See above - I'd suggest adding more times that should fail:
- "05:5:35"
- "05:05:5"
- "05:60:00"
- "05:00:60"
There was a problem hiding this comment.
Ok, in those unit tests the isValid return value is mocked and the parameter check is just here to verify it is passed unchanged. That way we don't test the validator library code at the same time. Do you think we shall have integration tests as well?
...emory-simple/src/main/java/org/mobilitydata/gtfsvalidator/db/InMemoryGtfsSpecRepository.java
Show resolved
Hide resolved
ghost
mentioned this pull request
-removed locale as the GTFS specification provide a format independent from the locale
1 participant
Summary:
This PR extends support of GTFS types to time and date
It also refactors some classes to make them injectable and the associated tests by introducing mockito framework use
Expected behavior:
All GTFS files should be validated, here for MTBA
close #14
Please make sure these boxes are checked before submitting your pull request - thanks!
gradle testto make sure you didn't break anything