From 25c147f45041f6bcfbff149a053a28e2bc37cea7 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Sun, 30 Apr 2023 14:14:35 -0700 Subject: [PATCH 1/6] Add notice documentation header check. --- .../notice/schema/NoticeSchemaGenerator.java | 2 +- .../validator/ExpiredCalendarValidator.java | 6 ++++++ .../FeedExpirationDateValidator.java | 8 ++++++++ .../validator/PathwayLoopValidator.java | 1 + .../validator/NoticeDocumentationTest.java | 20 +++++++++++++++++++ 5 files changed, 36 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/schema/NoticeSchemaGenerator.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/schema/NoticeSchemaGenerator.java index f4f92ed719..6bef9ed19f 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/schema/NoticeSchemaGenerator.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/schema/NoticeSchemaGenerator.java @@ -103,7 +103,7 @@ static NoticeSchema generateSchemaForNotice(Class noticeClass) return schema; } - private static NoticeDocComments loadComments(Class noticeClass) { + public static NoticeDocComments loadComments(Class noticeClass) { String resourceName = NoticeDocComments.getResourceNameForClass(noticeClass); InputStream is = noticeClass.getResourceAsStream(resourceName); if (is == null) { diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ExpiredCalendarValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ExpiredCalendarValidator.java index 0f35891549..1bfc53dd0f 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ExpiredCalendarValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ExpiredCalendarValidator.java @@ -66,6 +66,12 @@ public void validate(NoticeContainer noticeContainer) { } } + /** + * Dataset should not contain date ranges for services that have already expired. + * + *

This warning takes into account the `calendar_dates.txt` file as well as the `calendar.txt` + * file. + */ @GtfsValidationNotice( severity = WARNING, urls = { diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FeedExpirationDateValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FeedExpirationDateValidator.java index 6205accc62..414bc0ac1b 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FeedExpirationDateValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/FeedExpirationDateValidator.java @@ -78,6 +78,10 @@ public void validate(GtfsFeedInfo entity, NoticeContainer noticeContainer) { } } + /** + * The dataset expiration date defined in `feed_info.txt` is in seven days or less. At any time, + * the published GTFS dataset should be valid for at least the next 7 days. + */ @GtfsValidationNotice(severity = WARNING) static class FeedExpirationDate7DaysNotice extends ValidationNotice { @@ -106,6 +110,10 @@ static class FeedExpirationDate7DaysNotice extends ValidationNotice { } } + /** + * At any time, the GTFS dataset should cover at least the next 30 days of service, and ideally + * for as long as the operator is confident that the schedule will continue to be operated. + */ @GtfsValidationNotice( severity = WARNING, urls = { diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/PathwayLoopValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/PathwayLoopValidator.java index 748101071e..370c70f0d4 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/PathwayLoopValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/PathwayLoopValidator.java @@ -39,6 +39,7 @@ public void validate(GtfsPathway pathway, NoticeContainer noticeContainer) { } } + /** A pathway should not have same values for `from_stop_id` and `to_stop_id`x. */ @GtfsValidationNotice(severity = WARNING, files = @FileRefs(GtfsPathwaySchema.class)) static class PathwayLoopNotice extends ValidationNotice { diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeDocumentationTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeDocumentationTest.java index 99b941f5eb..ca78309f97 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeDocumentationTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeDocumentationTest.java @@ -1,11 +1,13 @@ package org.mobilitydata.gtfsvalidator.validator; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.regex.Matcher; @@ -16,6 +18,8 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mobilitydata.gtfsvalidator.notice.Notice; +import org.mobilitydata.gtfsvalidator.notice.NoticeDocComments; +import org.mobilitydata.gtfsvalidator.notice.schema.NoticeSchemaGenerator; @RunWith(JUnit4.class) public class NoticeDocumentationTest { @@ -51,6 +55,22 @@ public void testThatRulesMarkdownContainsHeadersForAllValidationNotices() throws assertThat(fromMarkdown).isEqualTo(fromSource); } + @Test + public void testThatAllValidationNoticesAreDocumented() { + List> noticesWithoutDocComment = + discoverValidationNoticeClasses() + .filter( + clazz -> { + NoticeDocComments docComments = NoticeSchemaGenerator.loadComments(clazz); + return docComments.getDocComment() == null; + }) + .collect(Collectors.toList()); + assertWithMessage( + "We expect all validation notices to have a documentation comment. If this test fails, it likely means that a Javadoc /** */ documentation header needs to be added to the following classes:") + .that(noticesWithoutDocComment) + .isEmpty(); + } + private static Stream> discoverValidationNoticeClasses() { return ClassGraphDiscovery.discoverNoticeSubclasses(ClassGraphDiscovery.DEFAULT_NOTICE_PACKAGES) .stream(); From 161674ae4b467e7c02691ec482284c69d9415e47 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Mon, 1 May 2023 09:56:31 -0700 Subject: [PATCH 2/6] Fix typo. --- .../gtfsvalidator/validator/PathwayLoopValidator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/PathwayLoopValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/PathwayLoopValidator.java index 370c70f0d4..602dcd6f52 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/PathwayLoopValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/PathwayLoopValidator.java @@ -39,7 +39,7 @@ public void validate(GtfsPathway pathway, NoticeContainer noticeContainer) { } } - /** A pathway should not have same values for `from_stop_id` and `to_stop_id`x. */ + /** A pathway should not have same values for `from_stop_id` and `to_stop_id`. */ @GtfsValidationNotice(severity = WARNING, files = @FileRefs(GtfsPathwaySchema.class)) static class PathwayLoopNotice extends ValidationNotice { From 6b09bd0bb2bcea961ef032bef14c48cdcd2b11f8 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Fri, 26 May 2023 09:32:23 -0700 Subject: [PATCH 3/6] Add a unit-test that enforces our documentation comment conventions, as described in https://github.com/MobilityData/gtfs-validator/blob/master/docs/NEW_RULES.md#2-document-the-new-rule --- .../validator/NoticeDocumentationTest.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeDocumentationTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeDocumentationTest.java index a10f2fa5c4..cff01fc902 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeDocumentationTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeDocumentationTest.java @@ -77,6 +77,40 @@ public void testThatAllValidationNoticesAreDocumented() { .isEmpty(); } + @Test + public void testThatAllValidationNoticesAreDocumentedWithFirstLine() { + List> noticesWithImproperMultilineDocComment = + discoverValidationNoticeClasses() + .filter( + clazz -> { + NoticeDocComments docComments = NoticeSchemaGenerator.loadComments(clazz); + if (docComments.getDocComment() == null) { + return false; + } + String[] lines = docComments.getDocComment().split("\n"); + if (lines.length == 0) { + return false; + } + // The first line of the comment must be a single sentence. + return lines[0].contains(". "); + }) + .collect(Collectors.toList()); + assertWithMessage( + "We expect all validation notices to have a documentation comment of the " + + "following form:\n" + + "\n" + + " Short single-sentence text describing the notice on a single line (required).\n" + + " \n" + + " Additional text further describing the notice with multiple additional sentences " + + "on multiple lines(optional).\n" + + "\n" + + "See https://github.com/MobilityData/gtfs-validator/blob/master/docs/NEW_RULES.md#2-document-the-new-rule for more details.
\n" + + "\n" + + "The following notice classes do not match that convention:") + .that(noticesWithImproperMultilineDocComment) + .isEmpty(); + } + @Test public void testThatValidationNoticesDoNotUseUnsupportedJavadocSyntax() { List noticesWithInvalidJavadoc = From 53b2cf40e9f3c00a6c93821cfd5eef9ad7c8f25f Mon Sep 17 00:00:00 2001 From: Kevin Clough Date: Mon, 29 May 2023 10:57:17 -0400 Subject: [PATCH 4/6] fix: correct typo for export notices schema flag (#1449) --- docs/USAGE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/USAGE.md b/docs/USAGE.md index bd1438156c..f6133fa2ba 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -71,7 +71,7 @@ Sample usage: ### Without file validation ``` -java -jar gtfs-validator-SNAPSHOT.jar --export_notice_schema +java -jar gtfs-validator-SNAPSHOT.jar --export_notices_schema ``` ...which will: @@ -79,7 +79,7 @@ java -jar gtfs-validator-SNAPSHOT.jar --export_notice_schema ### With file validation ``` -java -jar gtfs-validator-SNAPSHOT.jar --export_notice_schema --url https://url/to/dataset.zip --output relative/output/path --country_code --threads --storage_directory input.zip +java -jar gtfs-validator-SNAPSHOT.jar --export_notices_schema --url https://url/to/dataset.zip --output relative/output/path --country_code --threads --storage_directory input.zip ``` ...which will: From 707e5e6d264896f9d8d803d23eefb52547950356 Mon Sep 17 00:00:00 2001 From: Brian Donahue Date: Tue, 30 May 2023 13:07:25 -0400 Subject: [PATCH 5/6] fix: restore missing CSS (#1457) --- main/src/main/resources/report.html | 46 +++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/main/src/main/resources/report.html b/main/src/main/resources/report.html index f47bdf04ef..478529490c 100644 --- a/main/src/main/resources/report.html +++ b/main/src/main/resources/report.html @@ -146,7 +146,53 @@ flex-basis: 100%; } } + + table.accordion > tbody > tr.notice td, + table.accordion > tbody > tr.view th { + cursor: auto; + } + + table.accordion > tbody > tr.notice td:first-child, + table.accordion > tbody > tr.notice th:first-child { + position: relative; + padding-left: 20px; + } + + table.accordion > tbody > tr.notice td:first-child:before, + table.accordion > tbody > tr.notice th:first-child:before { + position: absolute; + top: 50%; + left: 5px; + width: 9px; + height: 16px; + margin-top: -8px; + color: #000; + content: "+"; + } + + table.accordion > tbody > tr.notice.open td:first-child:before, + table.accordion > tbody > tr.notice.open th:first-child:before { + content: "\2013"; + } + + table.accordion > tbody > tr.notice:hover { + background: #ddd; + } + + table.accordion > tbody > tr.notice.open { + background: #ddd; + color: black; + } + + table.accordion > tbody > tr.description { + display: none; + } + + table.accordion > tbody > tr.description.open { + display: table-row; + } +

GTFS Schedule Validation Report

From 9a4c85fb17af083dd3298f89d6ef286a43e27552 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Tue, 30 May 2023 20:21:58 -0700 Subject: [PATCH 6/6] Update for changes to NoticeDocComments. --- .../gtfsvalidator/validator/NoticeDocumentationTest.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeDocumentationTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeDocumentationTest.java index 03dfa89b91..72fd32375e 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeDocumentationTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/NoticeDocumentationTest.java @@ -84,15 +84,10 @@ public void testThatAllValidationNoticesAreDocumentedWithFirstLine() { .filter( clazz -> { NoticeDocComments docComments = NoticeSchemaGenerator.loadComments(clazz); - if (docComments.getDocComment() == null) { + if (docComments.getShortSummary() == null) { return false; } - String[] lines = docComments.getDocComment().split("\n"); - if (lines.length == 0) { - return false; - } - // The first line of the comment must be a single sentence. - return lines[0].contains(". "); + return docComments.getShortSummary().contains(". "); }) .collect(Collectors.toList()); assertWithMessage(