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 #16693: testament spec nimout too lax #16698

Merged
merged 21 commits into from
Apr 4, 2021
Merged
Show file tree
Hide file tree
Changes from 20 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
33 changes: 23 additions & 10 deletions testament/lib/stdtest/testutils.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import std/private/miscdollars
import std/strutils
from std/os import getEnv

template flakyAssert*(cond: untyped, msg = "", notifySuccess = true) =
Expand All @@ -26,15 +25,29 @@ template flakyAssert*(cond: untyped, msg = "", notifySuccess = true) =
msg2.add $expr & " " & msg
echo msg2

proc greedyOrderedSubsetLines*(lhs, rhs: string): bool =
## returns true if each stripped line in `lhs` appears in rhs, using a greedy matching.
let rhs = rhs.strip
var currentPos = 0
for line in lhs.strip.splitLines:
currentPos = rhs.find(line.strip, currentPos)
if currentPos < 0:
return false
return true
when not defined(js):
import std/strutils

proc greedyOrderedSubsetLines*(lhs, rhs: string): bool =
## Returns true if each stripped line in `lhs` appears in rhs, using a greedy matching.
iterator splitLinesClosure(): string {.closure.} =
for line in splitLines(rhs.strip):
yield line

var rhsIter = splitLinesClosure
var currentLine = strip(rhsIter())

for line in lhs.strip.splitLines:
let line = line.strip
if line.len != 0:
while line != currentLine:
currentLine = strip(rhsIter())
if rhsIter.finished:
Copy link
Member

Choose a reason for hiding this comment

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

just use a seq for rhs, it'll be clearer than with rhs.finished + rhsIter() (which complicates readability because of iterator semantics which requires calling iter before finished)

return false

if rhsIter.finished:
return false
return true

template enableRemoteNetworking*: bool =
## Allows contolling whether to run some test at a statement-level granularity.
Expand Down
3 changes: 2 additions & 1 deletion testament/tests/shouldfail/tccodecheck.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
discard """
ccodecheck: "baz"
targets: "c"
ccodecheck: "baz"
"""

proc foo(): void {.exportc: "bar".}=
Expand Down
7 changes: 4 additions & 3 deletions testament/tests/shouldfail/tcolumn.nim
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
discard """
errormsg: "undeclared identifier: 'undeclared'"
line: 8
column: 7
errormsg: "undeclared identifier: 'undeclared'"
targets: "c"
line: 9
column: 7
"""

# test should fail because the line directive is wrong
Expand Down
7 changes: 4 additions & 3 deletions testament/tests/shouldfail/terrormsg.nim
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
discard """
errormsg: "wrong error message"
line: 8
column: 6
errormsg: "wrong error message"
targets: "c"
line: 9
column: 6
"""

# test should fail because the line directive is wrong
Expand Down
3 changes: 2 additions & 1 deletion testament/tests/shouldfail/texitcode1.nim
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
discard """
exitcode: 1
targets: "c"
exitcode: 1
"""
5 changes: 3 additions & 2 deletions testament/tests/shouldfail/tfile.nim
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
discard """
errormsg: "undeclared identifier: 'undefined'"
file: "notthisfile.nim"
targets: "c"
errormsg: "undeclared identifier: 'undefined'"
file: "notthisfile.nim"
"""

echo undefined
7 changes: 4 additions & 3 deletions testament/tests/shouldfail/tline.nim
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
discard """
errormsg: "undeclared identifier: 'undeclared'"
line: 9
column: 6
targets: "c"
errormsg: "undeclared identifier: 'undeclared'"
line: 10
column: 6
"""

# test should fail because the line directive is wrong
Expand Down
3 changes: 2 additions & 1 deletion testament/tests/shouldfail/tmaxcodesize.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
discard """
maxcodesize: 1
targets: "c"
maxcodesize: 1
"""

echo "Hello World"
5 changes: 3 additions & 2 deletions testament/tests/shouldfail/tnimout.nim
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
discard """
nimout: "Hello World!"
action: compile
targets: "c"
nimout: "Hello World!"
action: compile
"""

static:
Expand Down
7 changes: 4 additions & 3 deletions testament/tests/shouldfail/toutput.nim
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
discard """
output: '''
done
'''
targets: "c"
output: '''
done
'''
"""

echo "broken"
3 changes: 2 additions & 1 deletion testament/tests/shouldfail/toutputsub.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
discard """
outputsub: "something else"
outputsub: "something else"
targets: "c"
"""

echo "Hello World!"
3 changes: 2 additions & 1 deletion testament/tests/shouldfail/treject.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
discard """
action: "reject"
action: "reject"
targets: "c"
"""

# Because we set action="reject", we expect this line not to compile. But the
Expand Down
11 changes: 6 additions & 5 deletions testament/tests/shouldfail/tsortoutput.nim
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
discard """
sortoutput: true
output: '''
2
1
'''
sortoutput: true
targets: "c"
output: '''
2
1
'''
"""

# this test should ensure that the output is actually sorted
Expand Down
1 change: 1 addition & 0 deletions testament/tests/shouldfail/ttimeout.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
discard """
timeout: "0.1"
targets: "c"
"""

import os
Expand Down
5 changes: 3 additions & 2 deletions testament/tests/shouldfail/tvalgrind.nim
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
discard """
valgrind: true
cmd: "nim $target --gc:arc -d:useMalloc $options $file"
valgrind: true
targets: "c"
cmd: "nim $target --gc:arc -d:useMalloc $options $file"
"""

# this is the same check used by testament/specs.nim whether or not valgrind
Expand Down
2 changes: 1 addition & 1 deletion tests/compilerfeatures/texpandmacro.nim
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
discard """
cmd: "nim c --expandMacro:foo $file"
nimout: '''Hint: expanded macro:
nimout: '''texpandmacro.nim(17, 1) Hint: expanded macro:
echo ["injected echo"]
var x = 4 [ExpandMacro]
'''
Expand Down
2 changes: 1 addition & 1 deletion tests/errmsgs/tgcsafety.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ discard """
cmd: "nim check $file"
errormsg: "type mismatch: got <AsyncHttpServer, Port, proc (req: Request): Future[system.void]{.locks: <unknown>.}>"
nimout: '''
type mismatch: got <AsyncHttpServer, Port, proc (req: Request): Future[system.void]{.locks: <unknown>.}>
tgcsafety.nim(30, 18) Error: type mismatch: got <AsyncHttpServer, Port, proc (req: Request): Future[system.void]{.locks: <unknown>.}>
but expected one of:
proc serve(server: AsyncHttpServer; port: Port;
callback: proc (request: Request): Future[void] {.closure, gcsafe.};
Expand Down
2 changes: 1 addition & 1 deletion tests/errmsgs/twrongcolon.nim
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
discard """
errormsg: "in expression ' do:"
nimout: '''
Error: in expression ' do:
twrongcolon.nim(11, 12) Error: in expression ' do:
890': identifier expected, but found ''
'''

Expand Down
2 changes: 1 addition & 1 deletion tests/exprs/tresultwarning.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
discard """
nimout: "Special variable 'result' is shadowed. [ResultShadowed]"
nimout: "tresultwarning.nim(6, 7) Warning: Special variable 'result' is shadowed. [ResultShadowed]"
"""

proc test(): string =
Expand Down
2 changes: 1 addition & 1 deletion tests/init/tuninit1.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
discard """
nimout: "Warning: use explicit initialization of 'y' for clarity [Uninit]"
nimout: "tuninit1.nim(35, 11) Warning: use explicit initialization of 'y' for clarity [Uninit]"
line:34
action: compile
"""
Expand Down
2 changes: 1 addition & 1 deletion tests/objvariant/tcheckedfield1.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
discard """
nimout: "Warning: cannot prove that field 'x.s' is accessible [ProveField]"
nimout: "tcheckedfield1.nim(40, 6) Warning: cannot prove that field 'x.s' is accessible [ProveField]"
line:51
Copy link
Member

@timotheecour timotheecour Apr 4, 2021

Choose a reason for hiding this comment

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

another bug: if i change this to line:1234, the test still passes

EDIT: filed #17636

can be fixed in future work

Copy link
Member Author

@ringabout ringabout Apr 4, 2021

Choose a reason for hiding this comment

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

where is the logic? Related to my patch?

Copy link
Member

Choose a reason for hiding this comment

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

pre-existing, it's unrelated to your PR, just that i noticed it while reviewing your PR

action: run
output: "abc abc"
Expand Down
17 changes: 17 additions & 0 deletions tests/pragmas/thintprocessing.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
discard """
matrix: "--hint:processing"
ringabout marked this conversation as resolved.
Show resolved Hide resolved
nimout: '''
compile start
..
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
..
compile start
..

Copy link
Member

Choose a reason for hiding this comment

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

shoot, this fails on windows:

....compile start

(known bug IIRC)

given that we already have a similar test in trunner.nim, maybe add disabled: windows?

refs:

  block: # `hintProcessing` dots should not interfere with `static: echo` + friends
    let cmd = fmt"""{nim} r {defaultHintsOff} --hint:processing -f --eval:"static: echo 1+1""""
    let (outp, exitCode) = execCmdEx(cmd, options = {poStdErrToStdOut})
    template check3(cond) = doAssert cond, $(outp,)
    doAssert exitCode == 0
    let lines = outp.splitLines
    check3 lines.len == 3
    when not defined(windows): # xxx: on windows, dots not properly handled, gives: `....2\n\n`
      check3 lines[0].isDots
      check3 lines[1] == "2"
      check3 lines[2] == ""
    else:
      check3 "2" in outp

warn_module.nim(6, 6) Hint: 'test' is declared but not used [XDeclaredButNotUsed]
compile end
'''
"""

static:
echo "compile start"

import warn_module

static:
echo "compile end"
2 changes: 0 additions & 2 deletions tests/pragmas/twarning_off.nim
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
discard """
matrix: "--hint:processing"
nimout: '''
compile start
..
warn_module.nim(6, 6) Hint: 'test' is declared but not used [XDeclaredButNotUsed]
compile end
'''
Expand Down
2 changes: 1 addition & 1 deletion tests/stdlib/tcstring.nim
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
discard """
targets: "c cpp js"
matrix: "; --gc:arc"
matrix: "--gc:refc; --gc:arc"
"""

from std/sugar import collect
Expand Down
5 changes: 5 additions & 0 deletions tests/stdlib/ttestutils.nim
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@ block: # greedyOrderedSubsetLines
doAssert greedyOrderedSubsetLines("a1\na3", "a0\na1\na2\na3\na4")
doAssert not greedyOrderedSubsetLines("a3\na1", "a0\na1\na2\na3\na4") # out of order
doAssert not greedyOrderedSubsetLines("a1\na5", "a0\na1\na2\na3\na4") # a5 not in lhs

doAssert not greedyOrderedSubsetLines("a1\na5", "a0\na1\na2\na3\na4\nprefix:a5")
doAssert not greedyOrderedSubsetLines("a1\na5", "a0\na1\na2\na3\na4\na5:suffix")
ringabout marked this conversation as resolved.
Show resolved Hide resolved
doAssert not greedyOrderedSubsetLines("a5", "a0\na1\na2\na3\na4\nprefix:a5")
doAssert not greedyOrderedSubsetLines("a5", "a0\na1\na2\na3\na4\na5:suffix")
2 changes: 1 addition & 1 deletion tests/varres/tprevent_forloopvar_mutations.nim
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
discard """
errormsg: "type mismatch: got <int>"
line: 17
Comment on lines 2 to 3
Copy link
Member

Choose a reason for hiding this comment

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

do we need those 2 lines, given that we already capture

nimout: '''tprevent_forloopvar_mutations.nim(17, 7) Error: type mismatch: got <int>

I'd prefer simplicity here and in other tests; specifying separately line, col, errormsg is more work and nimout already captures what we want to test.

Ditto in other examples above, eg, replacing:

  errormsg: "undeclared identifier: 'undeclared'"
  line: 10
  column: 6

by:

  nimout: '''tcolumn.nim .nim(10, 6) undeclared identifier: 'undeclared''''

and similar examples

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

Copy link
Member Author

Choose a reason for hiding this comment

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

then I need to change line number which is a huge work can be left in the future PR.

Copy link
Member

Choose a reason for hiding this comment

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

yes can be deferred

then I need to change line number

no, you could just add empty newlines

nimout: '''type mismatch: got <int>
nimout: '''tprevent_forloopvar_mutations.nim(17, 7) Error: type mismatch: got <int>
but expected one of:
proc inc[T: Ordinal](x: var T; y = 1)
first type mismatch at position: 1
Expand Down
8 changes: 6 additions & 2 deletions tests/vm/tcompiletimetable.nim
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
discard """
nimout: '''2
nimout: '''
2
3
4:2
Got Hi
Got Hey
'''
output:'''
ringabout marked this conversation as resolved.
Show resolved Hide resolved
a
b
c'''
c
'''
"""

# bug #404
Expand Down
3 changes: 1 addition & 2 deletions tests/vm/tmisc_vm.nim
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ foo4
(a: 0, b: 0)
'''
"""
import std/sets
ringabout marked this conversation as resolved.
Show resolved Hide resolved

#bug #1009
type
Expand Down Expand Up @@ -95,8 +96,6 @@ static: simpleTryFinally()

# bug #10981

import sets

proc main =
for i in 0..<15:
var someSets = @[initHashSet[int]()]
Expand Down