Append all output on failed test. (#11156)

2 files changed, 25 insertions, 42 deletions

diff --git a/testament/categories.nim b/testament/categories.nim
index a2ce8a094..a2a0c5d5b 100644
--- a/testament/categories.nim
+++ b/testament/categories.nim
@@ -515,30 +515,21 @@ proc testNimblePackages(r: var TResults, cat: Category) =
       let buildPath = packagesDir / name
       if not existsDir(buildPath):
         if hasDep:
-          let nimbleProcess = startProcess("nimble", "", ["install", "-y", name],
-                                           options = {poUsePath, poStdErrToStdOut})
-          let nimbleStatus = waitForExitEx(nimbleProcess)
-          nimbleProcess.close
+          let (nimbleOutput, nimbleStatus) = execCmdEx2("nimble", ["install", "-y", name])
           if nimbleStatus != QuitSuccess:
-            r.addResult(test, targetC, "", "'nimble install' failed", reInstallFailed)
+            r.addResult(test, targetC, "", "'nimble install' failed\n" & nimbleOutput, reInstallFailed)
             continue
 
-        let installProcess = startProcess("git", "", ["clone", url, buildPath],
-                                          options = {poUsePath, poStdErrToStdOut})
-        let installStatus = waitForExitEx(installProcess)
-        installProcess.close
+        let (installOutput, installStatus) = execCmdEx2("git", ["clone", url, buildPath])
         if installStatus != QuitSuccess:
-          r.addResult(test, targetC, "", "'git clone' failed", reInstallFailed)
+          r.addResult(test, targetC, "", "'git clone' failed\n" & installOutput, reInstallFailed)
           continue
 
       let cmdArgs = parseCmdLine(cmd)
-      let buildProcess = startProcess(cmdArgs[0], buildPath, cmdArgs[1..^1],
-                                      options = {poUsePath, poStdErrToStdOut})
-      let buildStatus = waitForExitEx(buildProcess)
-      buildProcess.close
 
+      let (buildOutput, buildStatus) = execCmdEx2(cmdArgs[0], cmdArgs[1..^1], workingDir=buildPath)
       if buildStatus != QuitSuccess:
-        r.addResult(test, targetC, "", "package test failed", reBuildFailed)
+        r.addResult(test, targetC, "", "package test failed\n" & buildOutput, reBuildFailed)
       else:
         inc r.passed
         r.addResult(test, targetC, "", "", reSuccess)
@@ -654,8 +645,7 @@ proc runJoinedTest(r: var TResults, cat: Category, testsDir: string) =
 
   let args = ["c", "--nimCache:" & outDir, "-d:testing", "--listCmd", "megatest.nim"]
   proc onStdout(line: string) = echo line
-  var (buf, exitCode) = execCmdEx2(command = compilerPrefix, args = args, options = {poStdErrToStdOut, poUsePath}, input = "",
-    onStdout = if verboseMegatest: onStdout else: nil)
+  var (buf, exitCode) = execCmdEx2(command = compilerPrefix, args = args, input = "")
   if exitCode != 0:
     echo buf.string
     quit("megatest compilation failed")
diff --git a/testament/tester.nim b/testament/tester.nim
index d55eb7bd2..b73190eba 100644
--- a/testament/tester.nim
+++ b/testament/tester.nim
@@ -19,8 +19,6 @@ include compiler/nodejs
 var useColors = true
 var backendLogging = true
 var simulate = false
-var verboseMegatest = false # very verbose but can be useful
-var verboseCommands = false
 
 const
   testsDir = "tests" & DirSep
@@ -39,8 +37,6 @@ Arguments:
   arguments are passed to the compiler
 Options:
   --print                   also print results to the console
-  --verboseMegatest         log to stdout megatetest compilation
-  --verboseCommands         log to stdout info about commands being run
   --simulate                see what tests would be run but don't run them (for debugging)
   --failing                 only show failing/ignored tests
   --targets:"c c++ js objc" run tests for specified targets (default: all)
@@ -92,11 +88,11 @@ proc getFileDir(filename: string): string =
   if not result.isAbsolute():
     result = getCurrentDir() / result
 
-proc execCmdEx2(command: string, args: openarray[string], options: set[ProcessOption], input: string, onStdout: proc(line: string) = nil): tuple[
+proc execCmdEx2(command: string, args: openarray[string]; workingDir, input: string = ""): tuple[
                 output: TaintedString,
                 exitCode: int] {.tags:
                 [ExecIOEffect, ReadIOEffect, RootEffect], gcsafe.} =
-  var p = startProcess(command, args=args, options=options)
+  var p = startProcess(command, workingDir=workingDir, args=args, options={poStdErrToStdOut, poUsePath})
   var outp = outputStream(p)
 
   # There is no way to provide input for the child process
@@ -112,20 +108,11 @@ proc execCmdEx2(command: string, args: openarray[string], options: set[ProcessOp
     if outp.readLine(line):
       result.output.string.add(line.string)
       result.output.string.add("\n")
-      if onStdout != nil: onStdout(line.string)
     else:
       result.exitCode = peekExitCode(p)
       if result.exitCode != -1: break
   close(p)
 
-  if verboseCommands:
-    var command2 = command
-    if args.len > 0: command2.add " " & args.quoteShellCommand
-    echo (msg: "execCmdEx2",
-      command: command2,
-      options: options,
-      exitCode: result.exitCode)
-
 proc nimcacheDir(filename, options: string, target: TTarget): string =
   ## Give each test a private nimcache dir so they don't clobber each other's.
   let hashInput = options & $target
@@ -267,17 +254,25 @@ proc addResult(r: var TResults, test: TTest, target: TTarget,
     maybeStyledEcho styleBright, fgRed, "FAIL: ", fgCyan, name
     maybeStyledEcho styleBright, fgCyan, "Test \"", test.name, "\"", " in category \"", test.cat.string, "\""
     maybeStyledEcho styleBright, fgRed, "Failure: ", $success
-    maybeStyledEcho fgYellow, "Expected:"
-    maybeStyledEcho styleBright, expected, "\n"
-    maybeStyledEcho fgYellow, "Gotten:"
-    maybeStyledEcho styleBright, given, "\n"
+    if success in {reBuildFailed, reNimcCrash, reInstallFailed}:
+      # expected is empty, no reason to print it.
+      echo given
+    else:
+      maybeStyledEcho fgYellow, "Expected:"
+      maybeStyledEcho styleBright, expected, "\n"
+      maybeStyledEcho fgYellow, "Gotten:"
+      maybeStyledEcho styleBright, given, "\n"
+
 
   if backendLogging and existsEnv("APPVEYOR"):
     let (outcome, msg) =
-      if success == reSuccess:
+      case success
+      of reSuccess:
         ("Passed", "")
-      elif success in {reDisabled, reJoined}:
+      of reDisabled, reJoined:
         ("Skipped", "")
+      of reBuildFailed, reNimcCrash, reInstallFailed:
+        ("Failed", "Failure: " & $success & "\n" & given)
       else:
         ("Failed", "Failure: " & $success & "\nExpected:\n" & expected & "\n\n" & "Gotten:\n" & given)
     var p = startProcess("appveyor", args=["AddTest", test.name.replace("\\", "/") & test.options,
@@ -425,7 +420,7 @@ proc testSpec(r: var TResults, test: TTest, targets: set[TTarget] = {}) =
       # nested conditionals - the empty rows in between to clarify the "danger"
       var given = callCompiler(expected.getCmd, test.name, test.options, target)
       if given.err != reSuccess:
-        r.addResult(test, target, "", given.msg, given.err)
+        r.addResult(test, target, "", given.nimout, given.err)
         continue
       let isJsTarget = target == targetJS
       var exeFile = changeFileExt(test.name, if isJsTarget: "js" else: ExeExt)
@@ -446,7 +441,7 @@ proc testSpec(r: var TResults, test: TTest, targets: set[TTarget] = {}) =
         args = concat(@[exeFile], args)
       else:
         exeCmd = exeFile
-      var (buf, exitCode) = execCmdEx2(exeCmd, args, options = {poStdErrToStdOut}, input = expected.input)
+      var (buf, exitCode) = execCmdEx2(exeCmd, args, input = expected.input)
       # Treat all failure codes from nodejs as 1. Older versions of nodejs used
       # to return other codes, but for us it is sufficient to know that it's not 0.
       if exitCode != 0: exitCode = 1
@@ -555,8 +550,6 @@ proc main() =
   while p.kind == cmdLongoption:
     case p.key.string.normalize
     of "print", "verbose": optPrintResults = true
-    of "verbosemegatest": verboseMegatest = true
-    of "verbosecommands": verboseCommands = true
     of "failing": optFailing = true
     of "pedantic": discard "now always enabled"
     of "targets":