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.

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

Google Feed Validator (Near) Parity #236

Merged
merged 7 commits into from
Jun 3, 2019
Merged

Google Feed Validator (Near) Parity #236

merged 7 commits into from
Jun 3, 2019

Conversation

landonreed
Copy link
Contributor

@landonreed landonreed commented May 24, 2019

Checklist

  • Appropriate branch selected (all PRs must first be merged to dev before they can be merged to master)
  • Any modified or new methods or classes have helpful JavaDoc and code is thoroughly commented
  • The description lists all applicable issues this PR seeks to resolve
  • The description lists any configuration setting(s) that differ from the default settings
  • All tests and CI builds passing

Description

Brings the validator library into (near) parity with the Google Feed Validator (re #167). The only thing remaining is better shape validation and some encoding/line ending stuff. This PR:

  • improves the time zone error message
  • checks that fare_attribute transfer_duration is zero if transfers are not permitted
  • adds some stop time checks (ensures a stop time has a purpose and that timepoints have times)
  • adds frequency overlap check
  • adds supporting tests

This also refactors how ErrorExpectations are handled so it partially addresses #233.

@landonreed landonreed requested review from a team and evansiroky May 24, 2019 15:57
@codecov-io
Copy link

codecov-io commented May 24, 2019

Codecov Report

Merging #236 into dev will increase coverage by 0.36%.
The diff coverage is 78.02%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #236      +/-   ##
============================================
+ Coverage     60.06%   60.43%   +0.36%     
- Complexity      904      927      +23     
============================================
  Files           145      147       +2     
  Lines          7435     7483      +48     
  Branches        877      885       +8     
============================================
+ Hits           4466     4522      +56     
+ Misses         2645     2634      -11     
- Partials        324      327       +3
Impacted Files Coverage Δ Complexity Δ
src/main/java/com/conveyal/gtfs/loader/Table.java 85.42% <ø> (ø) 73 <0> (ø) ⬇️
...conveyal/gtfs/validator/NewTripTimesValidator.java 65.21% <ø> (ø) 19 <0> (ø) ⬇️
...om/conveyal/gtfs/validator/SpeedTripValidator.java 46.15% <0%> (-1.47%) 14 <0> (ø)
src/main/java/com/conveyal/gtfs/loader/Feed.java 75.86% <100%> (+0.86%) 6 <0> (ø) ⬇️
...java/com/conveyal/gtfs/error/NewGTFSErrorType.java 100% <100%> (ø) 2 <0> (ø) ⬇️
...java/com/conveyal/gtfs/loader/EntityPopulator.java 84.79% <74.13%> (+1.67%) 26 <7> (+2) ⬆️
...va/com/conveyal/gtfs/validator/FaresValidator.java 85.71% <85.71%> (ø) 4 <4> (?)
...om/conveyal/gtfs/validator/FrequencyValidator.java 93.75% <93.75%> (ø) 14 <14> (?)
src/main/java/com/conveyal/gtfs/PatternFinder.java 85.22% <0%> (ø) 20% <0%> (ø) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79ec9dd...2a2aec7. Read the comment docs.

Copy link
Contributor

@evansiroky evansiroky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly good, just some minor comments about the frequency checks and recommendations for improving the tests.

@landonreed
Copy link
Contributor Author

Thanks, @evansiroky. Just addressed the comments.

@landonreed landonreed requested a review from evansiroky May 31, 2019 14:56
@landonreed landonreed mentioned this pull request Jun 3, 2019
5 tasks
// -- diagrams courtesy of esiroky --
// A wraps B.
// A: |---------|
// B: ___|--|____
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha, nice. :)

@evansiroky evansiroky assigned landonreed and unassigned evansiroky Jun 3, 2019
@landonreed landonreed merged commit 7fe6c6d into dev Jun 3, 2019
@landonreed landonreed deleted the feedvalidator-parity branch June 3, 2019 19:35
@landonreed
Copy link
Contributor Author

🎉 This PR is included in version 4.3.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants