testament: include extra options in test name (#19801)

there's currently no (simple) way to disambiguate which option failed

2 files changed, 44 insertions, 47 deletions

diff --git a/testament/categories.nim b/testament/categories.nim
index 46cc3b210..a092fec84 100644
--- a/testament/categories.nim
+++ b/testament/categories.nim
@@ -437,7 +437,7 @@ proc testNimblePackages(r: var TResults; cat: Category; packageFilter: string) =
           if pkg.allowFailure:
             inc r.passed
             inc r.failedButAllowed
-          addResult(r, test, targetC, "", cmd & "\n" & outp, reFailed, allowFailure = pkg.allowFailure)
+          addResult(r, test, targetC, "", "", cmd & "\n" & outp, reFailed, allowFailure = pkg.allowFailure)
           continue
         outp
 
@@ -450,21 +450,21 @@ proc testNimblePackages(r: var TResults; cat: Category; packageFilter: string) =
         discard tryCommand("nimble install --depsOnly -y", maxRetries = 3)
       discard tryCommand(pkg.cmd, reFailed = reBuildFailed)
       inc r.passed
-      r.addResult(test, targetC, "", "", reSuccess, allowFailure = pkg.allowFailure)
+      r.addResult(test, targetC, "", "", "", reSuccess, allowFailure = pkg.allowFailure)
 
     errors = r.total - r.passed
     if errors == 0:
-      r.addResult(packageFileTest, targetC, "", "", reSuccess)
+      r.addResult(packageFileTest, targetC, "", "", "", reSuccess)
     else:
-      r.addResult(packageFileTest, targetC, "", "", reBuildFailed)
+      r.addResult(packageFileTest, targetC, "", "", "", reBuildFailed)
 
   except JsonParsingError:
     errors = 1
-    r.addResult(packageFileTest, targetC, "", "Invalid package file", reBuildFailed)
+    r.addResult(packageFileTest, targetC, "", "", "Invalid package file", reBuildFailed)
     raise
   except ValueError:
     errors = 1
-    r.addResult(packageFileTest, targetC, "", "Unknown package", reBuildFailed)
+    r.addResult(packageFileTest, targetC, "", "", "Unknown package", reBuildFailed)
     raise # bug #18805
   finally:
     if errors == 0: removeDir(packagesDir)
diff --git a/testament/testament.nim b/testament/testament.nim
index 4b5a8a147..a4329b982 100644
--- a/testament/testament.nim
+++ b/testament/testament.nim
@@ -259,7 +259,8 @@ Tests skipped: $4 / $1 <br />
 """ % [$x.total, $x.passed, $x.failedButAllowed, $x.skipped]
 
 proc addResult(r: var TResults, test: TTest, target: TTarget,
-               expected, given: string, successOrig: TResultEnum, allowFailure = false, givenSpec: ptr TSpec = nil) =
+               extraOptions, expected, given: string, successOrig: TResultEnum,
+               allowFailure = false, givenSpec: ptr TSpec = nil) =
   # instead of `ptr TSpec` we could also use `Option[TSpec]`; passing `givenSpec` makes it easier to get what we need
   # instead of having to pass individual fields, or abusing existing ones like expected vs given.
   # test.name is easier to find than test.name.extractFilename
@@ -269,6 +270,7 @@ proc addResult(r: var TResults, test: TTest, target: TTarget,
   if allowFailure:
     name.add " (allowed to fail) "
   if test.options.len > 0: name.add ' ' & test.options
+  if extraOptions.len > 0: name.add ' ' & extraOptions
 
   let duration = epochTime() - test.startTime
   let success = if test.spec.timeout > 0.0 and duration > test.spec.timeout: reTimeout
@@ -333,7 +335,8 @@ proc addResult(r: var TResults, test: TTest, target: TTarget,
       discard waitForExit(p)
       close(p)
 
-proc checkForInlineErrors(r: var TResults, expected, given: TSpec, test: TTest, target: TTarget) =
+proc checkForInlineErrors(r: var TResults, expected, given: TSpec, test: TTest,
+                          target: TTarget, extraOptions: string) =
   let pegLine = peg"{[^(]*} '(' {\d+} ', ' {\d+} ') ' {[^:]*} ':' \s* {.*}"
   var covered = initIntSet()
   for line in splitLines(given.nimout):
@@ -367,10 +370,10 @@ proc checkForInlineErrors(r: var TResults, expected, given: TSpec, test: TTest,
         e.add ": "
         e.add expected.inlineErrors[j].msg
 
-        r.addResult(test, target, e, given.nimout, reMsgsDiffer)
+        r.addResult(test, target, extraOptions, e, given.nimout, reMsgsDiffer)
         break coverCheck
 
-    r.addResult(test, target, "", given.msg, reSuccess)
+    r.addResult(test, target, extraOptions, "", given.msg, reSuccess)
     inc(r.passed)
 
 proc nimoutCheck(expected, given: TSpec): bool =
@@ -381,22 +384,23 @@ proc nimoutCheck(expected, given: TSpec): bool =
   elif expected.nimout.len > 0 and not greedyOrderedSubsetLines(expected.nimout, given.nimout):
     result = false
 
-proc cmpMsgs(r: var TResults, expected, given: TSpec, test: TTest, target: TTarget) =
+proc cmpMsgs(r: var TResults, expected, given: TSpec, test: TTest,
+             target: TTarget, extraOptions: string) =
   if expected.inlineErrors.len > 0:
-    checkForInlineErrors(r, expected, given, test, target)
+    checkForInlineErrors(r, expected, given, test, target, extraOptions)
   elif strip(expected.msg) notin strip(given.msg):
-    r.addResult(test, target, expected.msg, given.msg, reMsgsDiffer)
+    r.addResult(test, target, extraOptions, expected.msg, given.msg, reMsgsDiffer)
   elif not nimoutCheck(expected, given):
-    r.addResult(test, target, expected.nimout, given.nimout, reMsgsDiffer)
+    r.addResult(test, target, extraOptions, expected.nimout, given.nimout, reMsgsDiffer)
   elif extractFilename(expected.file) != extractFilename(given.file) and
       "internal error:" notin expected.msg:
-    r.addResult(test, target, expected.file, given.file, reFilesDiffer)
+    r.addResult(test, target, extraOptions, expected.file, given.file, reFilesDiffer)
   elif expected.line != given.line and expected.line != 0 or
        expected.column != given.column and expected.column != 0:
-    r.addResult(test, target, $expected.line & ':' & $expected.column,
+    r.addResult(test, target, extraOptions, $expected.line & ':' & $expected.column,
                       $given.line & ':' & $given.column, reLinesDiffer)
   else:
-    r.addResult(test, target, expected.msg, given.msg, reSuccess)
+    r.addResult(test, target, extraOptions, expected.msg, given.msg, reSuccess)
     inc(r.passed)
 
 proc generatedFile(test: TTest, target: TTarget): string =
@@ -434,8 +438,8 @@ proc codegenCheck(test: TTest, target: TTarget, spec: TSpec, expectedMsg: var st
     given.err = reCodeNotFound
     echo getCurrentExceptionMsg()
 
-proc compilerOutputTests(test: TTest, target: TTarget, given: var TSpec,
-                         expected: TSpec; r: var TResults) =
+proc compilerOutputTests(test: TTest, target: TTarget, extraOptions: string,
+                         given: var TSpec, expected: TSpec; r: var TResults) =
   var expectedmsg: string = ""
   var givenmsg: string = ""
   if given.err == reSuccess:
@@ -449,7 +453,7 @@ proc compilerOutputTests(test: TTest, target: TTarget, given: var TSpec,
   else:
     givenmsg = "$ " & given.cmd & '\n' & given.nimout
   if given.err == reSuccess: inc(r.passed)
-  r.addResult(test, target, expectedmsg, givenmsg, given.err)
+  r.addResult(test, target, extraOptions, expectedmsg, givenmsg, given.err)
 
 proc getTestSpecTarget(): TTarget =
   if getEnv("NIM_COMPILE_TO_CPP", "false") == "true":
@@ -457,16 +461,6 @@ proc getTestSpecTarget(): TTarget =
   else:
     result = targetC
 
-proc checkDisabled(r: var TResults, test: TTest): bool =
-  if test.spec.err in {reDisabled, reJoined}:
-    # targetC is a lie, but parameter is required
-    r.addResult(test, targetC, "", "", test.spec.err)
-    inc(r.skipped)
-    inc(r.total)
-    result = false
-  else:
-    result = true
-
 var count = 0
 
 proc equalModuloLastNewline(a, b: string): bool =
@@ -474,8 +468,13 @@ proc equalModuloLastNewline(a, b: string): bool =
   result = a == b or b.endsWith("\n") and a == b[0 ..< ^1]
 
 proc testSpecHelper(r: var TResults, test: var TTest, expected: TSpec,
-                    target: TTarget, nimcache: string, extraOptions = "") =
+                    target: TTarget, extraOptions: string, nimcache: string) =
   test.startTime = epochTime()
+  if test.spec.err in {reDisabled, reJoined}:
+    r.addResult(test, target, extraOptions, "", "", test.spec.err)
+    inc(r.skipped)
+    return
+
   template callNimCompilerImpl(): untyped =
     # xxx this used to also pass: `--stdout --hint:Path:off`, but was done inconsistently
     # with other branches
@@ -483,21 +482,21 @@ proc testSpecHelper(r: var TResults, test: var TTest, expected: TSpec,
   case expected.action
   of actionCompile:
     var given = callNimCompilerImpl()
-    compilerOutputTests(test, target, given, expected, r)
+    compilerOutputTests(test, target, extraOptions, given, expected, r)
   of actionRun:
     var given = callNimCompilerImpl()
     if given.err != reSuccess:
-      r.addResult(test, target, "", "$ " & given.cmd & '\n' & given.nimout, given.err, givenSpec = given.addr)
+      r.addResult(test, target, extraOptions, "", "$ " & given.cmd & '\n' & given.nimout, given.err, givenSpec = given.addr)
     else:
       let isJsTarget = target == targetJS
       var exeFile = changeFileExt(test.name, if isJsTarget: "js" else: ExeExt)
       if not fileExists(exeFile):
-        r.addResult(test, target, expected.output,
+        r.addResult(test, target, extraOptions, expected.output,
                     "executable not found: " & exeFile, reExeNotFound)
       else:
         let nodejs = if isJsTarget: findNodeJs() else: ""
         if isJsTarget and nodejs == "":
-          r.addResult(test, target, expected.output, "nodejs binary not in PATH",
+          r.addResult(test, target, extraOptions, expected.output, "nodejs binary not in PATH",
                       reExeNotFound)
         else:
           var exeCmd: string
@@ -528,24 +527,24 @@ proc testSpecHelper(r: var TResults, test: var TTest, expected: TSpec,
             else:
               buf
           if exitCode != expected.exitCode:
-            r.addResult(test, target, "exitcode: " & $expected.exitCode,
+            r.addResult(test, target, extraOptions, "exitcode: " & $expected.exitCode,
                               "exitcode: " & $exitCode & "\n\nOutput:\n" &
                               bufB, reExitcodesDiffer)
           elif (expected.outputCheck == ocEqual and not expected.output.equalModuloLastNewline(bufB)) or
               (expected.outputCheck == ocSubstr and expected.output notin bufB):
             given.err = reOutputsDiffer
-            r.addResult(test, target, expected.output, bufB, reOutputsDiffer)
+            r.addResult(test, target, extraOptions, expected.output, bufB, reOutputsDiffer)
           else:
-            compilerOutputTests(test, target, given, expected, r)
+            compilerOutputTests(test, target, extraOptions, given, expected, r)
   of actionReject:
     let given = callNimCompilerImpl()
-    cmpMsgs(r, expected, given, test, target)
+    cmpMsgs(r, expected, given, test, target, extraOptions)
 
-proc targetHelper(r: var TResults, test: TTest, expected: TSpec, extraOptions = "") =
+proc targetHelper(r: var TResults, test: TTest, expected: TSpec, extraOptions: string) =
   for target in expected.targets:
     inc(r.total)
     if target notin gTargets:
-      r.addResult(test, target, "", "", reDisabled)
+      r.addResult(test, target, extraOptions, "", "", reDisabled)
       inc(r.skipped)
     elif simulate:
       inc count
@@ -553,16 +552,15 @@ proc targetHelper(r: var TResults, test: TTest, expected: TSpec, extraOptions =
     else:
       let nimcache = nimcacheDir(test.name, test.options, target)
       var testClone = test
-      testSpecHelper(r, testClone, expected, target, nimcache, extraOptions)
+      testSpecHelper(r, testClone, expected, target, extraOptions, nimcache)
 
 proc testSpec(r: var TResults, test: TTest, targets: set[TTarget] = {}) =
   var expected = test.spec
   if expected.parseErrors.len > 0:
     # targetC is a lie, but a parameter is required
-    r.addResult(test, targetC, "", expected.parseErrors, reInvalidSpec)
+    r.addResult(test, targetC, "", "", expected.parseErrors, reInvalidSpec)
     inc(r.total)
     return
-  if not checkDisabled(r, test): return
 
   expected.targets.incl targets
   # still no target specified at all
@@ -572,14 +570,13 @@ proc testSpec(r: var TResults, test: TTest, targets: set[TTarget] = {}) =
     for m in test.spec.matrix:
       targetHelper(r, test, expected, m)
   else:
-    targetHelper(r, test, expected)
+    targetHelper(r, test, expected, "")
 
 proc testSpecWithNimcache(r: var TResults, test: TTest; nimcache: string) {.used.} =
-  if not checkDisabled(r, test): return
   for target in test.spec.targets:
     inc(r.total)
     var testClone = test
-    testSpecHelper(r, testClone, test.spec, target, nimcache)
+    testSpecHelper(r, testClone, test.spec, target, "", nimcache)
 
 proc makeTest(test, options: string, cat: Category): TTest =
   result.cat = cat