From 369a14fab2218bc745ee20c1a8ed02857af0e61f Mon Sep 17 00:00:00 2001 From: Tim Nieradzik Date: Mon, 15 Jul 2019 10:11:25 +0300 Subject: [PATCH] ProcessHelper: Exit when command returned with non-zero code Currently, the build process continues when a command is not found or exits with a non-zero code. This is undesired in CI setups as the developers would not be aware of any problems related to custom build targets. --- src/main/scala/seed/Log.scala | 14 ++--- src/main/scala/seed/cli/Build.scala | 2 +- src/main/scala/seed/cli/BuildTarget.scala | 41 ++++++++------- src/main/scala/seed/cli/Link.scala | 2 +- src/main/scala/seed/cli/util/BloopCli.scala | 4 +- src/main/scala/seed/cli/util/Exit.scala | 10 ++++ .../scala/seed/process/ProcessHelper.scala | 51 +++++++++++-------- src/test/scala/seed/build/LinkSpec.scala | 21 ++++---- .../generation/BloopIntegrationSpec.scala | 24 ++++++--- .../generation/util/TestProcessHelper.scala | 26 ++-------- test/custom-command-target-fail/build.toml | 9 ++++ .../demo/Main.scala | 3 ++ 12 files changed, 114 insertions(+), 93 deletions(-) create mode 100644 src/main/scala/seed/cli/util/Exit.scala create mode 100644 test/custom-command-target-fail/build.toml create mode 100644 test/custom-command-target-fail/demo/Main.scala diff --git a/src/main/scala/seed/Log.scala b/src/main/scala/seed/Log.scala index 105af1e..d1f8d65 100644 --- a/src/main/scala/seed/Log.scala +++ b/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) \ No newline at end of file +object Log extends Log(println, identity) diff --git a/src/main/scala/seed/cli/Build.scala b/src/main/scala/seed/cli/Build.scala index 04a964b..83c219e 100644 --- a/src/main/scala/seed/cli/Build.scala +++ b/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(_ => ()) } diff --git a/src/main/scala/seed/cli/BuildTarget.scala b/src/main/scala/seed/cli/BuildTarget.scala index f4caf8e..9f332df 100644 --- a/src/main/scala/seed/cli/BuildTarget.scala +++ b/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 } } - } } } } diff --git a/src/main/scala/seed/cli/Link.scala b/src/main/scala/seed/cli/Link.scala index 838578a..9012e6f 100644 --- a/src/main/scala/seed/cli/Link.scala +++ b/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(_ => ()) } diff --git a/src/main/scala/seed/cli/util/BloopCli.scala b/src/main/scala/seed/cli/util/BloopCli.scala index 3079e60..cd10be9 100644 --- a/src/main/scala/seed/cli/util/BloopCli.scala +++ b/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: _*)) } } diff --git a/src/main/scala/seed/cli/util/Exit.scala b/src/main/scala/seed/cli/util/Exit.scala new file mode 100644 index 0000000..84b5d32 --- /dev/null +++ b/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 + } +} diff --git a/src/main/scala/seed/process/ProcessHelper.scala b/src/main/scala/seed/process/ProcessHelper.scala index 78abfbb..5a951d5 100644 --- a/src/main/scala/seed/process/ProcessHelper.scala +++ b/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,34 +67,39 @@ 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) @@ -103,17 +107,20 @@ object ProcessHelper { } 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) } diff --git a/src/test/scala/seed/build/LinkSpec.scala b/src/test/scala/seed/build/LinkSpec.scala index 456cbbe..ed1d1c0 100644 --- a/src/test/scala/seed/build/LinkSpec.scala +++ b/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")) + } } } diff --git a/src/test/scala/seed/generation/BloopIntegrationSpec.scala b/src/test/scala/seed/generation/BloopIntegrationSpec.scala index 30e8c04..38540dc 100644 --- a/src/test/scala/seed/generation/BloopIntegrationSpec.scala +++ b/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) + } } diff --git a/src/test/scala/seed/generation/util/TestProcessHelper.scala b/src/test/scala/seed/generation/util/TestProcessHelper.scala index 09c3933..81c7dd5 100644 --- a/src/test/scala/seed/generation/util/TestProcessHelper.scala +++ b/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) } } diff --git a/test/custom-command-target-fail/build.toml b/test/custom-command-target-fail/build.toml new file mode 100644 index 0000000..2cf2089 --- /dev/null +++ b/test/custom-command-target-fail/build.toml @@ -0,0 +1,9 @@ +[project] +scalaVersion = "2.13.0" + +[module.utils.target.gen-sources] +command = "exit 1" + +[module.demo.jvm] +moduleDeps = ["utils"] +sources = ["demo"] diff --git a/test/custom-command-target-fail/demo/Main.scala b/test/custom-command-target-fail/demo/Main.scala new file mode 100644 index 0000000..d5e9979 --- /dev/null +++ b/test/custom-command-target-fail/demo/Main.scala @@ -0,0 +1,3 @@ +object Main { + def main(args: Array[String]): Unit = println("hello") +} \ No newline at end of file