GTFS FaresV2 time limit types 2 and 3 #7062
Conversation
leonardehrenfried
added
+Sandbox
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #7062 +/- ##
=============================================
+ Coverage 72.29% 72.49% +0.20%
- Complexity 20182 20456 +274
=============================================
Files 2189 2205 +16
Lines 81217 82241 +1024
Branches 8140 8229 +89
=============================================
+ Hits 58712 59622 +910
- Misses 19630 19695 +65
- Partials 2875 2924 +49 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
leonardehrenfried
force-pushed
the
more-transfer-types
branch
from
b4b10a7 to
22f8440
Compare
binh-dam-ibigroup
left a comment
binh-dam-ibigroup
left a comment
There was a problem hiding this comment.
Could you rename one test and its helper method to clarify what it does? Everything else seems to work fine.
| .withEndTime("10:20") | ||
| .build(); | ||
|
|
||
| private static List<Arguments> withInLimitCases() { |
There was a problem hiding this comment.
Rename to withinLimitCases.
|
|
||
| @ParameterizedTest | ||
| @MethodSource("belowLimitCases") | ||
| void belowLimit(TimeLimitType type, Duration duration) { |
There was a problem hiding this comment.
Rename to pastLimit or outsideLimit, to make it easy to see that the leg times fall outside of the transfer limit. Rename the method source accordingly.
There was a problem hiding this comment.
Yeah, this name is clearly incorrect. Thanks for spotting.
t2gran
left a comment
t2gran
left a comment
There was a problem hiding this comment.
There is a dependency from the RuleMapper to the Sandbox, this was already there before this PR - so it is ok to leave it. But If we want to split out Sandbox code into a separate module (we should), then the only allowed dependencies are those who is used by dependency injection(dagger).
|
I have been thinking about moving the fares code into its own module for little while now and have a few ideas how to do it. Right now I am focussing on the street model, but I want to tackle it at some stage. |
3 participants
Summary
@daniel-heppner-ibigroup has reported that the time limit types 2 and 3 of the Fares V2 spec are not implemented. This is fixed by this PR.
Unit tests
The majority of the code in this PR is about testing: a new implementation of TransitLeg including an easy-to-use builder was added.
Bumping the serialization version id