feat: recommend trip_headsign - #2192
davidgamez merged 2 commits into
Conversation
skalexch
left a comment
There was a problem hiding this comment.
Tested on a couple feeds and works well! All good on the spec and tests side.
📝 Acceptance Test Report📋 Summary❌ The rule acceptance test has failed for commit 6c03b76 📊 Notices ComparisonNew Errors (0 out of 1001 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 1001 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (224 out of 1001 datasets, ~22%) ❌Details of new errors due to code change, which is above the provided threshold of 1%.
Dropped Warnings (0 out of 1001 datasets, ~0%) ✅No changes were detected due to the code change. New Info Notices (0 out of 1001 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Info Notices (0 out of 1001 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check2 out of 1003 sources (~0 %) are corrupted.
🔍 System errors for mdb-1114 (reference)[
{
"code": "i_o_error",
"severity": "ERROR",
"totalNotices": 1,
"sampleNotices": [
{
"exception": "java.util.zip.ZipException",
"message": "Archive is not a ZIP archive"
}
]
}
]🔍 System errors for mdb-1114 (latest)[
{
"code": "i_o_error",
"severity": "ERROR",
"totalNotices": 1,
"sampleNotices": [
{
"exception": "java.util.zip.ZipException",
"message": "Archive is not a ZIP archive"
}
]
}
]🔍 System errors for mdb-1123 (reference)[
{
"code": "i_o_error",
"severity": "ERROR",
"totalNotices": 1,
"sampleNotices": [
{
"exception": "java.util.zip.ZipException",
"message": "Archive is not a ZIP archive"
}
]
}
]🔍 System errors for mdb-1123 (latest)[
{
"code": "i_o_error",
"severity": "ERROR",
"totalNotices": 1,
"sampleNotices": [
{
"exception": "java.util.zip.ZipException",
"message": "Archive is not a ZIP archive"
}
]
}
]💾 Out of Memory CheckNo datasets experienced an OutOfMemoryError. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
|
Thanks, I’ve reviewed the acceptance-test result. The 224 new The run shows:
The two corrupted sources also show the same ZIP errors in both reference and latest reports. So I don’t currently see evidence of an unintended regression here; the failure appears to come from the acceptance workflow’s 1% warning threshold being exceeded by the intended validation change. Happy to adjust anything if you’d prefer a different treatment. |
Hi @mackenziereading19, Thanks for reviewing the acceptance tests. This is standard procedure to ensure the implementation is valid against real datasets. The failure lets us check cases that affect a large number of feeds. This case is expected. |
Summary
Fixes #1801.
Marks
trip_headsignas a recommended GTFS field by adding@RecommendedtoGtfsTripSchema.tripHeadsign().The generated
GtfsTripTableDescriptorthen classifiestrip_headsignasFieldLevelEnum.RECOMMENDED, so the existing parser emitsMissingRecommendedFieldNoticewhen the field is present but empty.Tests
Added focused loader regression coverage for:
trip_headsign->MissingRecommendedFieldNoticetrip_headsign-> no missing-recommended noticeLocal verification:
GtfsTripTableLoaderTest: 2 passedTripHeadsignValidatorTest: 8 passedFeedMetadataTest: passedFieldLevelEnum.RECOMMENDED:main:spotlessCheck: passedImplementation commit:
16357dcc680754633ca882d042a71109c497e00d