Shift mixed task/target group-count tests into `GraphTests` with the other `groupAroundNamedArgs` tests, and move the graph definitions themselves into `TestGraphs` so they can be shared

3 files changed, 104 insertions, 76 deletions

diff --git a/core/src/test/scala/mill/define/GraphTests.scala b/core/src/test/scala/mill/define/GraphTests.scala
index a64c1390..cbf0b189 100644
--- a/core/src/test/scala/mill/define/GraphTests.scala
+++ b/core/src/test/scala/mill/define/GraphTests.scala
@@ -1,6 +1,7 @@
 package mill.define
 
 
+import mill.discover.Discovered
 import mill.eval.Evaluator
 import mill.util.{OSet, TestGraphs, TestUtil}
 import utest._
@@ -73,14 +74,14 @@ object GraphTests extends TestSuite{
 
         val topoSorted = Graph.topoSorted(Graph.transitiveTargets(OSet(target(base))))
 
-        val important = important0.map(_(base))
+        val important = important0.map(_ (base))
         val grouped = Graph.groupAroundImportantTargets(topoSorted) {
           case t: Target[_] if important.contains(t) => t
         }
         val flattened = OSet.from(grouped.values().flatMap(_.items))
 
         TestUtil.checkTopological(flattened)
-        for((terminal, expectedSize) <- expected){
+        for ((terminal, expectedSize) <- expected) {
           val grouping = grouped.lookupKey(terminal)
           assert(
             grouping.size == expectedSize,
@@ -88,6 +89,7 @@ object GraphTests extends TestSuite{
           )
         }
       }
+
       'singleton - check(singleton)(
         _.single,
         OSet(_.single),
@@ -146,6 +148,51 @@ object GraphTests extends TestSuite{
         )
       )
     }
+    'multiTerminalGroupCounts - {
+      def countGroups[T: Discovered](t: T, goals: Task[_]*) = {
+        val labeling = Discovered.mapping(t)
+        val topoSorted = Graph.topoSorted(
+          Graph.transitiveTargets(OSet.from(goals))
+        )
+        val grouped = Graph.groupAroundImportantTargets(topoSorted) {
+          case t: Target[_] if labeling.contains(t) || goals.contains(t) => t
+          case t if goals.contains(t) => t
+        }
+        grouped.keyCount
+      }
+
+      'separateGroups - {
+        import separateGroups._
+        val groupCount = countGroups(separateGroups, right, left)
+        assert(groupCount == 3)
+      }
+
+      'triangleTask - {
+        // Make sure the following graph ends up as a single group, since although
+        // `right` depends on `left`, both of them depend on the un-cached `task`
+        // which would force them both to re-compute every time `task` changes
+        import triangleTask._
+        val groupCount = countGroups(triangleTask, right, left)
+        assert(groupCount == 2)
+      }
+
+
+      'multiTerminalGroup - {
+        // Make sure the following graph ends up as two groups
+        import multiTerminalGroup._
+        val groupCount = countGroups(multiTerminalGroup, right, left)
+        assert(groupCount == 2)
+      }
+
+
+      'multiTerminalBoundary - {
+        // Make sure the following graph ends up as a three groups: one for
+        // each cached target, and one for the downstream task we are running
+        import multiTerminalBoundary._
+        val groupCount = countGroups(multiTerminalBoundary, task2)
+        assert(groupCount == 3)
+      }
+    }
 
 
   }
diff --git a/core/src/test/scala/mill/eval/EvaluationTests.scala b/core/src/test/scala/mill/eval/EvaluationTests.scala
index 388f9f38..df18da15 100644
--- a/core/src/test/scala/mill/eval/EvaluationTests.scala
+++ b/core/src/test/scala/mill/eval/EvaluationTests.scala
@@ -47,17 +47,7 @@ object EvaluationTests extends TestSuite{
       }
     }
   }
-  def countGroups[T: Discovered](t: T, goals: Task[_]*) = {
-    val labeling = Discovered.mapping(t)
-    val topoSorted = Graph.topoSorted(
-      Graph.transitiveTargets(OSet.from(goals))
-    )
-    val grouped = Graph.groupAroundImportantTargets(topoSorted) {
-      case t: Target[_] if labeling.contains(t) || goals.contains(t) => t
-      case t if goals.contains(t) => t
-    }
-    grouped.keyCount
-  }
+
 
   val tests = Tests{
     val graphs = new mill.util.TestGraphs()
@@ -163,24 +153,9 @@ object EvaluationTests extends TestSuite{
       'separateGroups - {
         // Make sure that `left` and `right` are able to recompute separately,
         // even though one depends on the other
-        //
-        //        _ left _
-        //       /        \
-        //  task1 -------- right
-        //               _/
-        // change - task2
-        object build extends Module{
-          val task1 = T.task{ 1 }
-          def left = T{ task1() }
-          val change = test()
-          val task2 = T.task{ change() }
-          def right = T{ task1() + task2() + left() + 1 }
 
-        }
-        import build._
-        val groupCount = countGroups(build, right, left)
-        assert(groupCount == 3)
-        val checker = new Checker(build)
+        import separateGroups._
+        val checker = new Checker(separateGroups)
         val evaled1 = checker.evaluator.evaluate(OSet(right, left))
         val filtered1 = evaled1.evaluated.filter(_.isInstanceOf[Target[_]])
         assert(filtered1 == OSet(change, left, right))
@@ -195,63 +170,26 @@ object EvaluationTests extends TestSuite{
 
       }
       'triangleTask - {
-        // Make sure the following graph ends up as a single group, since although
-        // `right` depends on `left`, both of them depend on the un-cached `task`
-        // which would force them both to re-compute every time `task` changes
-        //
-        //      _ left _
-        //     /        \
-        // task -------- right
-        object build extends Module{
-          val task = T.task{ 1 }
-          def left = T{ task() }
-          def right = T{ task() + left() + 1 }
-        }
-        import build._
-        val groupCount = countGroups(build, right, left)
-        assert(groupCount == 2)
-        val checker = new Checker(build)
+
+        import triangleTask._
+        val checker = new Checker(triangleTask)
         checker(right, 3, OSet(left, right), extraEvaled = -1)
         checker(left, 1, OSet(), extraEvaled = -1)
 
       }
       'multiTerminalGroup - {
-        // Make sure the following graph ends up as a single group
-        //
-        //      _ left
-        //     /
-        // task -------- right
-        object build extends Module{
-          val task = T.task{ 1 }
-          def left = T{ task() }
-          def right = T{ task() }
-        }
-        val groupCount = countGroups(build, build.right, build.left)
-        assert(groupCount == 2)
+        import multiTerminalGroup._
 
-        val checker = new Checker(build)
-        checker(build.right, 1, OSet(build.right), extraEvaled = -1)
-        checker(build.left, 1, OSet(build.left), extraEvaled = -1)
+        val checker = new Checker(multiTerminalGroup)
+        checker(right, 1, OSet(right), extraEvaled = -1)
+        checker(left, 1, OSet(left), extraEvaled = -1)
       }
 
       'multiTerminalBoundary - {
-        // Make sure the following graph ends up as a single group
-        //
-        //       _ left _____________
-        //      /        \           \
-        // task1 -------- right ----- task2
-        object build extends Module{
-          val task1 = T.task{ 1 }
-          def left = T{ task1() }
-          def right = T{ task1() + left() + 1 }
-          val task2 = T.task{ left() + right() }
-        }
-        import build._
-        val groupCount = countGroups(build, task2)
-        assert(groupCount == 3)
 
+        import multiTerminalBoundary._
 
-        val checker = new Checker(build)
+        val checker = new Checker(multiTerminalBoundary)
         checker(task2, 4, OSet(right, left), extraEvaled = -1, secondRunNoOp = false)
         checker(task2, 4, OSet(), extraEvaled = -1, secondRunNoOp = false)
       }
diff --git a/core/src/test/scala/mill/util/TestGraphs.scala b/core/src/test/scala/mill/util/TestGraphs.scala
index f93b6152..af25aabb 100644
--- a/core/src/test/scala/mill/util/TestGraphs.scala
+++ b/core/src/test/scala/mill/util/TestGraphs.scala
@@ -91,5 +91,48 @@ class TestGraphs(){
     }
     val j = test(test(i), test(i, f), test(f))
   }
+
+  //        _ left _
+  //       /        \
+  //  task1 -------- right
+  //               _/
+  // change - task2
+  object separateGroups extends Module{
+    val task1 = T.task{ 1 }
+    def left = T{ task1() }
+    val change = test()
+    val task2 = T.task{ change() }
+    def right = T{ task1() + task2() + left() + 1 }
+
+  }
+
+  //      _ left _
+  //     /        \
+  // task -------- right
+  object triangleTask extends Module{
+    val task = T.task{ 1 }
+    def left = T{ task() }
+    def right = T{ task() + left() + 1 }
+  }
+
+
+  //      _ left
+  //     /
+  // task -------- right
+  object multiTerminalGroup extends Module{
+    val task = T.task{ 1 }
+    def left = T{ task() }
+    def right = T{ task() }
+  }
+
+  //       _ left _____________
+  //      /        \           \
+  // task1 -------- right ----- task2
+  object multiTerminalBoundary extends Module{
+    val task1 = T.task{ 1 }
+    def left = T{ task1() }
+    def right = T{ task1() + left() + 1 }
+    val task2 = T.task{ left() + right() }
+  }
 }