Merge pull request #1113 from hcoles/bug/fix_root_ordering

Ensure contained roots used before longer uncontained ones

pitest-entry/src/main/java/org/pitest/mutationtest/tooling/PathComparator.java

@@ -5,8 +5,12 @@
 /**
  * Comparator to allow ordering of Objects by their closeness to a
  * given root, assuming their toString provides a path like hierarchy.
+ * <p>
+ * Allows paths in same module as a file to be examined before those in other modules.
+ *
+ * Expects the base path supplied to be deeper than the shared root (e.g.
+ * /a/b/c/target/pit-reports when shared root is a/b/c)
  *
- * Allows paths in same module as a file to be examined before those in other modules
  */
 class PathComparator implements Comparator<Object> {
 
@@ -30,11 +34,19 @@ private int distanceFromBase(String s1) {
         String[] a = s1.split(separator);
 
         for (int i = 0; i != baseParts.length; i++) {
-            if (!a[i].equals(baseParts[i])) {
+            if (a.length == i || !a[i].equals(baseParts[i])) {
                 return i;
             }
+
+            if (a.length - 1 == i) {
+                // horrible fudge. This path is shorter than our base, but matches everything in it.
+                // We want to prioritise it above ones that mismatch, so we use a magic 1000. So long
+                // as the directory structure is not bizarrely deep, short fully matching paths will appear
+                // before longer mismatched paths.
+                return i + 1000;
+            }
         }
 
-        return baseParts.length;
+        return a.length;
     }
 }

pitest-entry/src/test/java/org/pitest/mutationtest/tooling/PathComparatorTest.java

@@ -7,6 +7,7 @@
 
 import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 
 
 public class PathComparatorTest {
@@ -19,6 +20,14 @@ public void identicalPathsCompareAsZero() {
         assertThat(underTest.compare(base,base)).isEqualTo(0);
     }
 
+    @Test
+    public void shortSubPathsAreWeightedAboveLongPathsThatDoNotMatchBase() {
+        String base = "a/b/more";
+        PathComparator underTest = new PathComparator(base, "/");
+
+        assertThat(underTest.compare("a/b", "a/b/c/d/e/f")).isLessThan(underTest.compare("a/b/c/d/e", "a/b/c/d/e/f/g/h/i"));
+    }
+
     @Test
     public void identicalPathsCompareAsZeroWhenNotUnderBase() {
         PathComparator underTest = new PathComparator(new File("different/path"), File.separator);
@@ -32,8 +41,7 @@ public void sameRootedPathComparesHigher() {
         PathComparator underTest = new PathComparator(base, File.separator);
         File sameRoot = new File("start/end/leaf");
         File differentRoot = new File("start/different/leaf");
-        assertThat(underTest.compare(differentRoot, sameRoot)).isEqualTo(1);
-        assertThat(underTest.compare(sameRoot, differentRoot)).isEqualTo(-1);
+        assertThat(underTest.compare(differentRoot, sameRoot)).isGreaterThan(underTest.compare(sameRoot, differentRoot));
     }
 
     @Test
@@ -72,4 +80,33 @@ public void worksWithLeadingSlashSeparator() {
         assertThat(paths).containsExactly("\\a\\b", "\\a\\b\\c", "\\a\\z");
     }
 
+    @Test
+    public void modulesUnderRootAlwaysSortedFirst() {
+        PathComparator underTest = new PathComparator("a/b/c/irrelevant", "/");
+        List<String> paths = asList("a/z", "a/b/c/d/", "a/b/c/d/e", "a/b/e", "a/b/cc");
+        paths.sort(underTest);
+        assertThat(paths.get(0)).isEqualTo("a/b/c/d/");
+        assertThat(paths.get(1)).isEqualTo("a/b/c/d/e");
+
+        paths = asList("a/b/cc", "a/b/e", "a/z", "a/b/c/d/", "a/b/c/d/e", "a/b/irrelevant" );
+
+        paths.sort(underTest);
+        assertThat(paths.get(0)).isEqualTo("a/b/c/d/");
+        assertThat(paths.get(1)).isEqualTo("a/b/c/d/e");
+    }
+
+    @Test
+    public void sortsByLengthWhenAllEquallyUnderRoot() {
+        PathComparator underTest = new PathComparator("a/b/c/irrelevant", "/");
+        List<String> paths = asList("a/b/x", "a/b/x/x", "a/b/x/x/x", "a/b/x/x/x/x", "a/b");
+        paths.sort(underTest);
+        assertThat(paths).containsExactly("a/b", "a/b/x",  "a/b/x/x", "a/b/x/x/x", "a/b/x/x/x/x");
+    }
+
+    @Test
+    public void handlesBaseIndexLongerThanSuppliedPath() {
+        PathComparator underTest = new PathComparator("/a/b/c/irrelevant", "/");
+        assertThatCode(() -> underTest.compare("a/z", "/a/b/c")).doesNotThrowAnyException();
+    }
+
 }