Add couple of sanity checks, improve e2e test usability and fix minor bugs

tool/pom.xml

@@ -63,6 +63,23 @@
             <version>4.11</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.google.truth</groupId>
+            <artifactId>truth</artifactId>
+            <version>0.46</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>com.google.truth.extensions</groupId>
+            <artifactId>truth-proto-extension</artifactId>
+            <version>0.46</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>com.google.guava</groupId>
+            <artifactId>guava</artifactId>
+            <version>25.1-jre</version>
+        </dependency>
     </dependencies>
 
     <build>

tool/src/test/java/org/datacommons/tool/GenMcfTest.java

@@ -14,16 +14,16 @@
 
 package org.datacommons.tool;
 
-import static org.junit.Assert.assertTrue;
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
+import static org.junit.Assert.assertEquals;
 
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
+import org.datacommons.util.TmcfCsvParser;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -33,15 +33,24 @@
 // directory, add an input directory and an output directory. In the input directory, put the test
 // files you want to run the lint tool against. In the output directory, put a report.json file with
 // the expected report output.
+//
+// These tests can be run in a mode to produce golden files, as below:
+//    mvn -DgoldenFilesPrefix=$PWD/tool/src/test/resources/org/datacommons/tool test
+//
 public class GenMcfTest {
   @Rule public TemporaryFolder testFolder = new TemporaryFolder();
 
   @Test
   public void GenMcfTest() throws IOException {
+    // Set this so that the generated node IDs are deterministic
+    TmcfCsvParser.TEST_mode = true;
+
+    String goldenFilesPrefix = System.getProperty("goldenFilesPrefix");
     Main app = new Main();
     CommandLine cmd = new CommandLine(app);
     File[] testDirectories = new File(resourceFile("genmcf")).listFiles(File::isDirectory);
     for (File directory : testDirectories) {
+      System.err.println("Processing " + directory.getName());
       List<String> argsList = new ArrayList<>();
       argsList.add("genmcf");
       File[] inputFiles = new File(Path.of(directory.getPath(), "input").toString()).listFiles();
@@ -51,38 +60,33 @@ public void GenMcfTest() throws IOException {
       argsList.add("--output-dir=" + testFolder.getRoot().getPath());
       String[] args = argsList.toArray(new String[argsList.size()]);
       cmd.execute(args);
-      String actualReportString = TestUtil.getStringFromTestFile(testFolder, "report.json");
-      String expectedReportString = TestUtil.getStringFromOutputReport(directory.getPath());
-      TestUtil.assertReportFilesAreSimilar(expectedReportString, actualReportString);
-      Path actualGeneratedFilePath = Paths.get(testFolder.getRoot().getPath(), "generated.mcf");
-      Path expectedGeneratedFilePath = Path.of(directory.getPath(), "output", "generated.mcf");
-      assertTrue(areSimilarGeneratedMcf(expectedGeneratedFilePath, actualGeneratedFilePath));
+
+      Path actualGeneratedFilePath = TestUtil.getTestFilePath(testFolder, "generated.mcf");
+      Path actualReportPath = TestUtil.getTestFilePath(testFolder, "report.json");
+
+      if (goldenFilesPrefix != null && !goldenFilesPrefix.isEmpty()) {
+        Path goldenGeneratedPath =
+            Path.of(goldenFilesPrefix, "genmcf", directory.getName(), "output", "generated.mcf");
+        Files.copy(actualGeneratedFilePath, goldenGeneratedPath, REPLACE_EXISTING);
+        Path goldenReportPath =
+            Path.of(goldenFilesPrefix, "genmcf", directory.getName(), "output", "report.json");
+        Files.copy(actualReportPath, goldenReportPath, REPLACE_EXISTING);
+      } else {
+        Path expectedGeneratedFilePath =
+            TestUtil.getOutputFilePath(directory.getPath(), "generated.mcf");
+        Path expectedReportPath = TestUtil.getOutputFilePath(directory.getPath(), "report.json");
+        TestUtil.assertReportFilesAreSimilar(
+            directory,
+            TestUtil.readStringFromPath(expectedReportPath),
+            TestUtil.readStringFromPath(actualReportPath));
+        assertEquals(
+            org.datacommons.util.TestUtil.mcfFromFile(expectedGeneratedFilePath.toString()),
+            org.datacommons.util.TestUtil.mcfFromFile(actualGeneratedFilePath.toString()));
+      }
     }
   }
 
   private String resourceFile(String resource) {
     return this.getClass().getResource(resource).getPath();
   }
-
-  // When testing GeneratedMcf, can't just check against an expected file because When generating
-  // SVO MCF from csv and tmcf, Nodes will be assigned an ID that may not always be the same
-  private boolean areSimilarGeneratedMcf(Path expectedFilePath, Path actualFilePath)
-      throws IOException {
-    Iterator<String> actualFileLines = Files.lines(actualFilePath).iterator();
-    if (!new File(expectedFilePath.toString()).isFile()) {
-      return !actualFileLines.hasNext();
-    }
-    Iterator<String> expectedFileLines = Files.lines(expectedFilePath).iterator();
-    while (expectedFileLines.hasNext() && actualFileLines.hasNext()) {
-      String expectedLine = expectedFileLines.next();
-      String actualLine = actualFileLines.next();
-      if (expectedLine.contains("Node") && !expectedLine.contains("dcid")) {
-        continue;
-      }
-      if (!actualLine.trim().equals(expectedLine.trim())) {
-        return false;
-      }
-    }
-    return true;
-  }
 }

tool/src/test/java/org/datacommons/tool/LintTest.java

@@ -1,10 +1,14 @@
 package org.datacommons.tool;
 
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
+
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.List;
+import org.datacommons.util.TmcfCsvParser;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -14,16 +18,25 @@
 // directory, add an input directory and an output directory. In the input directory, put the test
 // files you want to run the lint tool against. In the output directory, put a report.json file with
 // the expected report output.
+//
+// These tests can be run in a mode to produce golden files, as below:
+//    mvn -DgoldenFilesPrefix=$PWD/tool/src/test/resources/org/datacommons/tool test
+//
 // TODO(shanth): Incorporate e2e test-cases for existence checks once this is generalized.
 public class LintTest {
   @Rule public TemporaryFolder testFolder = new TemporaryFolder();
 
   @Test
   public void LintTest() throws IOException {
+    // Set this so that the generated node IDs are deterministic
+    TmcfCsvParser.TEST_mode = true;
+
+    String goldenFilesPrefix = System.getProperty("goldenFilesPrefix");
     Main app = new Main();
     CommandLine cmd = new CommandLine(app);
     File[] testDirectories = new File(resourceFile("lint")).listFiles(File::isDirectory);
     for (File directory : testDirectories) {
+      System.err.println("Processing " + directory.getName());
       List<String> argsList = new ArrayList<>();
       argsList.add("lint");
       File[] inputFiles = new File(Path.of(directory.getPath(), "input").toString()).listFiles();
@@ -33,9 +46,18 @@ public void LintTest() throws IOException {
       argsList.add("--output-dir=" + testFolder.getRoot().getPath());
       String[] args = argsList.toArray(new String[argsList.size()]);
       cmd.execute(args);
-      String actualReportString = TestUtil.getStringFromTestFile(testFolder, "report.json");
-      String expectedReportString = TestUtil.getStringFromOutputReport(directory.getPath());
-      TestUtil.assertReportFilesAreSimilar(expectedReportString, actualReportString);
+      Path actualReportPath = TestUtil.getTestFilePath(testFolder, "report.json");
+      if (goldenFilesPrefix != null && !goldenFilesPrefix.isEmpty()) {
+        Path goldenPath =
+            Path.of(goldenFilesPrefix, "lint", directory.getName(), "output", "report.json");
+        Files.copy(actualReportPath, goldenPath, REPLACE_EXISTING);
+      } else {
+        Path expectedReportPath = TestUtil.getOutputFilePath(directory.getPath(), "report.json");
+        TestUtil.assertReportFilesAreSimilar(
+            directory,
+            TestUtil.readStringFromPath(expectedReportPath),
+            TestUtil.readStringFromPath(actualReportPath));
+      }
     }
   }
 

tool/src/test/java/org/datacommons/tool/TestUtil.java

@@ -14,63 +14,109 @@
 
 package org.datacommons.tool;
 
-import static org.junit.Assert.assertEquals;
+import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
 import static org.junit.Assert.assertTrue;
 
+import com.google.protobuf.InvalidProtocolBufferException;
 import com.google.protobuf.util.JsonFormat;
 import java.io.File;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Collection;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 import org.apache.commons.io.FileUtils;
 import org.datacommons.proto.Debug;
 import org.junit.rules.TemporaryFolder;
 
 // Common set of utils used in e2e tests.
 public class TestUtil {
-  public static void assertReportFilesAreSimilar(String expected, String actual)
+  public static void assertReportFilesAreSimilar(File directory, String expected, String actual)
       throws IOException {
-    Debug.Log.Builder expectedLogBuilder = Debug.Log.newBuilder();
-    Debug.Log.Builder actualLogBuilder = Debug.Log.newBuilder();
-    JsonFormat.parser().merge(expected, expectedLogBuilder);
-    JsonFormat.parser().merge(actual, actualLogBuilder);
-    Debug.Log expectedLog = expectedLogBuilder.build();
-    Debug.Log actualLog = actualLogBuilder.build();
-    assertMapsAreEqual(
-        "Counter Set",
-        expectedLog.getCounterSet().getCountersMap(),
-        actualLog.getCounterSet().getCountersMap());
-    assertMapsAreEqual(
-        "Level Summary", expectedLog.getLevelSummaryMap(), actualLog.getLevelSummaryMap());
-    assertTrue(actualLog.getEntriesList().containsAll(expectedLog.getEntriesList()));
-    assertTrue(expectedLog.getEntriesList().containsAll(actualLog.getEntriesList()));
+    String testCase = directory.getName();
+    Debug.Log expectedLog = reportToProto(expected).build();
+    Debug.Log actualLog = reportToProto(actual).build();
+    // Compare the maps, printing log messages along the way and assert only at the very end.
+    boolean pass = true;
+    pass &=
+        areMapsEqual(
+            testCase,
+            "Counter Set",
+            expectedLog.getCounterSet().getCountersMap(),
+            actualLog.getCounterSet().getCountersMap());
+    pass &=
+        areMapsEqual(
+            testCase,
+            "Level Summary",
+            expectedLog.getLevelSummaryMap(),
+            actualLog.getLevelSummaryMap());
+    pass &= actualLog.getEntriesList().containsAll(expectedLog.getEntriesList());
+    pass &= expectedLog.getEntriesList().containsAll(actualLog.getEntriesList());
+    assertThat(actualLog).ignoringRepeatedFieldOrder().isEqualTo(expectedLog);
+    assertTrue(pass);
   }
 
-  public static void assertMapsAreEqual(
-      String mapType, Map<String, Long> expected, Map<String, Long> actual) {
-    assertEquals(
-        mapType + " has different size between actual and expected",
-        expected.keySet().size(),
-        actual.keySet().size());
+  private static boolean areMapsEqual(
+      String testCase, String mapType, Map<String, Long> expected, Map<String, Long> actual) {
+    boolean equal = true;
+    if (expected.keySet().size() > actual.keySet().size()) {
+      equal = false;
+      Set<String> diff = new HashSet<>((Collection<String>) expected.keySet());
+      diff.removeAll((Collection<String>) actual.keySet());
+      System.err.println(
+          testCase + " :: " + mapType + " has some missing keys: " + String.join(", ", diff));
+    } else if (expected.keySet().size() < actual.keySet().size()) {
+      equal = false;
+      Set<String> diff = new HashSet<>((Collection<String>) actual.keySet());
+      diff.removeAll((Collection<String>) expected.keySet());
+      System.err.println(
+          testCase + " :: " + mapType + " has some extra keys: " + String.join(", ", diff));
+    }
     for (String key : expected.keySet()) {
-      assertTrue(mapType + " actual report is missing the key: " + key, actual.containsKey(key));
-      assertEquals(
-          mapType + " has different values for the key: " + key,
-          expected.get(key),
-          actual.get(key));
+      if (!actual.containsKey(key)) {
+        equal = false;
+        System.err.println(mapType + " actual report is missing the key: " + key);
+      }
+      if (!expected.get(key).equals(actual.get(key))) {
+        equal = false;
+        System.err.println(
+            testCase
+                + " :: "
+                + mapType
+                + " has different values for the key "
+                + key
+                + " : expected ("
+                + expected.get(key)
+                + ") vs. actual ("
+                + actual.get(key)
+                + ")");
+      }
     }
+    return equal;
+  }
+
+  private static Debug.Log.Builder reportToProto(String report)
+      throws InvalidProtocolBufferException {
+    Debug.Log.Builder logBuilder = Debug.Log.newBuilder();
+    JsonFormat.parser().merge(report, logBuilder);
+    return logBuilder;
   }
 
-  public static String getStringFromTestFile(TemporaryFolder testFolder, String fileName)
+  public static Path getTestFilePath(TemporaryFolder testFolder, String fileName)
       throws IOException {
-    File file = new File(Paths.get(testFolder.getRoot().getPath(), fileName).toString());
-    return FileUtils.readFileToString(file, StandardCharsets.UTF_8);
+    return Paths.get(testFolder.getRoot().getPath(), fileName);
+  }
+
+  public static Path getOutputFilePath(String parentDirectoryPath, String fileName)
+      throws IOException {
+    return Path.of(parentDirectoryPath, "output", fileName);
   }
 
-  public static String getStringFromOutputReport(String parentDirectoryPath) throws IOException {
-    File file = new File(Path.of(parentDirectoryPath, "output", "report.json").toString());
+  public static String readStringFromPath(Path filePath) throws IOException {
+    File file = new File(filePath.toString());
     return FileUtils.readFileToString(file, StandardCharsets.UTF_8);
   }
 }