extract testable functionality to unit tests

ibi-group · Jan 11, 2024 · 7521cc7 · 7521cc7
1 parent a55e37e
commit 7521cc7
Show file tree

Hide file tree

Showing 5 changed files with 113 additions and 23 deletions.
diff --git a/src/main/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidator.java b/src/main/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidator.java
@@ -13,9 +13,17 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 
+enum SharedStopsHeader {
+    STOP_GROUP_ID_INDEX,
+    STOP_ID_INDEX,
+    IS_PRIMARY_INDEX,
+    FEED_ID_INDEX
+}
 
 public class SharedStopsValidator extends FeedValidator {
     private static final Logger LOG = LoggerFactory.getLogger(SharedStopsValidator.class);
@@ -47,52 +55,62 @@ public SharedStopsValidator(Feed feed, SQLErrorStorage errorStorage, Project pro
         this.feedId = feedId;
     }
 
-    @Override
-    public void validate() {
-        String config = project.sharedStopsConfig;
-        if (config == null) {
-            return;
-        }
-
-        CsvReader configReader = CsvReader.parse(config);
+    public static String[] getHeaders(CsvReader configReader) throws IOException {
+        configReader.setHeaders(configReader.getRawRecord().split(","));
+        return configReader.getHeaders();
+    }
 
-        int STOP_GROUP_ID_INDEX = -1;
-        int STOP_ID_INDEX = -1;
-        int IS_PRIMARY_INDEX = -1;
-        int FEED_ID_INDEX = -1;
+    public static Map<SharedStopsHeader, Integer> getHeaderIncedes(CsvReader configReader) {
+        HashMap map = new HashMap();
 
         try {
             configReader.readRecord();
+            String[] headers = getHeaders(configReader);
 
-            configReader.setHeaders(configReader.getRawRecord().split(","));
-            String[] headers = configReader.getHeaders();
             for (int i = 0; i < headers.length; i++) {
                 String header = headers[i];
                 switch (header) {
                     case "stop_group_id":
-                        STOP_GROUP_ID_INDEX = i;
+                        map.put(SharedStopsHeader.STOP_GROUP_ID_INDEX, i);
                         break;
                     case "feed_id":
-                        FEED_ID_INDEX = i;
+                        map.put(SharedStopsHeader.FEED_ID_INDEX, i);
                         break;
                     case "stop_id":
-                        STOP_ID_INDEX = i;
+                        map.put(SharedStopsHeader.STOP_ID_INDEX, i);
                         break;
                     case "is_primary":
-                        IS_PRIMARY_INDEX = i;
+                        map.put(SharedStopsHeader.IS_PRIMARY_INDEX, i);
                         break;
                     default:
                         throw new RuntimeException("shared_stops.csv contained invalid headers!");
                 }
             }
 
-            if (STOP_GROUP_ID_INDEX == -1 || FEED_ID_INDEX == -1 || STOP_ID_INDEX == -1 || IS_PRIMARY_INDEX == -1) {
+            // TODO: some way to generate this from the enum?
+            if (!map.containsKey(SharedStopsHeader.STOP_GROUP_ID_INDEX) ||
+                    !map.containsKey(SharedStopsHeader.FEED_ID_INDEX) ||
+                    !map.containsKey(SharedStopsHeader.STOP_ID_INDEX) ||
+                    !map.containsKey(SharedStopsHeader.IS_PRIMARY_INDEX)) {
                 throw new RuntimeException("shared_stops.csv is missing headers!");
             }
         } catch (IOException e) {
             throw new RuntimeException("shared_stops.csv was invalid!", e);
         }
 
+        return map;
+    }
+
+    @Override
+    public void validate() {
+        String config = project.sharedStopsConfig;
+        if (config == null) {
+            return;
+        }
+
+        CsvReader configReader = CsvReader.parse(config);
+
+
         // Build list of stop Ids.
         List<String> stopIds = new ArrayList<>();
         List<Stop> stops = new ArrayList<>();
@@ -102,15 +120,18 @@ public void validate() {
             stopIds.add(stop.stop_id);
         }
 
+        Map indeces = getHeaderIncedes(configReader);
+
         // Initialize hashmaps to hold duplication info
         HashSet<String> seenStopIds = new HashSet<>();
         HashSet<String> stopGroupsWithPrimaryStops = new HashSet<>();
 
         try {
             while (configReader.readRecord()) {
-                String stopGroupId = configReader.get(STOP_GROUP_ID_INDEX);
-                String stopId = configReader.get(STOP_ID_INDEX);
-                String sharedStopFeedId = configReader.get(FEED_ID_INDEX);
+                // TODO: Avoid casting here? It must be possible with the enums
+                String stopGroupId = configReader.get((Integer) indeces.get(SharedStopsHeader.STOP_GROUP_ID_INDEX));
+                String stopId = configReader.get((Integer) indeces.get(SharedStopsHeader.STOP_ID_INDEX));
+                String sharedStopFeedId = configReader.get((Integer) indeces.get(SharedStopsHeader.FEED_ID_INDEX));
 
                 if (stopId.equals("stop_id")) {
                     // Swallow header row
@@ -133,7 +154,7 @@ public void validate() {
                 }
 
                 // Check for SS_02 (multiple primary stops per stop group)
-                if (configReader.get(IS_PRIMARY_INDEX).equals("1") || configReader.get(IS_PRIMARY_INDEX).equals("true")) {
+                if (configReader.get((Integer) indeces.get(SharedStopsHeader.IS_PRIMARY_INDEX)).equals("1") || configReader.get((Integer) indeces.get(SharedStopsHeader.IS_PRIMARY_INDEX)).equals("true")) {
                     if (stopGroupsWithPrimaryStops.contains(stopGroupId)) {
                         registerError(NewGTFSError.forFeed(NewGTFSErrorType.SHARED_STOP_GROUP_MULTIPLE_PRIMARY_STOPS, stopGroupId));
                     } else {

diff --git a/src/test/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidatorTest.java b/src/test/java/com/conveyal/datatools/manager/jobs/validation/SharedStopsValidatorTest.java
@@ -0,0 +1,51 @@
+package com.conveyal.datatools.manager.jobs.validation;
+
+import com.conveyal.datatools.UnitTest;
+import com.csvreader.CsvReader;
+import org.junit.jupiter.api.Test;
+
+import java.util.Map;
+
+import static com.zenika.snapshotmatcher.SnapshotMatcher.matchesSnapshot;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class SharedStopsValidatorTest extends UnitTest {
+    private void attemptToParseInvalidCSV(String csv, String expectedError) {
+        CsvReader configReader = CsvReader.parse(csv);
+        Exception exception = assertThrows(RuntimeException.class, () -> {
+            SharedStopsValidator.getHeaderIncedes(configReader);
+        });
+
+        String error = exception.getMessage();
+        assertEquals(expectedError, error);
+    }
+
+    private Map parseValidCSV(String csv) {
+        CsvReader configReader = CsvReader.parse(csv);
+        return SharedStopsValidator.getHeaderIncedes(configReader);
+    }
+
+    @Test
+    void canHandleIncorrectCsv() {
+        String invalid = "shared_stops.csv contained invalid headers!";
+        String missing = "shared_stops.csv is missing headers!";
+
+        attemptToParseInvalidCSV("this is a great string, but it's not a shared_stops csv!", invalid);
+        attemptToParseInvalidCSV("", invalid);
+        attemptToParseInvalidCSV("is_primary,stop_id", missing);
+        attemptToParseInvalidCSV("is_primary,feed_id,,stop_group_id", invalid);
+        attemptToParseInvalidCSV("is_primary,feed_id,stop_group_id", missing);
+        attemptToParseInvalidCSV("feed_id,   is_primary,    stop_group_id,stop_id", invalid);
+    }
+
+    @Test
+    void canHandleVariousCorrectCSV() {
+        assertThat(parseValidCSV("is_primary,feed_id,stop_id,stop_group_id"), matchesSnapshot());
+        assertThat(parseValidCSV("feed_id,stop_id,stop_group_id,is_primary"), matchesSnapshot());
+
+        // Only last is handled
+        assertThat(parseValidCSV("feed_id,is_primary,stop_group_id,stop_id,feed_id"), matchesSnapshot());
+    }
+}
diff --git a/...atools/manager/jobs/validation/SharedStopsValidatorTest/canHandleVariousCorrectCSV-0.json b/...atools/manager/jobs/validation/SharedStopsValidatorTest/canHandleVariousCorrectCSV-0.json
@@ -0,0 +1,6 @@
+{
+  "STOP_GROUP_ID_INDEX" : 3,
+  "STOP_ID_INDEX" : 2,
+  "IS_PRIMARY_INDEX" : 0,
+  "FEED_ID_INDEX" : 1
+}
diff --git a/...atools/manager/jobs/validation/SharedStopsValidatorTest/canHandleVariousCorrectCSV-1.json b/...atools/manager/jobs/validation/SharedStopsValidatorTest/canHandleVariousCorrectCSV-1.json
@@ -0,0 +1,6 @@
+{
+  "STOP_GROUP_ID_INDEX" : 2,
+  "STOP_ID_INDEX" : 1,
+  "IS_PRIMARY_INDEX" : 3,
+  "FEED_ID_INDEX" : 0
+}
diff --git a/...atools/manager/jobs/validation/SharedStopsValidatorTest/canHandleVariousCorrectCSV-2.json b/...atools/manager/jobs/validation/SharedStopsValidatorTest/canHandleVariousCorrectCSV-2.json
@@ -0,0 +1,6 @@
+{
+  "STOP_GROUP_ID_INDEX" : 2,
+  "STOP_ID_INDEX" : 3,
+  "IS_PRIMARY_INDEX" : 1,
+  "FEED_ID_INDEX" : 4
+}