use NIM_TESTAMENT_BATCH for important_packages, improve formatting, code cleanups (#17301)

* use NIM_TESTAMENT_BATCH for important_packages, improve formatting, code cleanups
* workaround for nimcrypto not having `--path:.`; remove workaround "two are special snowflakes"
* add comment explaining why nimcrypto fails and what packages need to be testable

4 files changed, 176 insertions, 200 deletions

diff --git a/testament/categories.nim b/testament/categories.nim
index 3a34c51d4..acb3ee0a8 100644
--- a/testament/categories.nim
+++ b/testament/categories.nim
@@ -30,8 +30,7 @@ const
     "lib",
     "longgc",
     "manyloc",
-    "nimble-packages-1",
-    "nimble-packages-2",
+    "nimble-packages",
     "niminaction",
     "threads",
     "untestable", # see trunner_special
@@ -400,67 +399,48 @@ proc testStdlib(r: var TResults, pattern, options: string, cat: Category) =
     testSpec r, testObj
 
 # ----------------------------- nimble ----------------------------------------
-
-var nimbleDir = getEnv("NIMBLE_DIR")
-if nimbleDir.len == 0: nimbleDir = getHomeDir() / ".nimble"
-let
-  nimbleExe = findExe("nimble")
-  packageIndex = nimbleDir / "packages_official.json"
-
-type
-  PkgPart = enum
-    ppOne
-    ppTwo
-
-iterator listPackages(part: PkgPart): tuple[name, cmd, url: string, useHead: bool] =
+proc listPackages(packageFilter: string): seq[NimblePackage] =
+  # xxx document `packageFilter`, seems like a bad API (at least should be a regex; a substring match makes no sense)
+  var nimbleDir = getEnv("NIMBLE_DIR")
+  if nimbleDir.len == 0: nimbleDir = getHomeDir() / ".nimble"
+  let packageIndex = nimbleDir / "packages_official.json"
   let packageList = parseFile(packageIndex)
-  let importantList =
-    case part
-    of ppOne: important_packages.packages1
-    of ppTwo: important_packages.packages2
-  for n, cmd, url, useHead in importantList.items:
-    if url.len != 0:
-      yield (n, cmd, url, useHead)
-    else:
-      var found = false
-      for package in packageList.items:
-        let name = package["name"].str
-        if name == n:
-          found = true
-          let pUrl = package["url"].str
-          yield (name, cmd, pUrl, useHead)
-          break
-      if not found:
-        raise newException(ValueError, "Cannot find package '$#'." % n)
-
-proc makeSupTest(test, options: string, cat: Category): TTest =
+  proc findPackage(name: string): JsonNode =
+    for a in packageList:
+      if a["name"].str == name: return a
+  for pkg in important_packages.packages.items:
+    if isCurrentBatch(testamentData0, pkg.name) and packageFilter in pkg.name:
+      var pkg = pkg
+      if pkg.url.len == 0:
+        let pkg2 = findPackage(pkg.name)
+        if pkg2 == nil:
+          raise newException(ValueError, "Cannot find package '$#'." % pkg.name)
+        pkg.url = pkg2["url"].str
+      result.add pkg
+
+proc makeSupTest(test, options: string, cat: Category, debugInfo = ""): TTest =
   result.cat = cat
   result.name = test
   result.options = options
+  result.debugInfo = debugInfo
   result.startTime = epochTime()
 
 import std/private/gitutils
 
-proc testNimblePackages(r: var TResults; cat: Category; packageFilter: string, part: PkgPart) =
-  if nimbleExe == "":
-    echo "[Warning] - Cannot run nimble tests: Nimble binary not found."
-    return
-  if execCmd("$# update" % nimbleExe) == QuitFailure:
-    echo "[Warning] - Cannot run nimble tests: Nimble update failed."
-    return
-
+proc testNimblePackages(r: var TResults; cat: Category; packageFilter: string) =
+  let nimbleExe = findExe("nimble")
+  doAssert nimbleExe != "", "Cannot run nimble tests: Nimble binary not found."
+  doAssert execCmd("$# update" % nimbleExe) == 0, "Cannot run nimble tests: Nimble update failed."
   let packageFileTest = makeSupTest("PackageFileParsed", "", cat)
   let packagesDir = "pkgstemp"
   createDir(packagesDir)
   var errors = 0
   try:
-    for name, cmd, url, useHead in listPackages(part):
-      if packageFilter notin name:
-        continue
+    let pkgs = listPackages(packageFilter)
+    for i, pkg in pkgs:
       inc r.total
-      var test = makeSupTest(name, "", cat)
-      let buildPath = packagesDir / name
-
+      var test = makeSupTest(pkg.name, "", cat, "[$#/$#] " % [$i, $pkgs.len])
+      let buildPath = packagesDir / pkg.name
       template tryCommand(cmd: string, workingDir2 = buildPath, reFailed = reInstallFailed, maxRetries = 1): string =
         var outp: string
         let ok = retryCall(maxRetry = maxRetries, backoffDuration = 1.0):
@@ -473,13 +453,13 @@ proc testNimblePackages(r: var TResults; cat: Category; packageFilter: string, p
         outp
 
       if not dirExists(buildPath):
-        discard tryCommand("git clone $# $#" % [url.quoteShell, buildPath.quoteShell], workingDir2 = ".", maxRetries = 3)
-        if not useHead:
+        discard tryCommand("git clone $# $#" % [pkg.url.quoteShell, buildPath.quoteShell], workingDir2 = ".", maxRetries = 3)
+        if not pkg.useHead:
           discard tryCommand("git fetch --tags", maxRetries = 3)
           let describeOutput = tryCommand("git describe --tags --abbrev=0")
           discard tryCommand("git checkout $#" % [describeOutput.strip.quoteShell])
         discard tryCommand("nimble install --depsOnly -y", maxRetries = 3)
-      discard tryCommand(cmd, reFailed = reBuildFailed)
+      discard tryCommand(pkg.cmd, reFailed = reBuildFailed)
       inc r.passed
       r.addResult(test, targetC, "", "", reSuccess)
 
@@ -701,10 +681,8 @@ proc processCategory(r: var TResults, cat: Category,
       compileExample(r, "examples/*.nim", options, cat)
       compileExample(r, "examples/gtk/*.nim", options, cat)
       compileExample(r, "examples/talk/*.nim", options, cat)
-    of "nimble-packages-1":
-      testNimblePackages(r, cat, options, ppOne)
-    of "nimble-packages-2":
-      testNimblePackages(r, cat, options, ppTwo)
+    of "nimble-packages":
+      testNimblePackages(r, cat, options)
     of "niminaction":
       testNimInAction(r, cat, options)
     of "ic":
diff --git a/testament/important_packages.nim b/testament/important_packages.nim
index 4f8887057..a3b6db57d 100644
--- a/testament/important_packages.nim
+++ b/testament/important_packages.nim
@@ -1,5 +1,5 @@
 ##[
-## note
+## note 1
 `useHead` should ideally be used as the default but lots of packages (e.g. `chronos`)
 don't have release tags (or have really old ones compared to HEAD), making it
 impossible to test them reliably here.
@@ -7,153 +7,150 @@ impossible to test them reliably here.
 packages listed here should ideally have regularly updated release tags, so that:
 * we're testing recent versions of the package
 * the version that's tested is stable enough even if HEAD may occasionally break
-]##
 
+## note 2: D20210308T165435:here
+nimble packages should be testable as follows:
+git clone $url $dir && cd $dir
+NIMBLE_DIR=$TMP_NIMBLE_DIR XDG_CONFIG_HOME= nimble install --depsOnly -y
+NIMBLE_DIR=$TMP_NIMBLE_DIR XDG_CONFIG_HOME= nimble test
 
-#[
-xxx instead of pkg1, pkg2, use the more flexible `NIM_TESTAMENT_BATCH` (see #14823).
-]#
+if this fails (e.g. nimcrypto), it could be because a package lacks a `tests/nim.cfg` with `--path:..`,
+so the above commands would've worked by accident with `nimble install` but not with `nimble install --depsOnly`.
+When this is the case, a workaround is to test this package here by adding `--path:$srcDir` on the test `cmd`.
+]##
 
-template pkg1(name: string; cmd = "nimble test"; url = "", useHead = true): untyped =
-  packages1.add((name, cmd, url, useHead))
+type NimblePackage* = object
+  name*, cmd*, url*: string
+  useHead*: bool
 
-template pkg2(name: string; cmd = "nimble test"; url = "", useHead = true): untyped =
-  packages2.add((name, cmd, url, useHead))
+var packages*: seq[NimblePackage]
 
-var packages1*: seq[tuple[name, cmd: string; url: string, useHead: bool]] = @[]
-var packages2*: seq[tuple[name, cmd: string; url: string, useHead: bool]] = @[]
+proc pkg(name: string; cmd = "nimble test"; url = "", useHead = true) =
+  packages.add NimblePackage(name: name, cmd: cmd, url: url, useHead: useHead)
 
-# packages A-M
-# pkg1 "alea"
-pkg1 "argparse"
-pkg1 "arraymancer", "nim c tests/tests_cpu.nim"
-# pkg1 "ast_pattern_matching", "nim c -r --oldgensym:on tests/test1.nim"
-pkg1 "awk"
-pkg1 "bigints", url = "https://github.com/Araq/nim-bigints"
-pkg1 "binaryheap", "nim c -r binaryheap.nim"
-pkg1 "BipBuffer"
-# pkg1 "blscurve" # pending https://github.com/status-im/nim-blscurve/issues/39
-pkg1 "bncurve"
-pkg1 "brainfuck", "nim c -d:release -r tests/compile.nim"
-pkg1 "bump", "nim c --gc:arc --path:. -r tests/tbump.nim", "https://github.com/disruptek/bump"
-pkg1 "c2nim", "nim c testsuite/tester.nim"
-pkg1 "cascade"
-pkg1 "cello"
-pkg1 "chroma"
-pkg1 "chronicles", "nim c -o:chr -r chronicles.nim"
+# pkg "alea"
+pkg "argparse"
+pkg "arraymancer", "nim c tests/tests_cpu.nim"
+# pkg "ast_pattern_matching", "nim c -r --oldgensym:on tests/test1.nim"
+pkg "awk"
+pkg "bigints", url = "https://github.com/Araq/nim-bigints"
+pkg "binaryheap", "nim c -r binaryheap.nim"
+pkg "BipBuffer"
+# pkg "blscurve" # pending https://github.com/status-im/nim-blscurve/issues/39
+pkg "bncurve"
+pkg "brainfuck", "nim c -d:release -r tests/compile.nim"
+pkg "bump", "nim c --gc:arc --path:. -r tests/tbump.nim", "https://github.com/disruptek/bump"
+pkg "c2nim", "nim c testsuite/tester.nim"
+pkg "cascade"
+pkg "cello"
+pkg "chroma"
+pkg "chronicles", "nim c -o:chr -r chronicles.nim"
 # when not defined(osx): # testdatagram.nim(560, 54): Check failed
-#   pkg1 "chronos", "nim c -r -d:release tests/testall"
+#   pkg "chronos", "nim c -r -d:release tests/testall"
   # pending https://github.com/nim-lang/Nim/issues/17130
-pkg1 "cligen", "nim c --path:. -r cligen.nim"
-pkg1 "combparser", "nimble test --gc:orc"
-pkg1 "compactdict"
-pkg1 "comprehension", "nimble test", "https://github.com/alehander42/comprehension"
-# pkg1 "criterion" # pending https://github.com/disruptek/criterion/issues/3 (wrongly closed)
-pkg1 "dashing", "nim c tests/functional.nim"
-pkg1 "delaunay"
-pkg1 "docopt"
-pkg1 "easygl", "nim c -o:egl -r src/easygl.nim", "https://github.com/jackmott/easygl"
-pkg1 "elvis"
-# pkg1 "fidget" # pending https://github.com/treeform/fidget/issues/133
-pkg1 "fragments", "nim c -r fragments/dsl.nim"
-pkg1 "fusion"
-pkg1 "gara"
-pkg1 "glob"
-pkg1 "ggplotnim", "nim c -d:noCairo -r tests/tests.nim"
-# pkg1 "gittyup", "nimble test", "https://github.com/disruptek/gittyup"
-pkg1 "gnuplot", "nim c gnuplot.nim"
-# pkg1 "gram", "nim c -r --gc:arc --define:danger tests/test.nim", "https://github.com/disruptek/gram"
+pkg "cligen", "nim c --path:. -r cligen.nim"
+pkg "combparser", "nimble test --gc:orc"
+pkg "compactdict"
+pkg "comprehension", "nimble test", "https://github.com/alehander42/comprehension"
+# pkg "criterion" # pending https://github.com/disruptek/criterion/issues/3 (wrongly closed)
+pkg "dashing", "nim c tests/functional.nim"
+pkg "delaunay"
+pkg "docopt"
+pkg "easygl", "nim c -o:egl -r src/easygl.nim", "https://github.com/jackmott/easygl"
+pkg "elvis"
+# pkg "fidget" # pending https://github.com/treeform/fidget/issues/133
+pkg "fragments", "nim c -r fragments/dsl.nim"
+pkg "fusion"
+pkg "gara"
+pkg "glob"
+pkg "ggplotnim", "nim c -d:noCairo -r tests/tests.nim"
+# pkg "gittyup", "nimble test", "https://github.com/disruptek/gittyup"
+pkg "gnuplot", "nim c gnuplot.nim"
+# pkg "gram", "nim c -r --gc:arc --define:danger tests/test.nim", "https://github.com/disruptek/gram"
   # pending https://github.com/nim-lang/Nim/issues/16509
-pkg1 "hts", "nim c -o:htss src/hts.nim"
-# pkg1 "httpauth"
-pkg1 "illwill", "nimble examples"
-pkg1 "inim"
-pkg1 "itertools", "nim doc src/itertools.nim"
-pkg1 "iterutils"
-pkg1 "jstin"
-pkg1 "karax", "nim c -r tests/tester.nim"
-pkg1 "kdtree", "nimble test", "https://github.com/jblindsay/kdtree"
-pkg1 "loopfusion"
-pkg1 "macroutils"
-pkg1 "manu"
-pkg1 "markdown"
-pkg1 "memo"
-pkg1 "msgpack4nim", "nim c -r tests/test_spec.nim"
-
-# these two are special snowflakes
-pkg1 "nimcrypto", "nim c -r tests/testall.nim"
-pkg1 "stint", "nim c -o:stintt -r stint.nim"
-
-
-# packages N-Z
-pkg2 "nake", "nim c nakefile.nim"
-pkg2 "neo", "nim c -d:blas=openblas tests/all.nim"
-# pkg2 "nesm", "nimble tests" # notice plural 'tests'
-# pkg2 "nico"
-pkg2 "nicy", "nim c -r src/nicy.nim"
-pkg2 "nigui", "nim c -o:niguii -r src/nigui.nim"
-pkg2 "NimData", "nim c -o:nimdataa src/nimdata.nim"
-pkg2 "nimes", "nim c src/nimes.nim"
-pkg2 "nimfp", "nim c -o:nfp -r src/fp.nim"
-when false:
-  pkg2 "nimgame2", "nim c nimgame2/nimgame.nim"
-  # XXX Doesn't work with deprecated 'randomize', will create a PR.
-pkg2 "nimgen", "nim c -o:nimgenn -r src/nimgen/runcfg.nim"
-pkg2 "nimlsp"
-pkg2 "nimly", "nim c -r tests/test_readme_example.nim"
-# pkg2 "nimongo", "nimble test_ci"
-# pkg2 "nimph", "nimble test", "https://github.com/disruptek/nimph"
-pkg2 "nimpy", "nim c -r tests/nimfrompy.nim"
-pkg2 "nimquery"
-pkg2 "nimsl"
-pkg2 "nimsvg"
-pkg2 "nimterop", "nimble minitest"
-pkg2 "nimwc", "nim c nimwc.nim"
-# pkg2 "nimx", "nim c --threads:on test/main.nim"
-# pkg2 "nitter", "nim c src/nitter.nim", "https://github.com/zedeus/nitter"
-pkg2 "norm", "nim c -r tests/sqlite/trows.nim"
-pkg2 "npeg", "nimble testarc"
-pkg2 "numericalnim", "nim c -r tests/test_integrate.nim"
-pkg2 "optionsutils"
-pkg2 "ormin", "nim c -o:orminn ormin.nim"
-pkg2 "parsetoml"
-pkg2 "patty"
-pkg2 "pixie", useHead = false
-pkg2 "plotly", "nim c examples/all.nim"
-pkg2 "pnm"
-pkg2 "polypbren"
-pkg2 "prologue", "nimble tcompile"
-pkg2 "protobuf", "nim c -o:protobuff -r src/protobuf.nim"
-pkg2 "pylib"
-pkg2 "rbtree"
-pkg2 "react", "nimble example"
-pkg2 "regex", "nim c src/regex"
-pkg2 "result", "nim c -r result.nim"
-pkg2 "RollingHash", "nim c -r tests/test_cyclichash.nim"
-pkg2 "rosencrantz", "nim c -o:rsncntz -r rosencrantz.nim"
-pkg2 "sdl1", "nim c -r src/sdl.nim"
-pkg2 "sdl2_nim", "nim c -r sdl2/sdl.nim"
-pkg2 "sigv4", "nim c --gc:arc -r sigv4.nim", "https://github.com/disruptek/sigv4"
-pkg2 "snip", "nimble test", "https://github.com/genotrance/snip"
-pkg2 "strslice"
-pkg2 "strunicode", "nim c -r src/strunicode.nim"
-pkg2 "synthesis"
-pkg2 "telebot", "nim c -o:tbot -r src/telebot.nim"
-pkg2 "tempdir"
-pkg2 "templates"
-pkg2 "tensordsl", "nim c -r tests/tests.nim", "https://krux02@bitbucket.org/krux02/tensordslnim.git"
-pkg2 "terminaltables", "nim c src/terminaltables.nim"
-pkg2 "termstyle", "nim c -r termstyle.nim"
-pkg2 "timeit"
-pkg2 "timezones"
-pkg2 "tiny_sqlite"
-pkg2 "unicodedb", "nim c -d:release -r tests/tests.nim"
-pkg2 "unicodeplus", "nim c -d:release -r tests/tests.nim"
-pkg2 "unpack"
-pkg2 "websocket", "nim c websocket.nim"
-# pkg2 "winim"
-pkg2 "with"
-pkg2 "ws"
-pkg2 "yaml", "nim build"
-pkg2 "zero_functional", "nim c -r -d:nimWorkaround14447 test.nim"
-pkg2 "zippy"
+pkg "hts", "nim c -o:htss src/hts.nim"
+# pkg "httpauth"
+pkg "illwill", "nimble examples"
+pkg "inim"
+pkg "itertools", "nim doc src/itertools.nim"
+pkg "iterutils"
+pkg "jstin"
+pkg "karax", "nim c -r tests/tester.nim"
+pkg "kdtree", "nimble test", "https://github.com/jblindsay/kdtree"
+pkg "loopfusion"
+pkg "macroutils"
+pkg "manu"
+pkg "markdown"
+pkg "memo"
+pkg "msgpack4nim", "nim c -r tests/test_spec.nim"
+pkg "nake", "nim c nakefile.nim"
+pkg "neo", "nim c -d:blas=openblas tests/all.nim"
+# pkg "nesm", "nimble tests" # notice plural 'tests'
+# pkg "nico"
+pkg "nicy", "nim c -r src/nicy.nim"
+pkg "nigui", "nim c -o:niguii -r src/nigui.nim"
+pkg "nimcrypto", "nim r --path:. tests/testall.nim" # `--path:.` workaround needed, see D20210308T165435
+pkg "NimData", "nim c -o:nimdataa src/nimdata.nim"
+pkg "nimes", "nim c src/nimes.nim"
+pkg "nimfp", "nim c -o:nfp -r src/fp.nim"
+# pkg "nimgame2", "nim c nimgame2/nimgame.nim" # XXX Doesn't work with deprecated 'randomize', will create a PR.
+pkg "nimgen", "nim c -o:nimgenn -r src/nimgen/runcfg.nim"
+pkg "nimlsp"
+pkg "nimly", "nim c -r tests/test_readme_example.nim"
+# pkg "nimongo", "nimble test_ci"
+# pkg "nimph", "nimble test", "https://github.com/disruptek/nimph"
+pkg "nimpy", "nim c -r tests/nimfrompy.nim"
+pkg "nimquery"
+pkg "nimsl"
+pkg "nimsvg"
+pkg "nimterop", "nimble minitest"
+pkg "nimwc", "nim c nimwc.nim"
+# pkg "nimx", "nim c --threads:on test/main.nim"
+# pkg "nitter", "nim c src/nitter.nim", "https://github.com/zedeus/nitter"
+pkg "norm", "nim c -r tests/sqlite/trows.nim"
+pkg "npeg", "nimble testarc"
+pkg "numericalnim", "nim c -r tests/test_integrate.nim"
+pkg "optionsutils"
+pkg "ormin", "nim c -o:orminn ormin.nim"
+pkg "parsetoml"
+pkg "patty"
+pkg "pixie", useHead = false
+pkg "plotly", "nim c examples/all.nim"
+pkg "pnm"
+pkg "polypbren"
+pkg "prologue", "nimble tcompile"
+pkg "protobuf", "nim c -o:protobuff -r src/protobuf.nim"
+pkg "pylib"
+pkg "rbtree"
+pkg "react", "nimble example"
+pkg "regex", "nim c src/regex"
+pkg "result", "nim c -r result.nim"
+pkg "RollingHash", "nim c -r tests/test_cyclichash.nim"
+pkg "rosencrantz", "nim c -o:rsncntz -r rosencrantz.nim"
+pkg "sdl1", "nim c -r src/sdl.nim"
+pkg "sdl2_nim", "nim c -r sdl2/sdl.nim"
+pkg "sigv4", "nim c --gc:arc -r sigv4.nim", "https://github.com/disruptek/sigv4"
+pkg "snip", "nimble test", "https://github.com/genotrance/snip"
+pkg "stint", "nim r stint.nim"
+pkg "strslice"
+pkg "strunicode", "nim c -r src/strunicode.nim"
+pkg "synthesis"
+pkg "telebot", "nim c -o:tbot -r src/telebot.nim"
+pkg "tempdir"
+pkg "templates"
+pkg "tensordsl", "nim c -r tests/tests.nim", "https://krux02@bitbucket.org/krux02/tensordslnim.git"
+pkg "terminaltables", "nim c src/terminaltables.nim"
+pkg "termstyle", "nim c -r termstyle.nim"
+pkg "timeit"
+pkg "timezones"
+pkg "tiny_sqlite"
+pkg "unicodedb", "nim c -d:release -r tests/tests.nim"
+pkg "unicodeplus", "nim c -d:release -r tests/tests.nim"
+pkg "unpack"
+pkg "websocket", "nim c websocket.nim"
+# pkg "winim"
+pkg "with"
+pkg "ws"
+pkg "yaml", "nim build"
+pkg "zero_functional", "nim c -r -d:nimWorkaround14447 test.nim"
+pkg "zippy"
diff --git a/testament/specs.nim b/testament/specs.nim
index 8b90f8c91..6b80fe41d 100644
--- a/testament/specs.nim
+++ b/testament/specs.nim
@@ -230,7 +230,7 @@ proc addLine*(self: var string; a, b: string) =
 proc initSpec*(filename: string): TSpec =
   result.file = filename
 
-proc isCurrentBatch(testamentData: TestamentData; filename: string): bool =
+proc isCurrentBatch*(testamentData: TestamentData; filename: string): bool =
   if testamentData.testamentNumBatch != 0:
     hash(filename) mod testamentData.testamentNumBatch == testamentData.testamentBatch
   else:
diff --git a/testament/testament.nim b/testament/testament.nim
index 1307c19ef..d1f34cd98 100644
--- a/testament/testament.nim
+++ b/testament/testament.nim
@@ -76,6 +76,7 @@ type
     args: seq[string]
     spec: TSpec
     startTime: float
+    debugInfo: string
 
 # ----------------------------------------------------------------------------
 
@@ -285,7 +286,7 @@ proc addResult(r: var TResults, test: TTest, target: TTarget,
   template disp(msg) =
     maybeStyledEcho styleDim, fgYellow, msg & ' ', styleBright, fgCyan, name
   if success == reSuccess:
-    maybeStyledEcho fgGreen, "PASS: ", fgCyan, alignLeft(name, 60), fgBlue, " (", durationStr, " sec)"
+    maybeStyledEcho fgGreen, "PASS: ", fgCyan, test.debugInfo, alignLeft(name, 60), fgBlue, " (", durationStr, " sec)"
   elif success == reDisabled:
     if test.spec.inCurrentBatch: disp("SKIP:")
     else: disp("NOTINBATCH:")