Fix `dest`-assignment for overriden `T.command`s, and add basic unit tests to validate the `dest` paths of overriden commands and overriden targets

7 files changed, 86 insertions, 34 deletions

diff --git a/core/test/src/mill/define/CacherTests.scala b/core/test/src/mill/define/CacherTests.scala
index eb981d46..20e2be2c 100644
--- a/core/test/src/mill/define/CacherTests.scala
+++ b/core/test/src/mill/define/CacherTests.scala
@@ -27,36 +27,36 @@ object CacherTests extends TestSuite{
   val tests = Tests{
 
 
-    def eval[V](mapping: mill.Module, v: Task[V])(implicit tp: TestPath) = {
+    def eval[V](mapping: mill.Module, discover: Discover, v: Task[V])(implicit tp: TestPath) = {
       val workspace = ammonite.ops.pwd / 'target / 'workspace / tp.value
-      val evaluator = new Evaluator(workspace, ammonite.ops.pwd, mapping, DummyLogger)
+      val evaluator = new Evaluator(workspace, ammonite.ops.pwd, mapping, discover, DummyLogger)
       evaluator.evaluate(Agg(v)).values(0)
     }
 
     'simpleDefIsCached - assert(
       Base.value eq Base.value,
-      eval(Base, Base.value) == 1
+      eval(Base, Discover[Base.type], Base.value) == 1
     )
 
     'resultDefIsCached - assert(
       Base.result eq Base.result,
-      eval(Base, Base.result) == 1
+      eval(Base, Discover[Base.type], Base.result) == 1
     )
 
 
     'overridingDefIsAlsoCached - assert(
-      eval(Middle, Middle.value) == 3,
+      eval(Middle, Discover[Middle.type], Middle.value) == 3,
       Middle.value eq Middle.value
     )
 
     'overridenDefRemainsAvailable - assert(
-      eval(Middle, Middle.overriden) == 1
+      eval(Middle, Discover[Middle.type], Middle.overriden) == 1
     )
 
 
     'multipleOverridesWork- assert(
-      eval(Terminal, Terminal.value) == 7,
-      eval(Terminal, Terminal.overriden) == 1
+      eval(Terminal, Discover[Terminal.type], Terminal.value) == 7,
+      eval(Terminal, Discover[Terminal.type], Terminal.overriden) == 1
     )
     //    Doesn't fail, presumably compileError doesn't go far enough in the
     //    compilation pipeline to hit the override checks
diff --git a/core/test/src/mill/eval/EvaluationTests.scala b/core/test/src/mill/eval/EvaluationTests.scala
index 0d14988a..563a2428 100644
--- a/core/test/src/mill/eval/EvaluationTests.scala
+++ b/core/test/src/mill/eval/EvaluationTests.scala
@@ -2,7 +2,7 @@ package mill.eval
 
 
 import mill.util.TestUtil.{Test, test}
-import mill.define.{Graph, Target, Task}
+import mill.define.{Discover, Graph, Target, Task}
 import mill.{Module, T}
 import mill.util.{DummyLogger, TestGraphs, TestUtil}
 import mill.util.Strict.Agg
@@ -10,11 +10,11 @@ import utest._
 import utest.framework.TestPath
 
 object EvaluationTests extends TestSuite{
-  class Checker(module: mill.Module)(implicit tp: TestPath) {
+  class Checker(module: mill.Module, discover: Discover)(implicit tp: TestPath) {
     val workspace = ammonite.ops.pwd / 'target / 'workspace / tp.value
     ammonite.ops.rm(ammonite.ops.Path(workspace, ammonite.ops.pwd))
     // Make sure data is persisted even if we re-create the evaluator each time
-    def evaluator = new Evaluator(workspace, ammonite.ops.pwd, module, DummyLogger)
+    def evaluator = new Evaluator(workspace, ammonite.ops.pwd, module, discover, DummyLogger)
 
     def apply(target: Task[_], expValue: Any,
               expEvaled: Agg[Task[_]],
@@ -28,7 +28,8 @@ object EvaluationTests extends TestSuite{
 
       val evaled = evaluator.evaluate(Agg(target))
 
-      val (matchingReturnedEvaled, extra) = evaled.evaluated.indexed.partition(expEvaled.contains)
+      val (matchingReturnedEvaled, extra) =
+        evaled.evaluated.indexed.partition(expEvaled.contains)
 
       assert(
         evaled.values == Seq(expValue),
@@ -57,7 +58,7 @@ object EvaluationTests extends TestSuite{
 
       'singleton - {
         import singleton._
-        val check = new Checker(singleton)
+        val check = new Checker(singleton, Discover[singleton.type])
         // First time the target is evaluated
         check(single, expValue = 0, expEvaled = Agg(single))
 
@@ -67,7 +68,7 @@ object EvaluationTests extends TestSuite{
       }
       'pair - {
         import pair._
-        val check = new Checker(pair)
+        val check = new Checker(pair, Discover[pair.type])
         check(down, expValue = 0, expEvaled = Agg(up, down))
 
         down.counter += 1
@@ -78,7 +79,7 @@ object EvaluationTests extends TestSuite{
       }
       'anonTriple - {
         import anonTriple._
-        val check = new Checker(anonTriple)
+        val check = new Checker(anonTriple, Discover[anonTriple.type])
         val middle = down.inputs(0)
         check(down, expValue = 0, expEvaled = Agg(up, middle, down))
 
@@ -94,7 +95,7 @@ object EvaluationTests extends TestSuite{
       }
       'diamond - {
         import diamond._
-        val check = new Checker(diamond)
+        val check = new Checker(diamond, Discover[diamond.type])
         check(down, expValue = 0, expEvaled = Agg(up, left, right, down))
 
         down.counter += 1
@@ -112,7 +113,7 @@ object EvaluationTests extends TestSuite{
       }
       'anonDiamond - {
         import anonDiamond._
-        val check = new Checker(anonDiamond)
+        val check = new Checker(anonDiamond, Discover[anonDiamond.type])
         val left = down.inputs(0).asInstanceOf[TestUtil.Test]
         val right = down.inputs(1).asInstanceOf[TestUtil.Test]
         check(down, expValue = 0, expEvaled = Agg(up, left, right, down))
@@ -133,7 +134,7 @@ object EvaluationTests extends TestSuite{
 
       'bigSingleTerminal - {
         import bigSingleTerminal._
-        val check = new Checker(bigSingleTerminal)
+        val check = new Checker(bigSingleTerminal, Discover[bigSingleTerminal.type])
 
         check(j, expValue = 0, expEvaled = Agg(a, b, e, f, i, j), extraEvaled = 22)
 
@@ -156,7 +157,7 @@ object EvaluationTests extends TestSuite{
         // even though one depends on the other
 
         import separateGroups._
-        val checker = new Checker(separateGroups)
+        val checker = new Checker(separateGroups, Discover[separateGroups.type])
         val evaled1 = checker.evaluator.evaluate(Agg(right, left))
         val filtered1 = evaled1.evaluated.filter(_.isInstanceOf[Target[_]])
         assert(filtered1 == Agg(change, left, right))
@@ -173,7 +174,7 @@ object EvaluationTests extends TestSuite{
       'triangleTask - {
 
         import triangleTask._
-        val checker = new Checker(triangleTask)
+        val checker = new Checker(triangleTask, Discover[triangleTask.type])
         checker(right, 3, Agg(left, right), extraEvaled = -1)
         checker(left, 1, Agg(), extraEvaled = -1)
 
@@ -181,7 +182,7 @@ object EvaluationTests extends TestSuite{
       'multiTerminalGroup - {
         import multiTerminalGroup._
 
-        val checker = new Checker(multiTerminalGroup)
+        val checker = new Checker(multiTerminalGroup, Discover[multiTerminalGroup.type])
         checker(right, 1, Agg(right), extraEvaled = -1)
         checker(left, 1, Agg(left), extraEvaled = -1)
       }
@@ -190,16 +191,60 @@ object EvaluationTests extends TestSuite{
 
         import multiTerminalBoundary._
 
-        val checker = new Checker(multiTerminalBoundary)
+        val checker = new Checker(multiTerminalBoundary, Discover[multiTerminalBoundary.type])
         checker(task2, 4, Agg(right, left), extraEvaled = -1, secondRunNoOp = false)
         checker(task2, 4, Agg(), extraEvaled = -1, secondRunNoOp = false)
       }
 
       'overrideSuperTask - {
+        // Make sure you can override targets, call their supers, and have the
+        // overriden target be allocated a spot within the overriden/ folder of
+        // the main publically-available target
         import canOverrideSuper._
 
-        val checker = new Checker(canOverrideSuper)
+        val checker = new Checker(canOverrideSuper, Discover[canOverrideSuper.type])
         checker(foo, Seq("base", "object"), Agg(foo), extraEvaled = -1)
+
+
+        val public = ammonite.ops.read(checker.workspace / 'foo / "meta.json")
+        val overriden = ammonite.ops.read(
+          checker.workspace / 'foo /
+            'overriden / "mill.util.TestGraphs.BaseModule#foo"  / "meta.json"
+        )
+        assert(
+          public.contains("base"),
+          public.contains("object"),
+          overriden.contains("base"),
+          !overriden.contains("object")
+        )
+      }
+      'overrideSuperCommand - {
+        // Make sure you can override commands, call their supers, and have the
+        // overriden command be allocated a spot within the overriden/ folder of
+        // the main publically-available command
+        import canOverrideSuper._
+
+        val checker = new Checker(canOverrideSuper, Discover[canOverrideSuper.type])
+        val runCmd = cmd(1)
+        checker(
+          runCmd,
+          Seq("base1", "object1"),
+          Agg(runCmd),
+          extraEvaled = -1,
+          secondRunNoOp = false
+        )
+
+        val public = ammonite.ops.read(checker.workspace / 'cmd / "meta.json")
+        val overriden = ammonite.ops.read(
+          checker.workspace / 'cmd /
+          'overriden / "mill.util.TestGraphs.BaseModule#cmd"  / "meta.json"
+        )
+        assert(
+          public.contains("base1"),
+          public.contains("object1"),
+          overriden.contains("base1"),
+          !overriden.contains("object1")
+        )
       }
 
       'tasksAreUncached - {
@@ -235,7 +280,7 @@ object EvaluationTests extends TestSuite{
 
         // During the first evaluation, they get computed normally like any
         // cached target
-        val check = new Checker(build)
+        val check = new Checker(build, Discover[build.type])
         assert(leftCount == 0, rightCount == 0)
         check(down, expValue = 10101, expEvaled = Agg(up, right, down), extraEvaled = 8)
         assert(leftCount == 1, middleCount == 1, rightCount == 1)
diff --git a/core/test/src/mill/eval/FailureTests.scala b/core/test/src/mill/eval/FailureTests.scala
index 12ed345d..7d0d6421 100644
--- a/core/test/src/mill/eval/FailureTests.scala
+++ b/core/test/src/mill/eval/FailureTests.scala
@@ -1,6 +1,6 @@
 package mill.eval
 
-import mill.define.Target
+import mill.define.{Discover, Target}
 import mill.util.DummyLogger
 import mill.util.Strict.Agg
 import utest._
@@ -11,9 +11,9 @@ object FailureTests extends TestSuite{
   def workspace(implicit tp: TestPath) = {
     ammonite.ops.pwd / 'target / 'workspace / 'failure / implicitly[TestPath].value
   }
-  class Checker(module: mill.Module)(implicit tp: TestPath){
+  class Checker(module: mill.Module, discover: Discover)(implicit tp: TestPath){
 
-    val evaluator = new Evaluator(workspace, ammonite.ops.pwd, module, DummyLogger)
+    val evaluator = new Evaluator(workspace, ammonite.ops.pwd, module, discover, DummyLogger)
 
     def apply(target: Target[_], expectedFailCount: Int, expectedRawValues: Seq[Result[_]]) = {
 
@@ -37,7 +37,7 @@ object FailureTests extends TestSuite{
 
     'evaluateSingle - {
       ammonite.ops.rm(ammonite.ops.Path(workspace, ammonite.ops.pwd))
-      val check = new Checker(singleton)
+      val check = new Checker(singleton, Discover[singleton.type])
       check(
         target = singleton.single,
         expectedFailCount = 0,
@@ -73,7 +73,7 @@ object FailureTests extends TestSuite{
     }
     'evaluatePair - {
       ammonite.ops.rm(ammonite.ops.Path(workspace, ammonite.ops.pwd))
-      val check = new Checker(pair)
+      val check = new Checker(pair, Discover[pair.type])
       check(
         pair.down,
         expectedFailCount = 0,
diff --git a/core/test/src/mill/eval/JavaCompileJarTests.scala b/core/test/src/mill/eval/JavaCompileJarTests.scala
index 5dd9cae1..961c4b1f 100644
--- a/core/test/src/mill/eval/JavaCompileJarTests.scala
+++ b/core/test/src/mill/eval/JavaCompileJarTests.scala
@@ -2,7 +2,7 @@ package mill.eval
 
 import ammonite.ops.ImplicitWd._
 import ammonite.ops._
-import mill.define.{Input, Target, Task}
+import mill.define.{Discover, Input, Target, Task}
 import mill.modules.Jvm
 import mill.util.Ctx.DestCtx
 import mill.{Module, T}
@@ -50,7 +50,7 @@ object JavaCompileJarTests extends TestSuite{
       import Build._
 
       def eval[T](t: Task[T]) = {
-        val evaluator = new Evaluator(workspacePath, pwd, Build, DummyLogger)
+        val evaluator = new Evaluator(workspacePath, pwd, Build, Discover[Build.type], DummyLogger)
         val evaluated = evaluator.evaluate(Agg(t))
 
         if (evaluated.failing.keyCount == 0){
@@ -67,7 +67,7 @@ object JavaCompileJarTests extends TestSuite{
 
       }
       def check(targets: Agg[Task[_]], expected: Agg[Task[_]]) = {
-        val evaluator = new Evaluator(workspacePath, pwd, Build, DummyLogger)
+        val evaluator = new Evaluator(workspacePath, pwd, Build, Discover[Build.type], DummyLogger)
 
         val evaluated = evaluator.evaluate(targets)
           .evaluated
diff --git a/core/test/src/mill/eval/TaskTests.scala b/core/test/src/mill/eval/TaskTests.scala
index 3f265618..538b0c12 100644
--- a/core/test/src/mill/eval/TaskTests.scala
+++ b/core/test/src/mill/eval/TaskTests.scala
@@ -3,6 +3,7 @@ package mill.eval
 import utest._
 import ammonite.ops._
 import mill.T
+import mill.define.Discover
 import mill.util.TestEvaluator
 object TaskTests extends TestSuite{
   val tests = Tests{
@@ -57,6 +58,7 @@ object TaskTests extends TestSuite{
       // to re-evaluate, but normal Tasks behind a Target run once then are cached
       val check = new TestEvaluator(
         build,
+        Discover[build.type],
         pwd / 'target / 'workspace / "task-tests" / "inputs",
         pwd
       )
@@ -74,6 +76,7 @@ object TaskTests extends TestSuite{
       // Persistent tasks keep the working dir around between runs
       val check = new TestEvaluator(
         build,
+        Discover[build.type],
         pwd / 'target / 'workspace / "task-tests" / "persistent",
         pwd
       )
@@ -90,6 +93,7 @@ object TaskTests extends TestSuite{
       // Persistent task
       def check = new TestEvaluator(
         build,
+        Discover[build.type],
         pwd / 'target / 'workspace / "task-tests" / "worker",
         pwd
       )
diff --git a/core/test/src/mill/util/TestEvaluator.scala b/core/test/src/mill/util/TestEvaluator.scala
index e437017a..10483aae 100644
--- a/core/test/src/mill/util/TestEvaluator.scala
+++ b/core/test/src/mill/util/TestEvaluator.scala
@@ -1,13 +1,14 @@
 package mill.util
 
 import ammonite.ops.Path
-import mill.define.{Input, Target, Task}
+import mill.define.{Discover, Input, Target, Task}
 import mill.eval.{Evaluator, Result}
 import mill.util.Strict.Agg
 class TestEvaluator(module: mill.Module,
+                    discover: Discover,
                     workspacePath: Path,
                     basePath: Path){
-  val evaluator = new Evaluator(workspacePath, basePath, module, DummyLogger)
+  val evaluator = new Evaluator(workspacePath, basePath, module, discover, DummyLogger)
   //val evaluator = new Evaluator(workspacePath, basePath, module, new PrintLogger(true, ammonite.util.Colors.Default, System.out, System.out, System.err))
   def apply[T](t: Task[T]): Either[Result.Failing, (T, Int)] = {
     val evaluated = evaluator.evaluate(Agg(t))
diff --git a/core/test/src/mill/util/TestGraphs.scala b/core/test/src/mill/util/TestGraphs.scala
index 54c8d815..aa8865ce 100644
--- a/core/test/src/mill/util/TestGraphs.scala
+++ b/core/test/src/mill/util/TestGraphs.scala
@@ -174,10 +174,12 @@ object TestGraphs{
 
   trait BaseModule extends Module {
     def foo = T{ Seq("base") }
+    def cmd(i: Int) = T.command{ Seq("base" + i) }
   }
 
   object canOverrideSuper extends TestUtil.BaseModule with BaseModule {
     override def foo = T{ super.foo() ++ Seq("object") }
+    override def cmd(i: Int) = T.command{ super.cmd(i)() ++ Seq("object" + i) }
   }
 
   trait TraitWithModule extends Module{ outer =>