fix #17952: fix both false positives and false negatives for reInvalidSpec (#17956)

* fix #17952: fix both false positives and false negatives for reInvalidSpec

* handle megatest properly

* fix for tests/stdlib/tbase64.nim

6 files changed, 69 insertions, 43 deletions

diff --git a/testament/categories.nim b/testament/categories.nim
index f1dee3570..1253a2b8c 100644
--- a/testament/categories.nim
+++ b/testament/categories.nim
@@ -600,12 +600,19 @@ proc runJoinedTest(r: var TResults, cat: Category, testsDir: string, options: st
   ]#
   var specs: seq[TSpec] = @[]
   for kind, dir in walkDir(testsDir):
-    assert testsDir.startsWith(testsDir)
+    assert dir.startsWith(testsDir)
     let cat = dir[testsDir.len .. ^1]
     if kind == pcDir and cat notin specialCategories:
       for file in walkDirRec(testsDir / cat):
         if isTestFile(file):
-          let spec = parseSpec(file)
+          var spec: TSpec
+          try:
+            spec = parseSpec(file)
+          except ValueError:
+            # e.g. for `tests/navigator/tincludefile.nim` which have multiple
+            # specs; this will be handled elsewhere
+            echo "parseSpec failed for: '$1', assuming this will be handled outside of megatest" % file
+            continue
           if isJoinableSpec(spec):
             specs.add spec
 
diff --git a/testament/specs.nim b/testament/specs.nim
index 3774778b3..9257449c8 100644
--- a/testament/specs.nim
+++ b/testament/specs.nim
@@ -179,6 +179,7 @@ proc extractErrorMsg(s: string; i: int; line: var int; col: var int; spec: var T
 proc extractSpec(filename: string; spec: var TSpec): string =
   const
     tripleQuote = "\"\"\""
+    specStart = "discard " & tripleQuote
   var s = readFile(filename)
 
   var i = 0
@@ -187,25 +188,34 @@ proc extractSpec(filename: string; spec: var TSpec): string =
   var line = 1
   var col = 1
   while i < s.len:
-    if s.continuesWith(tripleQuote, i):
-      if a < 0: a = i
-      elif b < 0: b = i
-      inc i, 2
-      inc col
+    if (i == 0 or s[i-1] != ' ') and s.continuesWith(specStart, i):
+      # `s[i-1] == '\n'` would not work because of `tests/stdlib/tbase64.nim` which contains BOM (https://en.wikipedia.org/wiki/Byte_order_mark)
+      const lineMax = 10
+      if a != -1:
+        raise newException(ValueError, "testament spec violation: duplicate `specStart` found: " & $(filename, a, b, line))
+      elif line > lineMax:
+        # not overly restrictive, but prevents mistaking some `specStart` as spec if deeep inside a test file
+        raise newException(ValueError, "testament spec violation: `specStart` should be before line $1, or be indented; info: $2" % [$lineMax, $(filename, a, b, line)])
+      i += specStart.len
+      a = i
+    elif a > -1 and b == -1 and s.continuesWith(tripleQuote, i):
+      b = i
+      i += tripleQuote.len
     elif s[i] == '\n':
       inc line
+      inc i
       col = 1
     elif s.continuesWith(inlineErrorMarker, i):
       i = extractErrorMsg(s, i, line, col, spec)
     else:
       inc col
-    inc i
+      inc i
 
-  # look for """ only in the first section
-  if a >= 0 and b > a and a < 40:
-    result = s.substr(a+3, b-1).multiReplace({"'''": tripleQuote, "\\31": "\31"})
+  if a >= 0 and b > a:
+    result = s.substr(a, b-1).multiReplace({"'''": tripleQuote, "\\31": "\31"})
+  elif a >= 0:
+    raise newException(ValueError, "testament spec violation: `specStart` found but not trailing `tripleQuote`: $1" % $(filename, a, b, line))
   else:
-    #echo "warning: file does not contain spec: " & filename
     result = ""
 
 proc parseTargets*(value: string): set[TTarget] =
diff --git a/tests/manyloc/keineschweine/lib/sg_gui.nim b/tests/manyloc/keineschweine/lib/sg_gui.nim
index 83ad0d1bf..bcf415556 100644
--- a/tests/manyloc/keineschweine/lib/sg_gui.nim
+++ b/tests/manyloc/keineschweine/lib/sg_gui.nim
@@ -193,28 +193,28 @@ proc setActive*(t: PTextEntry) =
   if not t.isNil and not t.inputClient.isNil:
     input_helpers.setActive(t.inputClient)
 
+when false:
+  proc newMessageArea*(container: PGuiContainer; position: TVector2f): PMessageArea =
+    new(result)
+    result.messages = @[]
+    result.pos = position
+    container.add(result)
+  proc add*(m: PMessageArea, text: string): PText =
+    result = messageProto.copy()
+    result.setString(text)
+    m.messages.add(result)
+    let nmsgs = len(m.messages)
+    var pos   = vec2f(m.pos.x, m.pos.y)
+    for i in countdown(nmsgs - 1, max(nmsgs - 30, 0)):
+      setPosition(m.messages[i], pos)
+      pos.y -= 16.0
 
-discard """proc newMessageArea*(container: PGuiContainer; position: TVector2f): PMessageArea =
-  new(result)
-  result.messages = @[]
-  result.pos = position
-  container.add(result)
-proc add*(m: PMessageArea, text: string): PText =
-  result = messageProto.copy()
-  result.setString(text)
-  m.messages.add(result)
-  let nmsgs = len(m.messages)
-  var pos   = vec2f(m.pos.x, m.pos.y)
-  for i in countdown(nmsgs - 1, max(nmsgs - 30, 0)):
-    setPosition(m.messages[i], pos)
-    pos.y -= 16.0
+  proc draw*(window: PRenderWindow; m: PMessageArea) =
+    let nmsgs = len(m.messages)
+    if nmsgs == 0: return
+    for i in countdown(nmsgs - 1, max(nmsgs - 30, 0)):
+      window.draw(m.messages[i])
 
-proc draw*(window: PRenderWindow; m: PMessageArea) =
-  let nmsgs = len(m.messages)
-  if nmsgs == 0: return
-  for i in countdown(nmsgs - 1, max(nmsgs - 30, 0)):
-    window.draw(m.messages[i])
-"""
 proc newMessageArea*(container: PGuiContainer; position: TVector2f): PMessageArea =
   new(result)
   result.messages = @[]
diff --git a/tests/manyloc/nake/nakefile.nim b/tests/manyloc/nake/nakefile.nim
index 95058c277..fc479a5c2 100644
--- a/tests/manyloc/nake/nakefile.nim
+++ b/tests/manyloc/nake/nakefile.nim
@@ -29,11 +29,12 @@ task "test2", "Build release test build test release build":
   if shell("nim", ReleaseDefines, ReleaseTestDefines, "compile", ExeName) == 0:
     shell "."/ExeName
 
-discard """task "dirserver", "build the directory server":
-  withDir "server":
-    if shell("nim", ServerDefines, "compile", "dirserver") != 0:
-      echo "Failed to build the dirserver"
-      quit 1"""
+when false:
+  task "dirserver", "build the directory server":
+    withDir "server":
+      if shell("nim", ServerDefines, "compile", "dirserver") != 0:
+        echo "Failed to build the dirserver"
+        quit 1
 
 task "zoneserver", "build the zone server":
   withDir "enet_server":
diff --git a/tests/modules/texplicit_system_import.nim b/tests/modules/texplicit_system_import.nim
index bc4d018bf..0a4cedc71 100644
--- a/tests/modules/texplicit_system_import.nim
+++ b/tests/modules/texplicit_system_import.nim
@@ -1,9 +1,9 @@
-##.
 import system except `+`
+
 discard """
   errormsg: "undeclared identifier: '+'"
   line: 9
 """
-# Testament requires that the initial """ occurs before the 40th byte
-# in the file. No kidding...
+
+
 echo 4+5
diff --git a/tests/statictypes/tstatictypes.nim b/tests/statictypes/tstatictypes.nim
index a76276d2c..c0eb62e21 100644
--- a/tests/statictypes/tstatictypes.nim
+++ b/tests/statictypes/tstatictypes.nim
@@ -1,9 +1,17 @@
 discard """
+nimoutFull: true
 nimout: '''
 staticAlialProc instantiated with 358
 staticAlialProc instantiated with 368
 0: Foo
 1: Bar
+0: Foo
+1: Bar
+0: Foo
+1: Bar
+0: Foo
+1: Bar
+Hint: ***SLOW, DEBUG BUILD***; -d:release makes code run faster. [BuildMode]
 '''
 output: '''
 16
@@ -15,8 +23,11 @@ heyho
 Val1
 Val1
 '''
+matrix: "--hint:XDeclaredButNotUsed:off --hint:cc:off --hint:link:off --hint:SuccessX:off --hint:conf:off"
 """
 
+# pending https://github.com/nim-lang/Nim/pull/17852 use `--hints:none --hint:SuccessX:off`, or improve `isSuccess`
+
 import macros
 
 template ok(x) = doAssert(x)
@@ -247,9 +258,6 @@ echo t.foo, u.bar
 #------------------------------------------------------------------------------
 # issue #9679
 
-discard """
-  output: ''''''
-"""
 type
   Foo*[T] = object
     bar*: int