From 63cca93ea9bcab3c1fea39ff3789fd3a656d2db2 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Thu, 19 May 2022 17:11:53 +0200 Subject: [PATCH] testament: include extra options in test name (#19801) there's currently no (simple) way to disambiguate which option failed --- testament/categories.nim | 12 +++--- testament/testament.nim | 79 +++++++++++++++++++--------------------- 2 files changed, 44 insertions(+), 47 deletions(-) diff --git a/testament/categories.nim b/testament/categories.nim index 46cc3b210908..a092fec84bfe 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 4b5a8a147930..a4329b982950 100644 --- a/testament/testament.nim +++ b/testament/testament.nim @@ -259,7 +259,8 @@ Tests skipped: $4 / $1
""" % [$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