Inject logging callback into Evaluator, so we can silence the logging in our unit tests

6 files changed, 36 insertions, 100 deletions

diff --git a/core/src/main/scala/mill/Main.scala b/core/src/main/scala/mill/Main.scala
index 2c313e2d..bff8cb22 100644
--- a/core/src/main/scala/mill/Main.scala
+++ b/core/src/main/scala/mill/Main.scala
@@ -13,14 +13,19 @@ import ammonite.repl.Repl
 
 object Main {
 
-  def apply[T: Discovered](args: Seq[String], obj: T, watch: Path => Unit): Int = {
+  def apply[T: Discovered](args: Seq[String],
+                           obj: T,
+                           watch: Path => Unit,
+                           coloredOutput: Boolean): Int = {
+
+    val log = new Logger(coloredOutput)
 
     val Seq(selectorString, rest @_*) = args
     val selector = selectorString.split('.')
     val discovered = implicitly[Discovered[T]]
     val consistencyErrors = Discovered.consistencyCheck(obj, discovered)
     if (consistencyErrors.nonEmpty) {
-      println("Failed Discovered.consistencyCheck: " + consistencyErrors)
+      log.error("Failed Discovered.consistencyCheck: " + consistencyErrors)
       1
     } else {
       val mapping = Discovered.mapping(obj)(discovered)
@@ -48,7 +53,7 @@ object Main {
       }
       resolve(selector.toList, discovered.mirror) match{
         case Some(target) =>
-          val evaluator = new Evaluator(workspacePath, mapping)
+          val evaluator = new Evaluator(workspacePath, mapping, log.info)
           val evaluated = evaluator.evaluate(OSet(target))
           evaluated.transitive.foreach{
             case t: define.Source => watch(t.handle.path)
@@ -56,7 +61,11 @@ object Main {
           }
 
           val failing = evaluated.failing.items
-          println(evaluated.failing.keyCount + " targets failed")
+          evaluated.failing.keyCount match{
+            case 0 => // do nothing
+            case n => log.error(n + " targets failed")
+          }
+
 
           for((k, fs) <- failing){
             val ks = k match{
@@ -67,12 +76,12 @@ object Main {
               case Result.Exception(t) => t.toString
               case Result.Failure(t) => t
             }
-            println(ks + " " + fss.mkString(", "))
+            log.error(ks + " " + fss.mkString(", "))
           }
 
           if (evaluated.failing.keyCount == 0) 0 else 1
         case None =>
-          println("Unknown selector: " + selector.mkString("."))
+          log.error("Unknown selector: " + selector.mkString("."))
           1
       }
     }
@@ -132,19 +141,23 @@ object Main {
         }
     }
   }
-
 }
-class Main(config: Main.Config){
+
+class Logger(coloredOutput: Boolean){
   val colors =
-    if(config.colored.getOrElse(ammonite.Main.isInteractive())) Colors.Default
+    if(coloredOutput) Colors.Default
     else Colors.BlackWhite
 
-  def printInfo(s: String) = System.err.println(colors.info()(s))
-  def printError(s: String) = System.err.println(colors.error()(s))
+  def info(s: String) = System.err.println(colors.info()(s))
+  def error(s: String) = System.err.println(colors.error()(s))
+}
+class Main(config: Main.Config){
+  val coloredOutput = config.colored.getOrElse(ammonite.Main.isInteractive())
+  val log = new Logger(coloredOutput)
 
 
   def watchAndWait(watched: Seq[(Path, Long)]) = {
-    printInfo(s"Watching for changes to ${watched.length} files... (Ctrl-C to exit)")
+    log.info(s"Watching for changes to ${watched.length} files... (Ctrl-C to exit)")
     def statAll() = watched.forall{ case (file, lastMTime) =>
       Interpreter.pathSignature(file) == lastMTime
     }
@@ -154,11 +167,11 @@ class Main(config: Main.Config){
 
   def handleWatchRes[T](res: Res[T], printing: Boolean) = res match {
     case Res.Failure(msg) =>
-      printError(msg)
+      log.error(msg)
       false
 
     case Res.Exception(ex, s) =>
-      printError(
+      log.error(
         Repl.showException(ex, fansi.Color.Red, fansi.Attr.Reset, fansi.Color.Green)
       )
       false
@@ -194,7 +207,7 @@ class Main(config: Main.Config){
         val syntheticPath = pwd / 'out / "run.sc"
         write.over(
           syntheticPath,
-          """@main def run(args: String*) = mill.Main(args, ammonite.predef.FilePredef, interp.watch)
+          s"""@main def run(args: String*) = mill.Main(args, ammonite.predef.FilePredef, interp.watch, $coloredOutput)
             |
             |@main def idea() = mill.scalaplugin.GenIdea(ammonite.predef.FilePredef)
           """.stripMargin
@@ -216,7 +229,7 @@ class Main(config: Main.Config){
       }
 
       val delta = System.currentTimeMillis() - startTime
-      printInfo("Finished in " + delta/1000.0 + "s")
+      log.info("Finished in " + delta/1000.0 + "s")
       watchAndWait(watchedFiles)
       startTime = System.currentTimeMillis()
     } while(loop)
diff --git a/core/src/main/scala/mill/eval/Evaluator.scala b/core/src/main/scala/mill/eval/Evaluator.scala
index 3d9fbab0..be0edac4 100644
--- a/core/src/main/scala/mill/eval/Evaluator.scala
+++ b/core/src/main/scala/mill/eval/Evaluator.scala
@@ -8,7 +8,8 @@ import mill.util.{Args, MultiBiMap, OSet}
 
 import scala.collection.mutable
 class Evaluator(workspacePath: Path,
-                labeling: Map[Target[_], LabelledTarget[_]]){
+                labeling: Map[Target[_], LabelledTarget[_]],
+                log: String => Unit){
 
   def evaluate(goals: OSet[Task[_]]): Evaluator.Results = {
     mkdir(workspacePath)
@@ -78,7 +79,7 @@ class Evaluator(workspacePath: Path,
 
           case _ =>
 
-            println(fansi.Color.Blue("Running " + labelledTarget.segments.mkString(".")))
+            log("Running " + labelledTarget.segments.mkString("."))
             if (labelledTarget.target.flushDest) rm(destPath)
             val (newResults, newEvaluated) = evaluateGroup(group, results, Some(destPath))
 
diff --git a/core/src/test/scala/mill/MacroErrorTests.scala b/core/src/test/scala/mill/MacroErrorTests.scala
deleted file mode 100644
index b9de5594..00000000
--- a/core/src/test/scala/mill/MacroErrorTests.scala
+++ /dev/null
@@ -1,78 +0,0 @@
-package mill
-
-import utest._
-
-object MacroErrorTests extends TestSuite{
-
-  val tests = Tests{
-
-    'errors{
-      val expectedMsg =
-        "T{} members must be defs defined in a Cacher class/trait/object body"
-
-      val err = compileError("object Foo extends mill.define.Task.Module{ val x = T{1} }")
-      assert(err.msg == expectedMsg)
-    }
-
-    'badTmacro - {
-      // Make sure we can reference values from outside the T{...} block as part
-      // of our `Target#apply()` calls, but we cannot reference any values that
-      // come from inside the T{...} block
-      'pos - {
-        val e = compileError("""
-          val a = T{ 1 }
-          val arr = Array(a)
-          val b = {
-            val c = 0
-            T{
-              arr(c)()
-            }
-          }
-        """)
-        assert(e.msg.contains("must be defs"))
-      }
-      'neg - {
-
-        val expectedMsg =
-          "Target#apply() call cannot use `value n` defined within the T{...} block"
-        val err = compileError("""new mill.define.Task.Module{
-          def a = T{ 1 }
-          val arr = Array(a)
-          def b = {
-            T{
-              val n = 0
-              arr(n)()
-            }
-          }
-        }""")
-        assert(err.msg == expectedMsg)
-      }
-      'neg2 - {
-
-        val expectedMsg =
-          "Target#apply() call cannot use `value x` defined within the T{...} block"
-        val err = compileError("""new mill.define.Task.Module{
-          def a = T{ 1 }
-          val arr = Array(a)
-          def b = {
-            T{
-              arr.map{x => x()}
-            }
-          }
-        }""")
-        assert(err.msg == expectedMsg)
-      }
-      'neg3{
-        val borkedCachedDiamond1 = utest.compileError("""
-          object borkedCachedDiamond1 {
-            def up = T{ TestUtil.test() }
-            def left = T{ TestUtil.test(up) }
-            def right = T{ TestUtil.test(up) }
-            def down = T{ TestUtil.test(left, right) }
-          }
-        """)
-        assert(borkedCachedDiamond1.msg.contains("must be defs"))
-      }
-    }
-  }
-}
diff --git a/core/src/test/scala/mill/define/CacherTests.scala b/core/src/test/scala/mill/define/CacherTests.scala
index f3d07a29..bb86ae65 100644
--- a/core/src/test/scala/mill/define/CacherTests.scala
+++ b/core/src/test/scala/mill/define/CacherTests.scala
@@ -27,7 +27,7 @@ object CacherTests extends TestSuite{
 
     def eval[T: Discovered, V](base: T, v: Task[V])(implicit tp: TestPath) = {
       val workspace = ammonite.ops.pwd / 'target / 'workspace / tp.value
-      val evaluator = new Evaluator(workspace, Discovered.mapping(base))
+      val evaluator = new Evaluator(workspace, Discovered.mapping(base), _ => ())
       evaluator.evaluate(OSet(v)).values(0)
     }
 
diff --git a/core/src/test/scala/mill/eval/EvaluationTests.scala b/core/src/test/scala/mill/eval/EvaluationTests.scala
index 3b268911..ccb476e7 100644
--- a/core/src/test/scala/mill/eval/EvaluationTests.scala
+++ b/core/src/test/scala/mill/eval/EvaluationTests.scala
@@ -15,7 +15,7 @@ object EvaluationTests extends TestSuite{
     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, Discovered.mapping(base))
+    def evaluator = new Evaluator(workspace, Discovered.mapping(base), _ => ())
 
     def apply(target: Task[_], expValue: Any,
               expEvaled: OSet[Task[_]],
diff --git a/core/src/test/scala/mill/eval/JavaCompileJarTests.scala b/core/src/test/scala/mill/eval/JavaCompileJarTests.scala
index 6ec03cf3..72e8c858 100644
--- a/core/src/test/scala/mill/eval/JavaCompileJarTests.scala
+++ b/core/src/test/scala/mill/eval/JavaCompileJarTests.scala
@@ -49,7 +49,7 @@ object JavaCompileJarTests extends TestSuite{
       val mapping = Discovered.mapping(Build)
 
       def eval[T](t: Task[T]): Either[Result.Failing, (T, Int)] = {
-        val evaluator = new Evaluator(workspacePath, mapping)
+        val evaluator = new Evaluator(workspacePath, mapping, _ => ())
         val evaluated = evaluator.evaluate(OSet(t))
 
         if (evaluated.failing.keyCount == 0){
@@ -66,7 +66,7 @@ object JavaCompileJarTests extends TestSuite{
 
       }
       def check(targets: OSet[Task[_]], expected: OSet[Task[_]]) = {
-        val evaluator = new Evaluator(workspacePath, mapping)
+        val evaluator = new Evaluator(workspacePath, mapping, _ => ())
 
         val evaluated = evaluator.evaluate(targets)
           .evaluated