Merge remote-tracking branch 'otp/dev-2.x' into otp2_not_allowed_tran…

…sfers

.github/workflows/cibuild.yml

@@ -34,8 +34,15 @@ jobs:
           key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
           restore-keys: |
             ${{ runner.os }}-maven-
-      - name: Build and Test
-        run: mvn --batch-mode --update-snapshots verify
+      - name: Prepare coverage agent, build and test
+        run: mvn --batch-mode --update-snapshots jacoco:prepare-agent verify jacoco:report
+
+      - name: Send coverage data to codecov.io
+        if: github.repository_owner == 'opentripplanner'
+        uses: codecov/codecov-action@v2
+        with:
+          files: target/site/jacoco/jacoco.xml
+
       - name: Deploy to Github Package Registry
         if: github.event_name == 'push' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/dev-1.x' || github.ref == 'refs/heads/dev-2.x')
         env:

README.md

@@ -16,6 +16,8 @@ Additional information and instructions are available in the [main documentation
 
 ## Development 
 
+[![codecov](https://codecov.io/gh/opentripplanner/OpenTripPlanner/branch/dev-2.x/graph/badge.svg?token=ak4PbIKgZ1)](https://codecov.io/gh/opentripplanner/OpenTripPlanner)
+
 OpenTripPlanner is a collaborative project incorporating code, translation, and documentation from contributors around the world. We welcome new contributions. Further [development guidelines](http://docs.opentripplanner.org/en/latest/Developers-Guide/) can be found in the documentation.
 
 ### Development history

codecov.yml

@@ -0,0 +1 @@
+comment: false

docs/Changelog.md

@@ -110,6 +110,7 @@
 - Fix: Account for wait-time in no-wait Raptor strategy  [#3798](https://github.com/opentripplanner/OpenTripPlanner/pull/3798)
 - Read in flex window from Netex feeds [#3800](https://github.com/opentripplanner/OpenTripPlanner/pull/3800)
 - Fix NPE when routing on a graph without transit data. [#3804](https://github.com/opentripplanner/OpenTripPlanner/pull/3804)
+- Capture code coverage with jacoco and codecov.io [#3811](https://github.com/opentripplanner/OpenTripPlanner/pull/3811)
 [](AUTOMATIC_CHANGELOG_PLACEHOLDER_DO_NOT_REMOVE)
 
 

pom.xml

@@ -33,6 +33,10 @@
         <!-- Other properties -->
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <GITHUB_REPOSITORY>opentripplanner/OpenTripPlanner</GITHUB_REPOSITORY>
+        <!-- Set argLine to an empty string so that running the tests without the coverage agent works.
+             When running `mvn jacoco:prepare-agent test` argLine is replaced with the one activating the agent.
+        -->
+        <argLine></argLine>
     </properties>
 
     <distributionManagement>
@@ -222,8 +226,11 @@
                 <configuration>
                     <!-- we have to fork the JVM during tests so that the argLine is passed along -->
                     <forkCount>3</forkCount>
-                    <!-- enable the restricted reflection under Java 11 so that the ObjectDiffer works -->
+                    <!-- enable the restricted reflection under Java 11 so that the ObjectDiffer works
+                         the @{argLine} part is there to allow jacoco to insert its arguments as well
+                    -->
                     <argLine>
+                        @{argLine}
                         -Xmx2G
                         -Dfile.encoding=UTF-8
                         --illegal-access=permit
@@ -241,6 +248,12 @@
                     <disableXmlReport>false</disableXmlReport>
                 </configuration>
             </plugin>
+            <!-- code coverage report -->
+            <plugin>
+                <groupId>org.jacoco</groupId>
+                <artifactId>jacoco-maven-plugin</artifactId>
+                <version>0.8.7</version>
+            </plugin>
             <plugin>
                 <!-- Get current Git commit information for use in MavenVersion class.
                   Commit information is stored in Maven variables, which are then substituted

src/ext/java/org/opentripplanner/ext/transmodelapi/TransmodelGraphQLSchema.java

@@ -760,8 +760,10 @@ private GraphQLSchema create() {
                 .type(new GraphQLNonNull(Scalars.GraphQLID))
                 .build())
             .dataFetcher(environment -> {
-              return GqlUtil.getRoutingService(environment).getRouteForId(TransitIdMapper
-                  .mapIDToDomain(environment.getArgument("id")));
+                final String id = environment.getArgument("id");
+                if (id.isBlank()) { return null; }
+                return GqlUtil.getRoutingService(environment)
+                        .getRouteForId(TransitIdMapper.mapIDToDomain(id));
             })
             .build())
         .field(GraphQLFieldDefinition

src/main/java/org/opentripplanner/model/StationElement.java

@@ -116,6 +116,10 @@ public boolean isPartOfStation() {
    * (element).
    */
   public boolean isPartOfSameStationAs(StopLocation other) {
+    if (other == null) {
+      return false;
+    }
+
     return isPartOfStation() && parentStation.equals(other.getParentStation());
   }
 

src/main/java/org/opentripplanner/routing/api/request/RoutingRequest.java

@@ -1239,11 +1239,13 @@ public RoutingContext getRoutingContext() {
 
     /** Tear down any routing context (remove temporary edges from edge lists) */
     public void cleanup() {
-        if (this.rctx == null)
-            LOG.warn("routing context was not set, cannot destroy it.");
-        else {
-            rctx.destroy();
-            LOG.debug("routing context destroyed");
+        if (this.rctx != null) {
+            try {
+                rctx.destroy();
+            }
+            catch (Exception e) {
+                LOG.error("Could not destroy the routing context", e);
+            }
         }
     }
 

src/main/java/org/opentripplanner/routing/core/StateEditor.java

@@ -47,6 +47,11 @@ public StateEditor(State parent, Edge e) {
             child.backState = null;
             child.vertex = parent.vertex;
             child.stateData = child.stateData.clone();
+        } else if (e.getFromVertex() == null || e.getToVertex() == null) {
+            child.vertex = parent.vertex;
+            child.stateData = child.stateData.clone();
+            LOG.error("From or to vertex is null for {}", e);
+            defectiveTraversal = true;
         } else {
             // be clever
             // Note that we use equals(), not ==, here to allow for dynamically