testament: generic N-fold batching: windows CI 37mn=>16m (#14823)

* testament: run CI faster thanks to batching
* move ta_in, tstdin into existing tosproc
* move ta_out,tafalse,texitcode,tstderr into existing tosproc
* joinable osproc
* move tstdout into existing tosproc
* spec: batchable; fix tests
* fixup

azure-pipelines.yml

@@ -27,9 +27,19 @@ jobs:
         vmImage: 'macOS-10.15'
         CPU: amd64
         NIM_COMPILE_TO_CPP: true
-      Windows_amd64:
+      Windows_amd64_batch0_3:
         vmImage: 'windows-2019'
         CPU: amd64
+        # see also: `NIM_TEST_PACKAGES`
+        NIM_TESTAMENT_BATCH: "0_3"
+      Windows_amd64_batch1_3:
+        vmImage: 'windows-2019'
+        CPU: amd64
+        NIM_TESTAMENT_BATCH: "1_3"
+      Windows_amd64_batch2_3:
+        vmImage: 'windows-2019'
+        CPU: amd64
+        NIM_TESTAMENT_BATCH: "2_3"
 
   pool:
     vmImage: $(vmImage)

koch.nim

@@ -548,7 +548,12 @@ proc runCI(cmd: string) =
       execFold("Compile tester", "nim c -d:nimCoroutines --os:genode -d:posix --compileOnly testament/testament")
 
     # main bottleneck here
-    execFold("Run tester", "nim c -r -d:nimCoroutines testament/testament --pedantic all -d:nimCoroutines")
+    # xxx: even though this is the main bottlneck, we could use same code to batch the other tests
+    #[
+    BUG: with initOptParser, `--batch:'' all` interprets `all` as the argument of --batch
+    ]#
+    execFold("Run tester", "nim c -r -d:nimCoroutines testament/testament --pedantic --batch:$1 all -d:nimCoroutines" % ["NIM_TESTAMENT_BATCH".getEnv("_")])
+
     block CT_FFI:
       when defined(posix): # windows can be handled in future PR's
         execFold("nimble install -y libffi", "nimble install -y libffi")

lib/nimhcr.nim

@@ -1,3 +1,7 @@
+discard """
+batchable: false
+"""
+
 #
 #
 #            Nim's Runtime Library

lib/nimrtl.nim

@@ -1,3 +1,7 @@
+discard """
+batchable: false
+"""
+
 #
 #
 #            Nim's Runtime Library

testament/categories.nim

@@ -679,8 +679,9 @@ proc runJoinedTest(r: var TResults, cat: Category, testsDir: string) =
     quit 1
   else:
     echo "megatest output OK"
-    removeFile(outputGottenFile)
+    when false: # no point removing those, always good for debugging
-    removeFile(megatestFile)
+      removeFile(outputGottenFile)
+      removeFile(megatestFile) # keep it around
   #testSpec r, makeTest("megatest", options, cat)
 
 # ---------------------------------------------------------------------------

testament/specs.nim

@@ -8,6 +8,15 @@
 #
 
 import sequtils, parseutils, strutils, os, streams, parsecfg
+from hashes import hash
+
+type TestamentData* = ref object
+  # better to group globals under 1 object; could group the other ones here too
+  batchArg*: string
+  testamentNumBatch*: int
+  testamentBatch*: int
+
+let testamentData0* = TestamentData()
 
 var compilerPrefix* = findExe("nim")
 
@@ -68,11 +77,13 @@ type
     ccodeCheck*: string
     maxCodeSize*: int
     err*: TResultEnum
+    inCurrentBatch*: bool
     targets*: set[TTarget]
     matrix*: seq[string]
     nimout*: string
     parseErrors*: string # when the spec definition is invalid, this is not empty.
     unjoinable*: bool
+    unbatchable*: bool
     useValgrind*: bool
     timeout*: float # in seconds, fractions possible,
                     # but don't rely on much precision
@@ -138,6 +149,12 @@ proc addLine*(self: var string; a,b: string) =
 proc initSpec*(filename: string): TSpec =
   result.file = filename
 
+proc isCurrentBatch(testamentData: TestamentData, filename: string): bool =
+  if testamentData.testamentNumBatch != 0:
+    hash(filename) mod testamentData.testamentNumBatch == testamentData.testamentBatch
+  else:
+    true
+
 proc parseSpec*(filename: string): TSpec =
   result.file = filename
   let specStr = extractSpec(filename)
@@ -203,6 +220,8 @@ proc parseSpec*(filename: string): TSpec =
         result.action = actionReject
       of "nimout":
         result.nimout = e.value
+      of "batchable":
+        result.unbatchable = not parseCfgBool(e.value)
       of "joinable":
         result.unjoinable = not parseCfgBool(e.value)
       of "valgrind":
@@ -297,3 +316,7 @@ proc parseSpec*(filename: string): TSpec =
 
   if skips.anyIt(it in result.file):
     result.err = reDisabled
+
+  result.inCurrentBatch = isCurrentBatch(testamentData0, filename) or result.unbatchable
+  if not result.inCurrentBatch:
+    result.err = reDisabled

testament/testament.nim

@@ -273,12 +273,14 @@ proc addResult(r: var TResults, test: TTest, target: TTarget,
                             expected = expected,
                             given = given)
   r.data.addf("$#\t$#\t$#\t$#", name, expected, given, $success)
+  template disp(msg) =
+    maybeStyledEcho styleDim, fgYellow, msg & " ", styleBright, fgCyan, name
   if success == reSuccess:
     maybeStyledEcho fgGreen, "PASS: ", fgCyan, alignLeft(name, 60), fgBlue, " (", durationStr, " sec)"
   elif success == reDisabled:
-    maybeStyledEcho styleDim, fgYellow, "SKIP: ", styleBright, fgCyan, name
+    if test.spec.inCurrentBatch: disp("SKIP:")
-  elif success == reJoined:
+    else: disp("NOTINBATCH:")
-    maybeStyledEcho styleDim, fgYellow, "JOINED: ", styleBright, fgCyan, name
+  elif success == reJoined: disp("JOINED:")
   else:
     maybeStyledEcho styleBright, fgRed, failString, fgCyan, name
     maybeStyledEcho styleBright, fgCyan, "Test \"", test.name, "\"", " in category \"", test.cat.string, "\""
@@ -645,6 +647,15 @@ proc main() =
         useColors = false
       else:
         quit Usage
+    of "batch":
+      testamentData0.batchArg = p.val
+      if p.val != "_":
+        let s = p.val.split("_")
+        doAssert s.len == 2, $(p.val, s)
+        testamentData0.testamentBatch = s[0].parseInt
+        testamentData0.testamentNumBatch = s[1].parseInt
+        doAssert testamentData0.testamentNumBatch > 0
+        doAssert testamentData0.testamentBatch >= 0 and testamentData0.testamentBatch < testamentData0.testamentNumBatch
     of "simulate":
       simulate = true
     of "megatest":
@@ -682,6 +693,7 @@ proc main() =
       myself &= " " & quoteShell("--targets:" & targetsStr)
 
     myself &= " " & quoteShell("--nim:" & compilerPrefix)
+    myself &= " --batch:" & testamentData0.batchArg
 
     if skipFrom.len > 0:
       myself &= " " & quoteShell("--skipFrom:" & skipFrom)

tests/osproc/ta_in.nim

@@ -1,9 +0,0 @@
-discard """
-action: compile
-"""
-
-# This file is prefixed with an "a", because other tests
-# depend on it and it must be compiled first.
-import strutils
-let x = stdin.readLine()
-echo x.parseInt + 5

tests/osproc/ta_out.nim

@@ -1,33 +0,0 @@
-discard """
-output: '''
-start ta_out
-to stdout
-to stdout
-to stderr
-to stderr
-to stdout
-to stdout
-end ta_out
-'''
-"""
-
-echo "start ta_out"
-
-# This file is prefixed with an "a", because other tests
-# depend on it and it must be compiled first.
-stdout.writeLine("to stdout")
-stdout.flushFile()
-stdout.writeLine("to stdout")
-stdout.flushFile()
-
-stderr.writeLine("to stderr")
-stderr.flushFile()
-stderr.writeLine("to stderr")
-stderr.flushFile()
-
-stdout.writeLine("to stdout")
-stdout.flushFile()
-stdout.writeLine("to stdout")
-stdout.flushFile()
-
-echo "end ta_out"

tests/osproc/tafalse.nim

@@ -1,7 +0,0 @@
-discard """
-exitcode: 1
-"""
-
-# 'tafalse.nim' to ensure it is compiled before texitcode.nim
-import system
-quit(QuitFailure)

tests/osproc/texecps.nim

@@ -1,3 +1,7 @@
+discard """
+joinable: false
+"""
+
 import osproc, streams, strutils, os
 
 const NumberOfProcesses = 13

tests/osproc/texitcode.nim

@@ -1,26 +0,0 @@
-discard """
-  output: ""
-"""
-
-import osproc, os
-
-const filename = when defined(Windows): "tafalse.exe" else: "tafalse"
-let dir = getCurrentDir() / "tests" / "osproc"
-doAssert fileExists(dir / filename)
-
-var p = startProcess(filename, dir)
-doAssert(waitForExit(p) == QuitFailure)
-
-p = startProcess(filename, dir)
-var running = true
-while running:
-  running = running(p)
-doAssert(waitForExit(p) == QuitFailure)
-
-# make sure that first call to running() after process exit returns false
-p = startProcess(filename, dir)
-for j in 0..<30: # refs #13449
-  os.sleep(50)
-  if not running(p): break
-doAssert(not running(p))
-doAssert(waitForExit(p) == QuitFailure) # avoid zombies

tests/osproc/tstderr.nim

@@ -1,37 +0,0 @@
-discard """
-  output: '''
-start tstderr
---------------------------------------
-to stderr
-to stderr
---------------------------------------
-'''
-"""
-
-echo "start tstderr"
-
-import osproc, os, streams
-
-const filename = "ta_out".addFileExt(ExeExt)
-
-doAssert fileExists(getCurrentDir() / "tests" / "osproc" / filename)
-
-var p = startProcess(filename, getCurrentDir() / "tests" / "osproc",
-                     options={})
-
-try:
-  let stdoutStream = p.outputStream
-  let stderrStream = p.errorStream
-  var x = newStringOfCap(120)
-  var output = ""
-  while stderrStream.readLine(x.TaintedString):
-    output.add(x & "\n")
-
-  echo "--------------------------------------"
-  stdout.flushFile()
-  stderr.write output
-  stderr.flushFile()
-  echo "--------------------------------------"
-  stdout.flushFile()
-finally:
-  p.close()

tests/osproc/tstdin.nim

@@ -1,17 +0,0 @@
-discard """
-  output: "10"
-"""
-import osproc, os, streams
-
-const filename = when defined(Windows): "ta_in.exe" else: "ta_in"
-
-doAssert fileExists(getCurrentDir() / "tests" / "osproc" / filename)
-
-var p = startProcess(filename, getCurrentDir() / "tests" / "osproc")
-p.inputStream.write("5\n")
-p.inputStream.flush()
-
-var line = ""
-
-while p.outputStream.readLine(line.TaintedString):
-  echo line

tests/osproc/tstdout.nim

@@ -1,31 +0,0 @@
-discard """
-  output: '''--------------------------------------
-start ta_out
-to stdout
-to stdout
-to stderr
-to stderr
-to stdout
-to stdout
-end ta_out
---------------------------------------
-'''
-"""
-import osproc, os, streams
-
-const filename = when defined(Windows): "ta_out.exe" else: "ta_out"
-
-doAssert fileExists(getCurrentDir() / "tests" / "osproc" / filename)
-
-var p = startProcess(filename, getCurrentDir() / "tests" / "osproc",
-                     options={poStdErrToStdOut})
-
-let outputStream = p.outputStream
-var x = newStringOfCap(120)
-var output = ""
-while outputStream.readLine(x.TaintedString):
-  output.add(x & "\n")
-
-echo "--------------------------------------"
-stdout.write output
-echo "--------------------------------------"

tests/stdlib/tosproc.nim

@@ -5,9 +5,10 @@ joinable: false
 #[
 joinable: false
 because it'd need cleanup up stdout
-]#
 
-import stdtest/[specialpaths, unittest_light]
+see also: tests/osproc/*.nim; consider merging those into a single test here
+(easier to factor and test more things as a single self contained test)
+]#
 
 when defined(case_testfile): # compiled test file for child process
   from posix import exitnow
@@ -51,22 +52,58 @@ when defined(case_testfile): # compiled test file for child process
       echo args[1]
   main()
 
-else:
+elif defined(case_testfile2):
+  import strutils
+  let x = stdin.readLine()
+  echo x.parseInt + 5
 
+elif defined(case_testfile3):
+  echo "start ta_out"
+  stdout.writeLine("to stdout")
+  stdout.flushFile()
+  stdout.writeLine("to stdout")
+  stdout.flushFile()
+
+  stderr.writeLine("to stderr")
+  stderr.flushFile()
+  stderr.writeLine("to stderr")
+  stderr.flushFile()
+
+  stdout.writeLine("to stdout")
+  stdout.flushFile()
+  stdout.writeLine("to stdout")
+  stdout.flushFile()
+  echo "end ta_out"
+
+elif defined(case_testfile4):
+  import system # we could remove that
+  quit(QuitFailure)
+
+else: # main driver
+  import stdtest/[specialpaths, unittest_light]
   import os, osproc, strutils
   const nim = getCurrentCompilerExe()
+  const sourcePath = currentSourcePath()
+  let dir = getCurrentDir() / "tests" / "osproc"
+
+  template deferScoped(cleanup, body) =
+    # pending https://github.com/nim-lang/RFCs/issues/236#issuecomment-646855314
+    # xxx move to std/sugar or (preferably) some low level module
+    try: body
+    finally: cleanup
+
+  # we're testing `execShellCmd` so don't rely on it to compile test file
+  # note: this should be exported in posix.nim
+  proc c_system(cmd: cstring): cint {.importc: "system", header: "<stdlib.h>".}
+
+  proc compileNimProg(opt: string, name: string): string =
+    result = buildDir / name.addFileExt(ExeExt)
+    let cmd = "$# c -o:$# --hints:off $# $#" % [nim.quoteShell, result.quoteShell, opt, sourcePath.quoteShell]
+    doAssert c_system(cmd) == 0, $cmd
+    doAssert result.fileExists
 
   block execShellCmdTest:
-    ## first, compile child program
+    let output = compileNimProg("-d:release -d:case_testfile", "D20190111T024543")
-    const sourcePath = currentSourcePath()
-    let output = buildDir / "D20190111T024543".addFileExt(ExeExt)
-    let cmd = "$# c -o:$# -d:release -d:case_testfile $#" % [nim, output,
-        sourcePath]
-    # we're testing `execShellCmd` so don't rely on it to compile test file
-    # note: this should be exported in posix.nim
-    proc c_system(cmd: cstring): cint {.importc: "system",
-      header: "<stdlib.h>".}
-    assertEquals c_system(cmd), 0
 
     ## use it
     template runTest(arg: string, expected: int) =
@@ -79,7 +116,7 @@ else:
     runTest("quit_139", 139)
 
   block execProcessTest:
-    let dir = parentDir(currentSourcePath())
+    let dir = sourcePath.parentDir
     let (_, err) = execCmdEx(nim & " c " & quoteShell(dir / "osproctest.nim"))
     doAssert err == 0
     let exePath = dir / addFileExt("osproctest", ExeExt)
@@ -101,6 +138,7 @@ else:
       discard
 
   import std/streams
+
   block: # test for startProcess (more tests needed)
     # bugfix: windows stdin.close was a noop and led to blocking reads
     proc startProcessTest(command: string, options: set[ProcessOption] = {
@@ -126,6 +164,56 @@ else:
     var result = startProcessTest("nim r --hints:off -", options = {}, input = "echo 3*4")
     doAssert result == ("12\n", 0)
 
+  block: # startProcess stdin (replaces old test `tstdin` + `ta_in`)
+    let output = compileNimProg("-d:case_testfile2", "D20200626T215919")
+    var p = startProcess(output, dir) # dir not needed though
+    p.inputStream.write("5\n")
+    p.inputStream.flush()
+    var line = ""
+    var s: seq[string]
+    while p.outputStream.readLine(line.TaintedString):
+      s.add line
+    doAssert s == @["10"]
+
+  block:
+    let output = compileNimProg("-d:case_testfile3", "D20200626T221233")
+    var x = newStringOfCap(120)
+    block: # startProcess stdout poStdErrToStdOut (replaces old test `tstdout` + `ta_out`)
+      var p = startProcess(output, dir, options={poStdErrToStdOut})
+      deferScoped: p.close()
+      do:
+        var sout: seq[string]
+        while p.outputStream.readLine(x.TaintedString): sout.add x
+        doAssert sout == @["start ta_out", "to stdout", "to stdout", "to stderr", "to stderr", "to stdout", "to stdout", "end ta_out"]
+    block: # startProcess stderr (replaces old test `tstderr` + `ta_out`)
+      var p = startProcess(output, dir, options={})
+      deferScoped: p.close()
+      do:
+        var serr, sout: seq[string]
+        while p.errorStream.readLine(x.TaintedString): serr.add x
+        while p.outputStream.readLine(x.TaintedString): sout.add x
+        doAssert serr == @["to stderr", "to stderr"]
+        doAssert sout == @["start ta_out", "to stdout", "to stdout", "to stdout", "to stdout", "end ta_out"]
+
+  block: # startProcess exit code (replaces old test `texitcode` + `tafalse`)
+    let output = compileNimProg("-d:case_testfile4", "D20200626T224758")
+    var p = startProcess(output, dir)
+    doAssert waitForExit(p) == QuitFailure
+    p = startProcess(output, dir)
+    var running = true
+    while running:
+      # xxx: avoid busyloop?
+      running = running(p)
+    doAssert waitForExit(p) == QuitFailure
+
+    # make sure that first call to running() after process exit returns false
+    p = startProcess(output, dir)
+    for j in 0..<30: # refs #13449
+      os.sleep(50)
+      if not running(p): break
+    doAssert not running(p)
+    doAssert waitForExit(p) == QuitFailure # avoid zombies
+
   import std/strtabs
   block execProcessTest:
     var result = execCmdEx("nim r --hints:off -", options = {}, input = "echo 3*4")