fix: update TripAndShapeDistanceValidator to ignore shape with no shape_dist_traveled (#2018)

[thelleboid-tsr]

Summary:
update TripAndShapeDistanceValidator to ignore shape with no shape_dist_traveled

Expected behavior:

Some GTFS feeds do not provide shape_dist_traveled in shapes.txt In this case, the value is set to 0 in the code, so sorting shapes based on ShapeDistTraveled does not work and usually returns the first shape row.

It leads to an error, the first shape of the shape is usually very far away from the last stop of the trips following the shape.

We can assume the biggest shape_dist_traveled of a given shape should never be 0, otherwise it means the value is not set. In this case, we return early to ensure no validation error will be issued.

Fixes #2018

⚠️ I've not been able to run unit tests on this PR

• Run the unit tests with gradle test to make sure you didn't break anything
• Add or update any needed documentation to the repo
• Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
• Linked all relevant issues
• Include screenshot(s) showing how this pull request works and fixes the issue(s)

[welcome]

Thanks for opening this pull request! You're awesome. We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix. Examples of titles with semantic prefixes:

• fix: Bug with ssl network connections + Java module permissions.
• feat: Initial support for multiple @PrimaryKey annotations.
• docs: update RELEASE.md with new process
  To get this PR to the finish line, please do the following:
• Read our Contribution Guidelines
• Follow Google Java style coding standards
• Include tests when adding/changing behavior
• Include screenshots

[CLAassistant]

All committers have signed the CLA.

[emmambd]

Thank you for this great contribution @thelleboid-tsr! The team at MobilityData will take a look and review this week.

[jcpitre]

Thanks for the contribution!
Why this change?
It makes some tests fail:

TripAndShapeDistanceValidatorTest.testTripDistanceExceedsShapeDistance
TripAndShapeDistanceValidatorTest.testTripDistanceExceedsShapeDistanceWarning

Your test will work if you keep the line as is (i.e +1), since you set shapeDistTraveled to 0 when calling createShapeTable and only one row is created.

You can run the tests locally with:
./gradlew test

[jcpitre]

I think this will not really test your change. You want to set test data that would normally create a notice but will not because shapeTable.shapeDistTraveled is 0 (please correct me if I am wrong).
But to generate a notice you need the distance to be at least 11.1 meters. And this test sets the distance to 10 meters. So the test as it is now will not generate notices, because the distance falls below the threshold.

I believe that if you set createStopTimesTable(1, 100.0) (for example), it will test properly, meaning the distance is above the 11.1 meter threshold, but it will still not generate a notice because shapeTable.shapeDistTraveled is 0.

[thelleboid-tsr]

What I wanted to implement: "if ShapeDistTraveled is zero, then it does not generate notice nor warning" (because it returns early on line 89).

That's why I've tried setShapeDistTraveled(shapeDistTraveled * i) , so when shapeDistTraveled is 0, shapeDistTraveled * i is also 0 too.

I'm a bit lost about which tests currently fail. I do see job https://github.com/MobilityData/gtfs-validator/actions/runs/14531693846/job/40961320600?pr=2020 failed, but can't find where.

I'll revert my change to set shapeDistTraveled + i back, but I suspect the test I've introduced will raise a notice, instead of returning without any notice/warning.

Feel free to push changes on my PR to fix it! I don't mind at all (and will learn a bit about Java :-)

[skalexch]

@thelleboid-tsr I don't see a problem spec-wise. Once you're able to generate acceptance tests, I can take a look at the data and notices.

[emmambd]

Hi @skalexch! @thelleboid-tsr already generated the acceptance tests at https://github.com/MobilityData/gtfs-validator/actions/runs/14531693839 so we can review them there. There's a separate issue for generating a comment when the acceptance test is created from forked PRs. .

[skalexch]

@emmambd thank you. @thelleboid-tsr looking at the datasets, it seems like the error is dropped for both feeds that do not have the column (eg mdb-67) and feeds that have the field but it's empty (eg mdb-1270). All good spec-wise! I'll leave the approval for dev.