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

BlockTripsWithOverlappingStopTimesNotice error not being caught #1089

Closed
rpedraza01 opened this issue Jan 28, 2022 · 6 comments · May be fixed by #1474
Closed

BlockTripsWithOverlappingStopTimesNotice error not being caught #1089

rpedraza01 opened this issue Jan 28, 2022 · 6 comments · May be fixed by #1474
Assignees
Labels
bug Something isn't working (crash, a rule has a problem) dependencies Pull requests that update a dependency file GTFS Reference Used for Adding or changing rules that belong in the GTFS reference status: Work in progress A PR that would close this issue has been opened.

Comments

@rpedraza01
Copy link

rpedraza01 commented Jan 28, 2022

Bug report

Describe the bug
Trillium Solutions is building a UI for the gtfs-validator. During our QA process, we encountered a potential bug in which the validator tool doesn't catch overlapping blocks and the BlockTripsWithOverlappingStopTimesNotice rule doesn't get triggered. We've created a GTFS dataset with purposely "bad" data to test the validator.

How we reproduce the bug
Steps to reproduce the behaviour:

  1. Find a GTFS dataset with known block overlaps or use the file we created
    (gtfs-validator-test-us.zip).
  2. Run your GTFS dataset of choice through the gtfs-validator.
  3. Read through the output searching for the BlockTripsWithOverlappingStopTimesNotice error but not find the error message.

Expected behaviour
We would expect the BlockTripsWithOverlappingStopTimesNotice error message to appear with all necessary identifying data points to allow a user to make corrections to the dataset.

Observed behaviour
BlockTripsWithOverlappingStopTimesNotice doesn't appear to be working properly.

Screenshots:
Screen Shot 2022-01-27 at 15 47 08
Image is from Trillium Solutions GTFS Manager software
There are six block overlaps that our own software is catching but gtfs-validator isn't.

We think investigating this possible bug at this file location would be a good start main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ and adding more test cases here.

Environment versions

  • validator version: 3.0
  • Java version: 11
  • OS versions: macOS 10.15.7

Please let me know if there's any other info y'all need.

@rpedraza01 rpedraza01 added the bug Something isn't working (crash, a rule has a problem) label Jan 28, 2022
@github-actions
Copy link
Contributor

Thank you for your reporting a bug. The issue has been placed in triage, the MobilityData team will follow-up on it.

@lionel-nj
Copy link
Contributor

lionel-nj commented Jan 31, 2022

Thanks for flagging this @rpedraza01 - From the first steps of the troubleshooting it appears that the root of the problem might be deeper than checking the rule's logic We might have to design additional tests to verify that validators were invoked. I'll check with @isabelle-dr and will get back to you shortly.

@isabelle-dr isabelle-dr added the GTFS Reference Used for Adding or changing rules that belong in the GTFS reference label Jun 29, 2022
@maximearmstrong maximearmstrong self-assigned this Jul 5, 2022
@maximearmstrong
Copy link
Contributor

Hi @rpedraza01! After testing the validator with your dataset, I found the problem. Currently, the validator uses the first and last stop times of each trip to compute the overlap intervals. If at least one arrival or departure time is missing for these stop times, the trip is excluded from the validation process. Specifically for your dataset, the problem is that the last stop time for each trip is missing both arrival and departure times, so the BlockTripsWithOverlappingStopTimesNotice is never generated.

Since the specification is allowing missing arrival and departure times, we should be considering this use case. Here are some options we can consider:

  1. Select the nearest stop time in the trip if the first or last one is missing time information. This would reduce the time window of the trip, but would still validate the trip. If the arrival or departure time is missing for each stop time in a trip, the trip would still be excluded.
  2. Keep the rule as is, but generate a WARNING notice, something like BlockTripsWithStopTimesWithoutTimesNotice, which would indicate a problem with the first or last stop time of a trip.
  3. Implement option 1 and generate a WARNING notice similarly to option 2 if the arrival or departure time is missing for each stop time of a trip.

These are ideas and should also be discussed with @isabelle-dr. She is currently out of the office, back on July 18. @rpedraza01 if you have other ideas or think any of these options would better solve your use case, please let us know :)

@dhersz
Copy link

dhersz commented Jul 6, 2022

Hi @maximearmstrong, just a quick note on your reply:

Since the specification is allowing missing arrival and departure times, we should be considering this use case.

Currently the specification for stop_times arrival_time reads:

(...) An arrival time must be specified for the first and the last stop in a trip.

It also reads for departure_time:

(...) If there are not separate times for arrival and departure at a stop, enter the same value for arrival_time and departure_time. (...)

So my interpretation of the specification is that it does not allow missing arrival and departure times as in the case of the sample GTFS provided. It allows missing arrival/departure times when they are not the first and last stops of a trip. The trip t_2027500_b_33665_tn_1, for example, is missing the second and the last stop times.

@maximearmstrong
Copy link
Contributor

Hi @dhersz, you're right. I missed that yesterday.

In that case, we may need a new ERROR notice, such as stop_time_trip_without_times, which would allow us to detect this problem and indicate that we can't generate the ERROR notice block_trips_with_overlapping_stop_times for the same trip because we are missing time information.

@isabelle-dr isabelle-dr added the status: Needs discussion We need a discussion on requirements before calling this issue ready label Oct 3, 2022
@isabelle-dr isabelle-dr added this to the Q1 2023 milestone Feb 14, 2023
@KClough KClough self-assigned this Feb 18, 2023
@isabelle-dr isabelle-dr moved this from Q1 2023 backlog to In Progress in Schedule Validator Q2 backlog Mar 21, 2023
@isabelle-dr isabelle-dr modified the milestones: Q1 2023, Next Mar 24, 2023
@isabelle-dr isabelle-dr moved this from In Progress to Jarvus Backlog (for MobilityData) in Schedule Validator Q2 backlog Apr 10, 2023
@isabelle-dr isabelle-dr added status: Work in progress A PR that would close this issue has been opened. and removed status: Needs discussion We need a discussion on requirements before calling this issue ready labels May 23, 2023
@isabelle-dr isabelle-dr added the dependencies Pull requests that update a dependency file label Jun 6, 2023
@isabelle-dr isabelle-dr removed this from the MobilityData Next milestone Jun 12, 2023
@isabelle-dr isabelle-dr added this to the MobilityData Now milestone Jun 12, 2023
@isabelle-dr isabelle-dr moved this from Jarvus Backlog (for MobilityData) to MobilityData Backlog in Schedule Validator Q2 backlog Jun 19, 2023
@isabelle-dr
Copy link
Contributor

Closing, this is replaced by #1485

@github-project-automation github-project-automation bot moved this from MobilityData Backlog (bugs) to Done in Schedule Validator Q2 backlog Jun 19, 2023
@emmambd emmambd removed this from the Now milestone Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working (crash, a rule has a problem) dependencies Pull requests that update a dependency file GTFS Reference Used for Adding or changing rules that belong in the GTFS reference status: Work in progress A PR that would close this issue has been opened.
Projects
Development

Successfully merging a pull request may close this issue.

7 participants