Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #14242 testament r tests/js/foo now works; testament now honors --targets #16163

Merged
merged 2 commits into from
Nov 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions testament/categories.nim
Original file line number Diff line number Diff line change
Expand Up @@ -563,13 +563,13 @@ proc `&.?`(a, b: string): string =
# candidate for the stdlib?
result = if b.startsWith(a): b else: a & b

proc processSingleTest(r: var TResults, cat: Category, options, test: string) =
let test = testsDir &.? cat.string / test
let target = if cat.string.normalize == "js": targetJS else: targetC
if fileExists(test):
testSpec r, makeTest(test, options, cat), {target}
else:
doAssert false, test & " test does not exist"
proc processSingleTest(r: var TResults, cat: Category, options, test: string, targets: set[TTarget], targetsSet: bool) =
var targets = targets
if not targetsSet:
let target = if cat.string.normalize == "js": targetJS else: targetC
targets = {target}
doAssert fileExists(test), test & " test does not exist"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is input validaton, not an assertion.

Copy link
Member Author

@timotheecour timotheecour Nov 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree but the doAssert is pre-existing, look at the diff.

we can discuss how to properly fix this in subsequent PR (quit is not the answer as it doesn't play well with library integration among other issues; enforce is the most natural fit but enforce PR #15606 is still open)

Copy link
Member

@Araq Araq Nov 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quit is an answer until testament exposes an API. And only then we can add a callback for error handling or use a raise statement etc.

testSpec r, makeTest(test, options, cat), targets

proc isJoinableSpec(spec: TSpec): bool =
result = not spec.sortoutput and
Expand Down
8 changes: 4 additions & 4 deletions testament/specs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ type
reInvalidSpec # test had problems to parse the spec

TTarget* = enum
targetC = "C"
targetCpp = "C++"
targetObjC = "ObjC"
targetJS = "JS"
targetC = "c"
targetCpp = "cpp"
ringabout marked this conversation as resolved.
Show resolved Hide resolved
targetObjC = "objc"
targetJS = "js"

InlineError* = object
kind*: string
Expand Down
14 changes: 9 additions & 5 deletions testament/testament.nim
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ let
pegOfInterest = pegLineError / pegOtherError

var gTargets = {low(TTarget)..high(TTarget)}
var targetsSet = false

proc isSuccess(input: string): bool =
# not clear how to do the equivalent of pkg/regex's: re"FOO(.*?)BAR" in pegs
Expand Down Expand Up @@ -685,6 +686,7 @@ proc main() =
of "targets":
targetsStr = p.val.string
gTargets = parseTargets(targetsStr)
targetsSet = true
of "nim":
compilerPrefix = addFileExt(p.val.string, ExeExt)
of "directory":
Expand Down Expand Up @@ -794,14 +796,16 @@ proc main() =
p.next
processPattern(r, pattern, p.cmdLineRest.string, simulate)
of "r", "run":
# "/pathto/tests/stdlib/nre/captures.nim" -> "stdlib" + "tests/stdlib/nre/captures.nim"
var subPath = p.key.string
if subPath.isAbsolute: subPath = subPath.relativePath(getCurrentDir())
let nimRoot = currentSourcePath / "../.."
# makes sure points to this regardless of cwd or which nim is used to compile this.
doAssert existsDir(nimRoot/testsDir) # sanity check
timotheecour marked this conversation as resolved.
Show resolved Hide resolved
if subPath.isAbsolute: subPath = subPath.relativePath(nimRoot)
# at least one directory is required in the path, to use as a category name
let pathParts = split(subPath, {DirSep, AltSep})
# "stdlib/nre/captures.nim" -> "stdlib" + "nre/captures.nim"
ringabout marked this conversation as resolved.
Show resolved Hide resolved
let pathParts = subPath.relativePath(testsDir).split({DirSep, AltSep})
let cat = Category(pathParts[0])
subPath = joinPath(pathParts[1..^1])
processSingleTest(r, cat, p.cmdLineRest.string, subPath)
processSingleTest(r, cat, p.cmdLineRest.string, subPath, gTargets, targetsSet)
of "html":
generateHtml(resultsFile, optFailing)
else:
Expand Down
28 changes: 14 additions & 14 deletions tests/testament/tshould_not_work.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,36 @@ discard """
cmd: "testament/testament --directory:testament --colors:off --backendLogging:off --nim:../compiler/nim category shouldfail"
action: compile
nimout: '''
FAIL: tests/shouldfail/tccodecheck.nim C
FAIL: tests/shouldfail/tccodecheck.nim c
Failure: reCodegenFailure
Expected:
baz
FAIL: tests/shouldfail/tcolumn.nim C
FAIL: tests/shouldfail/tcolumn.nim c
Failure: reLinesDiffer
FAIL: tests/shouldfail/terrormsg.nim C
FAIL: tests/shouldfail/terrormsg.nim c
Failure: reMsgsDiffer
FAIL: tests/shouldfail/texitcode1.nim C
FAIL: tests/shouldfail/texitcode1.nim c
Failure: reExitcodesDiffer
FAIL: tests/shouldfail/tfile.nim C
FAIL: tests/shouldfail/tfile.nim c
Failure: reFilesDiffer
FAIL: tests/shouldfail/tline.nim C
FAIL: tests/shouldfail/tline.nim c
Failure: reLinesDiffer
FAIL: tests/shouldfail/tmaxcodesize.nim C
FAIL: tests/shouldfail/tmaxcodesize.nim c
Failure: reCodegenFailure
max allowed size: 1
FAIL: tests/shouldfail/tnimout.nim C
FAIL: tests/shouldfail/tnimout.nim c
Failure: reMsgsDiffer
FAIL: tests/shouldfail/toutput.nim C
FAIL: tests/shouldfail/toutput.nim c
Failure: reOutputsDiffer
FAIL: tests/shouldfail/toutputsub.nim C
FAIL: tests/shouldfail/toutputsub.nim c
Failure: reOutputsDiffer
FAIL: tests/shouldfail/treject.nim C
FAIL: tests/shouldfail/treject.nim c
Failure: reFilesDiffer
FAIL: tests/shouldfail/tsortoutput.nim C
FAIL: tests/shouldfail/tsortoutput.nim c
Failure: reOutputsDiffer
FAIL: tests/shouldfail/ttimeout.nim C
FAIL: tests/shouldfail/ttimeout.nim c
Failure: reTimeout
FAIL: tests/shouldfail/tvalgrind.nim C
FAIL: tests/shouldfail/tvalgrind.nim c
Failure: reExitcodesDiffer
'''
"""
Expand Down