Merge #457

457: Skip "failing known issue" in CI post-failing r=haxscramper a=haxscramper - Clean up implementation of the backend reporting: pass around proper report object that we already have instead of taking it apart into half a dozen string arguments that need to be pieced back into a JSON object anyway. - Add information about known issues to the testament JSON cache so they can be used in later PR for tracking known issues that no longer fail - Skip failing issues in the CI post-processing in order to reduce visual clutter. Co-authored-by: haxscramper <haxscramper@gmail.com>
nim-works · Nov 4, 2022 · 57b3c17 · 57b3c17
2 parents c1f45f0 + cfa6b47
commit 57b3c17
Show file tree

Hide file tree

Showing 5 changed files with 117 additions and 57 deletions.
diff --git a/testament/backend.nim b/testament/backend.nim
@@ -6,7 +6,55 @@
 #    Look at license.txt for more info.
 #    All rights reserved.
 
-import strutils, os, osproc, json
+import
+  std/[
+    strutils,
+    os,
+    osproc,
+    json,
+    options,
+    tables
+  ],
+  specs, # Basic testament types
+  experimental/[
+    sexp,
+    sexp_diff
+  ]
+
+type
+  TOutReport* = object
+    inline*: Option[InlineError]
+    node*: SexpNode
+    file*: string
+
+  TOutCompare* = ref object
+    ## Result of comparing two data outputs for a given spec
+    match*: bool
+    expectedReports*: seq[TOutReport]
+    givenReports*: seq[TOutReport]
+    sortedMapping*: seq[tuple[pair: (int, int), cost: int]]
+    diffMap*: Table[(int, int), seq[SexpMismatch]]
+    ignoredExpected*: seq[int]
+    ignoredGiven*: seq[int]
+    cantIgnoreGiven*: bool
+
+  ReportParams* = object
+    ## Contains additional data about report execution state.
+    duration*: float ## Test execution duration
+    name*: string ## Name of the test
+    origName*: string
+    cat*: string ## Test category
+    action*: TTestAction ## Test action type
+    targetStr*: string
+    debugInfo*: string
+    outCompare*: TOutCompare
+    success*: TResultEnum
+    knownIssues*: seq[string] ## Whether the test was marked as a 'known
+                              ## issue'
+    inCurrentBatch*: bool
+    expected*: string ## Expected run output
+    given*: string ## Given run output
+
 
 type
   MachineId* = distinct string
@@ -40,7 +88,16 @@ var
   results: JsonNode
   currentCategory: string
 
-proc writeTestResult*(name, category, target, action, result, expected, given: string) =
+proc writeTestResult*(param: ReportParams) =
+  let
+    name = param.name
+    category = param.cat
+    target = param.targetStr
+    action = $param.action
+    result = $param.success
+    expected = param.expected
+    given = param.given
+
   createDir("testresults")
   if currentCategory != category:
     if currentCategory.len > 0:
@@ -53,9 +110,19 @@ proc writeTestResult*(name, category, target, action, result, expected, given: s
   if results.isNil():
     results = newJArray()
 
-  results.add %*{"name": name, "category": category, "target": target,
-    "action": action, "result": result, "expected": expected, "given": given,
-    "machine": thisMachine.string, "commit": thisCommit.string, "branch": thisBranch}
+  results.add %*{
+    "name": name,
+    "category": category,
+    "target": target,
+    "action": action,
+    "result": result,
+    "expected": expected,
+    "given": given,
+    "machine": thisMachine.string,
+    "commit": thisCommit.string,
+    "branch": thisBranch,
+    "knownIssues": %param.knownIssues
+  }
 
 proc open*() =
   thisMachine = getMachine()

diff --git a/testament/categories.nim b/testament/categories.nim
@@ -434,13 +434,16 @@ proc runJoinedTest(r: var TResults, cat: Category, testsDir: string, options: st
     ##
     ## We expect the program to exit soon after
     if backendLogging:
-      backend.writeTestResult(name = MegaTestCat,
-                              category = MegaTestCat,
-                              target = "c",
-                              action = "run",
-                              result = $res,
-                              expected = "",
-                              given = errorOutput)
+      backend.writeTestResult(ReportParams(
+        name: MegaTestCat,
+        cat: MegaTestCat,
+        targetStr: "c",
+        action: actionRun,
+        success: res,
+        expected: "",
+        given: errorOutput,
+        knownIssues: @[]
+      ))
 
       # Flush all buffers
       backend.close()
@@ -541,4 +544,4 @@ proc processCategory(r: var TResults, cat: Category,
         # `pkgs` because bug #16556 creates `pkgs` dirs and this can affect some users
         # that try an old version of choosenim.
       doAssert cat.string in allowedDirs,
-        "Invalid category specified: '$#' not in allow list: $#" % [cat.string, $allowedDirs]
+        "Invalid category specified: '$#' not in allow list: $#" % [cat.string, $allowedDirs]
diff --git a/testament/specs.nim b/testament/specs.nim
@@ -7,8 +7,18 @@
 #    distribution, for details about the copyright.
 #
 
-import sequtils, parseutils, strutils, os, streams, parsecfg,
-  tables, hashes, sets
+import
+  std/[
+    sequtils,
+    parseutils,
+    strutils,
+    os,
+    streams,
+    parsecfg,
+    tables,
+    hashes,
+    sets
+  ]
 
 type TestamentData* = ref object
   # better to group globals under 1 object; could group the other ones here too
@@ -570,4 +580,4 @@ proc parseSpec*(filename: string,
 
   result.inCurrentBatch = isCurrentBatch(testamentData0, filename) or result.unbatchable
   if not result.inCurrentBatch:
-    result.err = reDisabled
+    result.err = reDisabled
diff --git a/testament/testament.nim b/testament/testament.nim
@@ -142,23 +142,6 @@ proc trimUnitSep(x: var string) =
   if L > 0 and x[^1] == '\31':
     setLen x, L-1
 
-type
-  TOutReport = object
-    inline: Option[InlineError]
-    node: SexpNode
-    file: string
-
-  TOutCompare = ref object
-    ## Result of comparing two data outputs for a given spec
-    match: bool
-    expectedReports: seq[TOutReport]
-    givenReports: seq[TOutReport]
-    sortedMapping: seq[tuple[pair: (int, int), cost: int]]
-    diffMap: Table[(int, int), seq[SexpMismatch]]
-    ignoredExpected: seq[int]
-    ignoredGiven: seq[int]
-    cantIgnoreGiven: bool
-
 proc diffStrings*(a, b: string): tuple[output: string, same: bool] =
   let a = a.split("\n")
   let b = b.split("\n")
@@ -441,21 +424,6 @@ proc getName(run: TestRun): string =
   if run.test.options.len > 0:
     result.add ' ' & run.test.options
 
-type
-  ReportParams = object
-    ## Contains additional data about report execution state.
-    duration: float
-    name: string
-    origName: string
-    cat: string
-    action: TTestAction
-    targetStr: string
-    debugInfo: string
-    outCompare: TOutCompare
-    success: TResultEnum
-    inCurrentBatch: bool
-    expected, given: string
-
 proc logToConsole(param: ReportParams, givenSpec: ptr TSpec = nil) =
   ## Format test infomation to the console. `test` contains information
   ## about the test itself, `param` contains additional data about test
@@ -529,13 +497,7 @@ proc logToConsole(param: ReportParams, givenSpec: ptr TSpec = nil) =
 
 proc logToBackend(param: ReportParams) =
   if backendLogging:
-    backend.writeTestResult(name = param.name,
-                            category = param.cat,
-                            target = param.targetStr,
-                            action = $param.action,
-                            result = $param.success,
-                            expected = param.expected,
-                            given = param.given)
+    backend.writeTestResult(param)
 
 
 proc addResult(r: var TResults, param: ReportParams, givenSpec: ptr TSpec) =
@@ -584,6 +546,7 @@ proc addResult(
       debugInfo: run.debugInfo,
       outCompare: outCompare,
       success: success,
+      knownIssues: if isNil(givenSpec): @[] else: givenSpec[].knownIssues,
       inCurrentBatch: run.test.spec.inCurrentBatch,
       expected: expected,
       given: given
@@ -614,6 +577,7 @@ proc addResult(r: var TResults, test: TTest) =
       debugInfo: "",
       success: test.spec.err,
       inCurrentBatch: test.spec.inCurrentBatch,
+      knownIssues: test.spec.knownIssues,
       expected: "",
       given: given,
       outCompare: nil

diff --git a/tools/ci_testresults.nim b/tools/ci_testresults.nim
@@ -1,8 +1,24 @@
 ## Print summary of failed tests for CI
 
-import os, json, sets, strformat, strutils
+import
+  std/[
+    os,
+    json,
+    sets,
+    strformat,
+    strutils
+  ]
+
+const skip = toHashSet([
+  "reDisabled",
+  "reIgnored",
+  "reSuccess",
+  "reJoined",
+  "reKnownIssue"
+  # The test is a known issue that failed to execute, to avoud
+  # cluttering the CI output they are skipped.
+])
 
-const skip = toHashSet(["reDisabled", "reIgnored", "reSuccess", "reJoined"])
 let githubActions = existsEnv"GITHUB_ACTIONS"
 
 func formatResult(j: JsonNode): string =