Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
📝 Acceptance Test Report📋 Summary❌ The rule acceptance test has failed for commit d1887dc 📊 Notices ComparisonNew Errors (0 out of 987 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 987 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (165 out of 987 datasets, ~17%) ❌Details of new errors due to code change, which is above the provided threshold of 1%.
Dropped Warnings (0 out of 987 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check16 out of 1003 sources (~2 %) are corrupted.
⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
skalexch
approved these changes
Feb 17, 2026
Contributor
skalexch
left a comment
skalexch
left a comment
There was a problem hiding this comment.
Logic-wise and looking at the tests, everything looks correct. I also checked a couple datasets from the acceptance tests and they check out. One of them even does the bad practice of defining all the service_ids with no active days in calendar.txt and then specifying the active days fully in calendar_dates.txt.
The acceptance tests are failing but I find it normal that a lot of the datasets are affected by this.
Member
Should we add the calendar_dates.txt to the notice's logic? Is it not a valid use case to have service IDs defined only in calendar_dates.txt in a feed that contains both files? |
Contributor
|
@davidgamez it is a valid use case to have service IDs defined only in calendar_dates.txt in a feed that contains both files.
The is what the spec actually says.:
The case that you mention has more to do with an "unusable_service_id". This is a very rare case where all the dates and added services are negated by I these cases can be left to an INFO notice for an upcoming release. |
Contributor
📝 Acceptance Test Report📋 Summary❌ The rule acceptance test has failed for commit 8dbbb47 📊 Notices ComparisonNew Errors (0 out of 987 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 987 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (165 out of 987 datasets, ~17%) ❌Details of new errors due to code change, which is above the provided threshold of 1%.
Dropped Warnings (0 out of 987 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check16 out of 1003 sources (~2 %) are corrupted.
⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
This pull request introduces a new validator to ensure that each service listed in
calendar.txthas at least one active day of the week. The validator checks for services that are not valid for any day and emits a warning notice if such cases are found. Comprehensive unit tests are also added to verify the validator's behavior.New validator for service activity:
ServiceHasNoActiveDayOfTheWeekValidatorclass to detect services inGtfsCalendarTableContainerthat have no active day of the week, emitting a warning notice for each such service.Expected behavior:
Please make sure these boxes are checked before submitting your pull request - thanks!
gradle testto make sure you didn't break anything