feat: add performance assessment to acceptance tests (#1771)

.github/workflows/acceptance_test.yml

@@ -1,5 +1,4 @@
 name: Rule acceptance tests
-
 on:
   pull_request:
     branches: [ master ]
@@ -16,16 +15,24 @@ on:
       - 'web/**'
       - '.github/workflows/web_**.yml'
       - '.github/workflows/stg_web_**.yml'
+    types: [ opened, synchronize, reopened, ready_for_review ]
 concurrency:
   group: ${{ github.head_ref }}
   cancel-in-progress: true
 jobs:
+  fail_if_pull_request_is_draft: # Fails in order to indicate that pull request needs to be marked as ready to review to pass.
+    if: github.event.pull_request.draft == true
+    runs-on: ubuntu-latest
+    steps:
+      - name: Fail if PR is a draft
+        run: exit 1
   pre_ci:
     name: Prepare CI environment
+    if: github.event.pull_request.draft == false # Skip this job and its dependencies if the PR is draft
     runs-on: ubuntu-latest
     steps:
       - name: Checkout Project
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
         with:
           # We need to fetch with a depth of 2 for pull_request so we can do HEAD^2
           fetch-depth: 2
@@ -48,62 +55,62 @@ jobs:
     needs: pre_ci
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
-      - uses: gradle/wrapper-validation-action@v1
+      - uses: actions/checkout@v4
+      - uses: gradle/actions/wrapper-validation@v3
   pack-snapshot:
     needs: [ validate-gradle-wrapper ]
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Set up JDK 11
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@v4
         with:
           java-version: '11'
           distribution: 'temurin'
       - name: Cache Gradle packages
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: ~/.gradle/caches
           key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
           restore-keys: ${{ runner.os }}-gradle
+      - name: Set up Gradle
+        uses: gradle/actions/setup-gradle@v3
       - name: Package cli app jar with Gradle
-        uses: gradle/gradle-build-action@v2
-        with:
-          arguments: shadowJar
+        run: ./gradlew shadowJar
       - name: Persist gtfs-validator snapshot jar
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: gtfs-validator-snapshot
           path: cli/build/libs/gtfs-validator-*-cli.jar
       - name: Persist comparator snapshot jar
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: comparator-snapshot
           path: output-comparator/build/libs/output-comparator-*-cli.jar
   pack-master:
     needs: [ validate-gradle-wrapper ]
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
         with:
           ref: master
       - name: Set up JDK 11
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@v4
         with:
           java-version: '11'
           distribution: 'temurin'
       - name: Cache Gradle packages
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: ~/.gradle/caches
           key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
           restore-keys: ${{ runner.os }}-gradle
+      - name: Set up Gradle
+        uses: gradle/actions/setup-gradle@v3
       - name: Package cli app jar with Gradle
-        uses: gradle/gradle-build-action@v2
-        with:
-          arguments: shadowJar
+        run: ./gradlew shadowJar
       - name: Persist gtfs-validator jar from master branch
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: gtfs-validator-master
           path: cli/build/libs/gtfs-validator-*-cli.jar
@@ -113,7 +120,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout repository code
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
       - name: Install dependencies
         run: |
           pip install -r scripts/mobility-database-harvester/requirements.txt
@@ -125,7 +132,7 @@ jobs:
           echo "matrix=$DATASETS" >> $GITHUB_OUTPUT
       - name: Persist metadata
         if: always()
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: datasets_metadata
           path: scripts/mobility-database-harvester/datasets_metadata
@@ -139,43 +146,58 @@ jobs:
     strategy:
       matrix: ${{ fromJson(needs.fetch-urls.outputs.matrix) }}
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Download .jar file from master branch
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         with:
           name: gtfs-validator-master
           path: gtfs-validator-master
       - name: Download latest changes .jar file from previous job
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         with:
           name: gtfs-validator-snapshot
           path: gtfs-validator-snapshot
+      - name: Extract and concatenate IDs
+        run: |
+          concatenated_ids=$(bash ./scripts/extract_ids.sh '${{ matrix.data }}')
+          echo "CONCATENATED_IDS=$concatenated_ids" >> $GITHUB_ENV
+          echo "CONCATENATED_IDS=$concatenated_ids"
       - name: Run validators on queued URLs
         run: |
           queue="${{ matrix.data }}"
           bash ./scripts/queue_runner.sh --include-master $queue
         env:
           OUTPUT_BASE: ${{ github.sha }}
       - name: Persist reports
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
-          name: reports_all
+          name: reports_${{ env.CONCATENATED_IDS }}
           path: ${{ github.sha }}/output
+  merge-reports-artifacts:
+     runs-on: ubuntu-latest
+     needs: [ get-reports ]
+     steps:
+       - name: Merge Artifacts
+         uses: actions/upload-artifact/merge@v4
+         with:
+           name: reports_all
+           pattern: reports_*
+           delete-merged: true
   compare-outputs:
-    needs: [ get-reports ]
+    needs: [ merge-reports-artifacts ]
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Download comparator .jar file from previous job
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         with:
           name: comparator-snapshot
       - name: Retrieve reports from previous job
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         with:
           name: reports_all
       - name: Retrieve gtfs latest versions from previous job
-        uses: actions/download-artifact@v3
+        uses: actions/download-artifact@v4
         with:
           name: datasets_metadata
       - name: Generate acceptance report test
@@ -193,15 +215,15 @@ jobs:
            --run_id ${{github.run_id}}
       - name: Persist acceptance test reports
         if: always()
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: acceptance_test_report
           path: acceptance-test-output
       - name: Generate PR comment
         id: generate-comment
         if: always()
         run: |
-          PR_COMMENT=$(< acceptance-test-output/acceptance_report_summary.txt)
+          PR_COMMENT=$(< acceptance-test-output/acceptance_report_summary.md)
           echo "PR_COMMENT<<EOF" >> $GITHUB_ENV
           echo "$PR_COMMENT" >> $GITHUB_ENV
           echo "EOF" >> $GITHUB_ENV

core/src/main/java/org/mobilitydata/gtfsvalidator/io/ValidationReportDeserializer.java

@@ -42,19 +42,28 @@
 public class ValidationReportDeserializer implements JsonDeserializer<ValidationReport> {
 
   private static final String NOTICES_MEMBER_NAME = "notices";
+  private static final String SUMMARY_MEMBER_NAME = "summary";
+  private static final String VALIDATION_TIME_MEMBER_NAME = "validationTimeSeconds";
 
   @Override
   public ValidationReport deserialize(
       JsonElement json, Type typoOfT, JsonDeserializationContext context) {
     Set<NoticeReport> notices = new LinkedHashSet<>();
     JsonObject rootObject = json.getAsJsonObject();
     // Note that the json file contains the summary in addition to the notices, but it is ignored
-    // since currently the report comparison is only on the notices
+    // since currently the report comparison is only on the notices and the validation time.
+    Double validationTimeSeconds = null;
+    if (rootObject.has(SUMMARY_MEMBER_NAME)) {
+      JsonObject summaryObject = rootObject.getAsJsonObject(SUMMARY_MEMBER_NAME);
+      if (summaryObject.has(VALIDATION_TIME_MEMBER_NAME)) {
+        validationTimeSeconds = summaryObject.get(VALIDATION_TIME_MEMBER_NAME).getAsDouble();
+      }
+    }
     JsonArray noticesArray = rootObject.getAsJsonArray(NOTICES_MEMBER_NAME);
     for (JsonElement childObject : noticesArray) {
       notices.add(Notice.GSON.fromJson(childObject, NoticeReport.class));
     }
-    return new ValidationReport(notices);
+    return new ValidationReport(notices, validationTimeSeconds);
   }
 
   public static <T extends Notice> JsonObject serialize(

core/src/main/java/org/mobilitydata/gtfsvalidator/model/ValidationReport.java

@@ -41,6 +41,7 @@ public class ValidationReport {
           .serializeSpecialFloatingPointValues()
           .create();
   private final Set<NoticeReport> notices;
+  private final Double validationTimeSeconds;
 
   /**
    * Public constructor needed for deserialization by {@code ValidationReportDeserializer}. Only
@@ -49,7 +50,19 @@ public class ValidationReport {
    * @param noticeReports set of {@code NoticeReport}s
    */
   public ValidationReport(Set<NoticeReport> noticeReports) {
+    this(noticeReports, null);
+  }
+
+  /**
+   * Public constructor needed for deserialization by {@code ValidationReportDeserializer}. Only
+   * stores information for error {@code NoticeReport}.
+   *
+   * @param noticeReports set of {@code NoticeReport}s
+   * @param validationTimeSeconds the time taken to validate the GTFS dataset
+   */
+  public ValidationReport(Set<NoticeReport> noticeReports, Double validationTimeSeconds) {
     this.notices = Collections.unmodifiableSet(noticeReports);
+    this.validationTimeSeconds = validationTimeSeconds;
   }
 
   /**
@@ -69,6 +82,10 @@ public Set<NoticeReport> getNotices() {
     return notices;
   }
 
+  public Double getValidationTimeSeconds() {
+    return validationTimeSeconds;
+  }
+
   /**
    * Determines if two validation reports are equal regardless of the order of the fields in the set
    * of {@code NoticeReport}.

main/src/main/java/org/mobilitydata/gtfsvalidator/report/JsonReportSummary.java

@@ -33,6 +33,7 @@ public class JsonReportSummary {
   private JsonReportFeedInfo feedInfo;
   private List<JsonReportAgencyMetadata> agencies;
   private Set<String> files;
+  private Double validationTimeSeconds;
 
   @SerializedName("counts")
   private JsonReportCounts jsonReportCounts;
@@ -65,6 +66,7 @@ public JsonReportSummary(
     if (feedMetadata != null) {
       if (feedMetadata.feedInfo != null) {
         this.feedInfo = new JsonReportFeedInfo(feedMetadata.feedInfo);
+        this.validationTimeSeconds = feedMetadata.validationTimeSeconds;
       } else {
         logger.atSevere().log(
             "No feed info for feed "

main/src/main/java/org/mobilitydata/gtfsvalidator/report/model/FeedMetadata.java

@@ -41,6 +41,8 @@ public class FeedMetadata {
   public ArrayList<AgencyMetadata> agencies = new ArrayList<>();
   private ImmutableSortedSet<String> filenames;
 
+  public double validationTimeSeconds;
+
   // List of features that only require checking the presence of one record in the file.
   private final List<Pair<String, String>> FILE_BASED_FEATURES =
       List.of(

main/src/main/java/org/mobilitydata/gtfsvalidator/runner/ValidationRunner.java

@@ -134,21 +134,22 @@ public Status run(ValidationRunnerConfig config) {
     }
     FeedMetadata feedMetadata = FeedMetadata.from(feedContainer, gtfsInput.getFilenames());
     closeGtfsInput(gtfsInput, noticeContainer);
+    feedMetadata.validationTimeSeconds = (System.nanoTime() - startNanos) / 1e9;
 
     // Output
     exportReport(feedMetadata, noticeContainer, config, versionInfo);
-    printSummary(startNanos, feedContainer, feedLoader, anyTableLoader);
+    printSummary(feedMetadata, feedContainer, feedLoader, anyTableLoader);
     return Status.SUCCESS;
   }
 
   /**
    * Prints validation metadata.
    *
-   * @param startNanos start time as nanoseconds
+   * @param feedMetadata the {@code FeedMetadata}
    * @param feedContainer the {@code GtfsFeedContainer}
    */
   public static void printSummary(
-      long startNanos,
+      FeedMetadata feedMetadata,
       GtfsFeedContainer feedContainer,
       GtfsFeedLoader loader,
       AnyTableLoader anyTableLoader) {
@@ -198,7 +199,7 @@ public static void printSummary(
 
       logger.atSevere().log(b.toString());
     }
-    logger.atInfo().log("Validation took %.3f seconds%n", (endNanos - startNanos) / 1e9);
+    logger.atInfo().log("Validation took %.3f seconds%n", feedMetadata.validationTimeSeconds);
     logger.atInfo().log(feedContainer.tableTotalsText());
   }
 

main/src/test/java/org/mobilitydata/gtfsvalidator/report/model/JsonReportSummaryTest.java

@@ -75,6 +75,7 @@ private static FeedMetadata generateFeedMetaData() {
             3 // Should not be present in the resulting GSON
             );
     feedMetadata.specFeatures = Map.of("Feature1", false, "Feature2", true);
+    feedMetadata.validationTimeSeconds = 100.0;
     return feedMetadata;
   }
 
@@ -118,6 +119,7 @@ public void withFeedMetadataWithConfigTest() throws Exception {
             + "\"countryCode\":\"GB\","
             + "\"dateForValidation\":\"2020-01-02\","
             + "\"feedInfo\":{\"publisherName\":\"value1\",\"publisherUrl\":\"value2\"},"
+            + "\"validationTimeSeconds\":100.0,"
             + "\"agencies\":["
             + "{\"name\":\"agency1\",\"url\":\"some URL 1\",\"phone\":\"phone1\",\"email\":\"email1\"},"
             + "{\"name\":\"agency1\",\"url\":\"some URL 1\",\"phone\":\"phone1\",\"email\":\"email1\"}],"
@@ -139,6 +141,7 @@ public void withFeedMetadataNoConfigTest() throws Exception {
             + "\"validatedAt\":\"now\","
             + "\"threads\":0,"
             + "\"feedInfo\":{\"publisherName\":\"value1\",\"publisherUrl\":\"value2\"},"
+            + "\"validationTimeSeconds\":100.0,"
             + "\"agencies\":["
             + "{\"name\":\"agency1\",\"url\":\"some URL 1\",\"phone\":\"phone1\",\"email\":\"email1\"},"
             + "{\"name\":\"agency1\",\"url\":\"some URL 1\",\"phone\":\"phone1\",\"email\":\"email1\"}],"

output-comparator/src/main/java/org/mobilitydata/gtfsvalidator/outputcomparator/cli/Main.java

@@ -36,11 +36,15 @@ public class Main {
 
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
   static final String ACCEPTANCE_REPORT_JSON = "acceptance_report.json";
-  static final String ACCEPTANCE_REPORT_SUMMARY_TXT = "acceptance_report_summary.txt";
+  static final String ACCEPTANCE_REPORT_SUMMARY_MD = "acceptance_report_summary.md";
   private static final int IO_EXCEPTION_EXIT_CODE = 1;
   private static final int COMPARISON_FAILURE_EXIT_CODE = 2;
   private static final Gson GSON =
-      new GsonBuilder().serializeNulls().disableHtmlEscaping().create();
+      new GsonBuilder()
+          .serializeNulls()
+          .disableHtmlEscaping()
+          .serializeSpecialFloatingPointValues()
+          .create();
 
   public static void main(String[] argv) {
     int rc = run(argv);
@@ -122,7 +126,7 @@ private static void exportAcceptanceReport(AcceptanceReport report, String outpu
   private static void exportReportSummary(String reportSummary, String outputBase) {
     try {
       Files.write(
-          Paths.get(outputBase, ACCEPTANCE_REPORT_SUMMARY_TXT),
+          Paths.get(outputBase, ACCEPTANCE_REPORT_SUMMARY_MD),
           reportSummary.getBytes(StandardCharsets.UTF_8));
     } catch (IOException e) {
       logger.atSevere().withCause(e).log("Cannot store acceptance test report summary file");