Merge pull request #25 from tindzk/bug/target-exit-code

ProcessHelper: Exit when command returned with non-zero code

src/main/scala/seed/Log.scala

@@ -3,18 +3,20 @@ package seed
 import seed.cli.util.Ansi._
 import seed.cli.util.ColourScheme._
 
-class Log(f: String => Unit) {
+class Log(f: String => Unit, map: String => String = identity) {
+  def prefix(text: String): Log = new Log(f, text + _)
+
   def error(message: String): Unit =
-    f(foreground(red2)(bold("[error]") + " " + message))
+    f(foreground(red2)(bold("[error]") + " " + map(message)))
 
   def warn(message: String): Unit =
-    f(foreground(yellow2)(bold("[warn]") + " " + message))
+    f(foreground(yellow2)(bold("[warn]") + "  " + map(message)))
 
   def debug(message: String): Unit =
-    f(foreground(green2)(bold("[debug]") + " " + message))
+    f(foreground(green2)(bold("[debug]") + " " + map(message)))
 
   def info(message: String): Unit =
-    f(foreground(blue2)(bold("[info]") + " " + message))
+    f(foreground(blue2)(bold("[info]") + "  " + map(message)))
 }
 
-object Log extends Log(println)
+object Log extends Log(println, identity)

src/main/scala/seed/cli/Build.scala

@@ -68,7 +68,7 @@ object Build {
 
           val bloop = util.BloopCli.compile(
             build, projectPath, buildModules, watch, log, onStdOut(build)
-          ).fold(Future.unit)(_.termination.map(_ => ()))
+          ).fold(Future.unit)(_.success)
 
           Future.sequence(futures :+ bloop).map(_ => ())
         }

src/main/scala/seed/cli/BuildTarget.scala

@@ -10,7 +10,6 @@ import seed.process.ProcessHelper
 
 import scala.concurrent.{Await, Future}
 import scala.concurrent.duration.Duration
-import scala.concurrent.ExecutionContext.Implicits.global
 
 object BuildTarget {
   def buildTargets(build: model.Build,
@@ -51,51 +50,55 @@ object BuildTarget {
     log.info(s"Build path: $buildPath")
 
     allTargets.map { case (m, t) =>
+      val customLog = log.prefix(s"[${format(m, t)}]: ")
+
       val modulePath = moduleProjectPaths(m)
       val target     = build.module(m).target(t)
 
       target.`class` match {
         case Some(c) =>
-          val bloopName = BuildConfig.targetName(build, c.module.module, c.module.platform)
+          val bloopName = BuildConfig.targetName(build, c.module.module,
+            c.module.platform)
           val args = List("run", bloopName, "-m", c.main)
-          val process = ProcessHelper.runBloop(projectPath, log.info,
-            Some(modulePath.toAbsolutePath.toString), Some(buildPath.toAbsolutePath.toString))(args: _*)
+          val process = ProcessHelper.runBloop(projectPath, customLog,
+            customLog.info, Some(modulePath.toAbsolutePath.toString),
+            Some(buildPath.toAbsolutePath.toString)
+          )(args: _*)
 
           if (target.await) {
-            log.info(s"[${format(m, t)}]: Awaiting process termination...")
-            Await.result(process.termination, Duration.Inf)
+            customLog.info("Awaiting process termination...")
+            Await.result(process.success, Duration.Inf)
             Future.unit
           } else {
-            process.termination.map(_ => ())
+            process.success
           }
 
         case None =>
-          if (watch && target.watchCommand.isDefined) {
+          if (watch && target.watchCommand.isDefined)
             target.watchCommand match {
-              case None          => Future.unit
+              case None      => Future.unit
               case Some(cmd) =>
                 val process = ProcessHelper.runShell(modulePath, cmd,
-                  buildPath.toAbsolutePath.toString,
-                  output => log.info(s"[${format(m, t)}]: " + output))
-                process.termination.map(_ => ())
+                  buildPath.toAbsolutePath.toString, customLog, customLog.info)
+                process.success
             }
-          } else {
+          else
             target.command match {
               case None => Future.unit
               case Some(cmd) =>
                 val process =
-                  ProcessHelper.runShell(modulePath, cmd, buildPath.toAbsolutePath.toString,
-                    output => log.info(s"[${format(m, t)}]: " + output))
+                  ProcessHelper.runShell(modulePath, cmd,
+                    buildPath.toAbsolutePath.toString, customLog,
+                    customLog.info)
 
                 if (target.await) {
-                  log.info(s"[${format(m, t)}]: Awaiting process termination...")
-                  Await.result(process.termination, Duration.Inf)
+                  customLog.info("Awaiting process termination...")
+                  Await.result(process.success, Duration.Inf)
                   Future.unit
                 } else {
-                  process.termination.map(_ => ())
+                  process.success
                 }
             }
-        }
       }
     }
   }

src/main/scala/seed/cli/Link.scala

@@ -69,7 +69,7 @@ object Link {
 
           val bloop = util.BloopCli.link(
             build, projectPath, linkModules, watch, log, onStdOut(build)
-          ).fold(Future.unit)(_.termination.map(_ => ()))
+          ).fold(Future.unit)(_.success)
 
           Future.sequence(futures :+ bloop).map(_ => ())
         }

src/main/scala/seed/cli/util/BloopCli.scala

@@ -58,7 +58,7 @@ object BloopCli {
     if (bloopModules.isEmpty) None
     else {
       val args = "compile" +: ((if (!watch) List() else List("--watch")) ++ bloopModules)
-      Some(ProcessHelper.runBloop(projectPath, onStdOut)(args: _*))
+      Some(ProcessHelper.runBloop(projectPath, log, onStdOut)(args: _*))
     }
 
   def link(build: Build,
@@ -71,6 +71,6 @@ object BloopCli {
     if (bloopModules.isEmpty) None
     else {
       val args = "link" +: ((if (!watch) List() else List("--watch")) ++ bloopModules)
-      Some(ProcessHelper.runBloop(projectPath, onStdOut)(args: _*))
+      Some(ProcessHelper.runBloop(projectPath, log, onStdOut)(args: _*))
     }
 }

src/main/scala/seed/cli/util/Exit.scala

@@ -0,0 +1,10 @@
+package seed.cli.util
+
+object Exit {
+  var TestCases = false
+
+  def error(): Throwable = {
+    if (!TestCases) System.exit(1)
+    new Throwable
+  }
+}

src/main/scala/seed/process/ProcessHelper.scala

@@ -4,12 +4,11 @@ import java.nio.ByteBuffer
 import java.nio.file.Path
 
 import com.zaxxer.nuprocess.{NuAbstractProcessHandler, NuProcess, NuProcessBuilder}
+import seed.Log
 
 import scala.collection.JavaConverters._
 import scala.concurrent.{Future, Promise}
-
-import seed.Log
-import seed.cli.util.{Ansi, BloopCli}
+import seed.cli.util.{Ansi, BloopCli, Exit}
 
 sealed trait ProcessOutput
 object ProcessOutput {
@@ -49,11 +48,11 @@ class ProcessHandler(onLog: ProcessOutput => Unit,
 
 object ProcessHelper {
   /**
-    * @param nuProcess   Underlying NuProcess instance
-    * @param termination Future that terminates with status code
+    * @param nuProcess  Underlying NuProcess instance
+    * @param success    Future that terminates upon successful completion
     */
   class Process(private val nuProcess: NuProcess,
-                val termination: Future[Int]) {
+                val success: Future[Unit]) {
     private var _killed = false
 
     def isRunning: Boolean = nuProcess.isRunning
@@ -68,52 +67,60 @@ object ProcessHelper {
                   cmd: List[String],
                   modulePath: Option[String] = None,
                   buildPath: Option[String] = None,
-                  log: String => Unit
+                  log: Log,
+                  onStdOut: String => Unit
                  ): Process = {
-    log(s"Running command '${Ansi.italic(cmd.mkString(" "))}'...")
-    log(s"    Working directory: ${Ansi.italic(cwd.toString)}")
+    log.info(s"Running command '${Ansi.italic(cmd.mkString(" "))}'...")
+    log.debug(s"    Working directory: ${Ansi.italic(cwd.toString)}")
 
-    val termination = Promise[Int]()
+    val termination = Promise[Unit]()
 
     val pb = new NuProcessBuilder(cmd.asJava)
 
     modulePath.foreach { mp =>
       pb.environment().put("MODULE_PATH", mp)
-      log(s"    Module path: ${Ansi.italic(mp)}")
+      log.debug(s"    Module path: ${Ansi.italic(mp)}")
     }
 
     buildPath.foreach { bp =>
       pb.environment().put("BUILD_PATH", bp)
-      log(s"    Build path: ${Ansi.italic(bp)}")
+      log.debug(s"    Build path: ${Ansi.italic(bp)}")
     }
 
     pb.setProcessListener(new ProcessHandler(
       {
-        case ProcessOutput.StdOut(output) => log(output)
-        case ProcessOutput.StdErr(output) => log(output)
+        case ProcessOutput.StdOut(output) => onStdOut(output)
+        case ProcessOutput.StdErr(output) => log.error(output)
       },
-      pid => log("PID: " + pid),
-      { code =>
-        log("Process exited with code: " + code)
-        termination.success(code)
+      pid => log.debug("PID: " + pid),
+      code => {
+        log.debug("Exit code: " + code)
+        if (code == 0) termination.success(())
+        else {
+          log.error(s"Process exited with non-zero exit code")
+          termination.failure(Exit.error())
+        }
       }))
 
     if (cwd.toString != "") pb.setCwd(cwd)
     new Process(pb.start(), termination.future)
   }
 
   def runBloop(cwd: Path,
-               log: String => Unit,
+               log: Log,
+               onStdOut: String => Unit,
                modulePath: Option[String] = None,
                buildPath: Option[String] = None
               )(args: String*): Process =
     runCommmand(cwd, List("bloop") ++ args, modulePath, buildPath,
-      output => if (!BloopCli.skipOutput(output)) log(output))
+      log, output => if (!BloopCli.skipOutput(output)) onStdOut(output))
 
   def runShell(cwd: Path,
                command: String,
                buildPath: String,
-               log: String => Unit
+               log: Log,
+               onStdOut: String => Unit
               ): Process =
-    runCommmand(cwd, List("/bin/sh", "-c", command), None, Some(buildPath), log)
+    runCommmand(cwd, List("/bin/sh", "-c", command), None, Some(buildPath), log,
+      onStdOut)
 }

src/test/scala/seed/build/LinkSpec.scala

@@ -30,19 +30,16 @@ object LinkSpec extends TestSuite[Unit] {
       build, projectPath, List("example-js"), watch = false, Log, onStdOut)
 
     assert(process.isDefined)
-    TestProcessHelper.scheduleTermination(process.get)
 
     for {
-      code <- process.get.termination
-      _ <- Future {
-        require(process.get.killed || code == 0)
-        require(events.length == 3)
-        require(events(0) == BuildEvent.Compiling("example", Platform.JavaScript))
-        require(events(1) == BuildEvent.Compiled("example", Platform.JavaScript))
-        require(events(2).isInstanceOf[BuildEvent.Linked])
-        require(events(2).asInstanceOf[BuildEvent.Linked]
-          .path.endsWith("test/module-link/build/example.js"))
-      }
-    } yield ()
+      _ <- process.get.success
+    } yield {
+      require(events.length == 3)
+      require(events(0) == BuildEvent.Compiling("example", Platform.JavaScript))
+      require(events(1) == BuildEvent.Compiled("example", Platform.JavaScript))
+      require(events(2).isInstanceOf[BuildEvent.Linked])
+      require(events(2).asInstanceOf[BuildEvent.Linked]
+        .path.endsWith("test/module-link/build/example.js"))
+    }
   }
 }

src/test/scala/seed/generation/BloopIntegrationSpec.scala

@@ -7,6 +7,7 @@ import minitest.TestSuite
 import org.apache.commons.io.FileUtils
 import seed.{Log, cli}
 import seed.Cli.{Command, PackageConfig}
+import seed.cli.util.Exit
 import seed.config.BuildConfig
 import seed.generation.util.TestProcessHelper
 import seed.generation.util.TestProcessHelper.ec
@@ -16,6 +17,8 @@ import scala.concurrent.duration._
 import scala.concurrent.{Await, Future}
 
 object BloopIntegrationSpec extends TestSuite[Unit] {
+  Exit.TestCases = true
+
   override def setupSuite(): Unit = TestProcessHelper.semaphore.acquire()
   override def tearDownSuite(): Unit = TestProcessHelper.semaphore.release()
 
@@ -71,7 +74,7 @@ object BloopIntegrationSpec extends TestSuite[Unit] {
       }
   }
 
-  def buildCustomTarget(name: String): Future[Unit] = {
+  def buildCustomTarget(name: String, expectFailure: Boolean = false): Future[Unit] = {
     val path = Paths.get(s"test/$name")
 
     val BuildConfig.Result(build, projectPath, _) =
@@ -94,14 +97,17 @@ object BloopIntegrationSpec extends TestSuite[Unit] {
 
     val future = result.right.get
 
-    Await.result(future, 30.seconds)
+    if (expectFailure) future.failed.map(_ => ())
+    else {
+      Await.result(future, 30.seconds)
 
-    assert(Files.exists(generatedFile))
+      assert(Files.exists(generatedFile))
 
-    TestProcessHelper.runBloop(projectPath)("run", "demo")
-      .map { x =>
-        assertEquals(x.split("\n").count(_ == "42"), 1)
-      }
+      TestProcessHelper.runBloop(projectPath)("run", "demo")
+        .map { x =>
+          assertEquals(x.split("\n").count(_ == "42"), 1)
+        }
+    }
   }
 
   testAsync("Build project with custom class target") { _ =>
@@ -123,4 +129,8 @@ object BloopIntegrationSpec extends TestSuite[Unit] {
       assertEquals(result.project.dependencies, List())
     }
   }
+
+  testAsync("Build project with failing custom command target") { _ =>
+    buildCustomTarget("custom-command-target-fail", expectFailure = true)
+  }
 }

src/test/scala/seed/generation/util/TestProcessHelper.scala

@@ -1,7 +1,7 @@
 package seed.generation.util
 
 import java.nio.file.Path
-import java.util.concurrent.{Executors, Semaphore, TimeUnit}
+import java.util.concurrent.{Executors, Semaphore}
 
 import scala.concurrent.{ExecutionContext, Future}
 import seed.Log
@@ -16,35 +16,15 @@ object TestProcessHelper {
   // processes from running concurrently.
   val semaphore = new Semaphore(1)
 
-  private val scheduler = Executors.newScheduledThreadPool(1)
-  def schedule(seconds: Int)(f: => Unit): Unit =
-    scheduler.schedule({ () => f }: Runnable, seconds, TimeUnit.SECONDS)
-
-  /**
-    * Work around a CI problem where onExit() does not get called on
-    * [[seed.process.ProcessHandler]].
-    */
-  def scheduleTermination(process: ProcessHelper.Process): Unit =
-    TestProcessHelper.schedule(60) {
-      if (process.isRunning) {
-        Log.error(s"Process did not terminate after 60s")
-        Log.error("Forcing termination...")
-        process.kill()
-      }
-    }
-
   def runBloop(cwd: Path)(args: String*): Future[String] = {
     val sb = new StringBuilder
     val process = ProcessHelper.runBloop(cwd,
+      Log,
       { out =>
         Log.info(s"Process output: $out")
         sb.append(out + "\n")
       })(args: _*)
-    scheduleTermination(process)
 
-    process.termination.flatMap { statusCode =>
-      if (process.killed || statusCode == 0) Future.successful(sb.toString)
-      else Future.failed(new Exception("Status code: " + statusCode))
-    }
+    process.success.map(_ => sb.toString)
   }
 }