fix #16693: testament spec nimout too lax
#16698
Conversation
how about we add EDIT: see #16713 |
|
friendly ping on this! |
|
After I clean up all my working PRs. |
|
will continue on this PR |
| errormsg: "type mismatch: got <int>" | ||
| line: 17 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
then I need to change line number which is a huge work can be left in the future PR.
There was a problem hiding this comment.
yes can be deferred
then I need to change line number
no, you could just add empty newlines
| @@ -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 | |||
There was a problem hiding this comment.
another bug: if i change this to line:1234, the test still passes
EDIT: filed #17636
can be fixed in future work
There was a problem hiding this comment.
where is the logic? Related to my patch?
There was a problem hiding this comment.
pre-existing, it's unrelated to your PR, just that i noticed it while reviewing your PR
Co-authored-by: Timothee Cour <[email protected]>
| discard """ | ||
| matrix: "--hint:processing" | ||
| nimout: ''' | ||
| .. |
There was a problem hiding this comment.
| .. | |
| compile start | |
| .. |
There was a problem hiding this comment.
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
| if line.len != 0: | ||
| while line != currentLine: | ||
| currentLine = strip(rhsIter()) | ||
| if rhsIter.finished: |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
LGTM after addressing CI failure on windows, for eg via #16698 (comment)
(remaining points can be addressed in this PR or in future PRs)
note: the targets: "c" became needed after this PR because now nimout whole lines must be matched, eg in
tests/testament/tshould_not_work.nim:32:24:FAIL: tests/shouldfail/ttimeout.nim c
to re-enable non-c targets (ie cpp here), we can in future work run tests/testament/tshould_not_work.nim via trunner.nim instead of via testament spec (only needs to be done for this file IIUC)
…nal param to greedyOrderedSubsetLines
…nal param to greedyOrderedSubsetLines
…nal param to greedyOrderedSubsetLines
…work`; mitigate #17946 tchannels timeouts (#17947) * refs #17946; refactor testament test summary, show test duration for failures; increase timeout tchannels * revert workarounds from #16698 and add allowPrefixMatch optional param to greedyOrderedSubsetLines * add test * workaround for yet another testament bug
Co-authored-by: Timothee Cour <[email protected]>
…work`; mitigate nim-lang#17946 tchannels timeouts (nim-lang#17947) * refs nim-lang#17946; refactor testament test summary, show test duration for failures; increase timeout tchannels * revert workarounds from nim-lang#16698 and add allowPrefixMatch optional param to greedyOrderedSubsetLines * add test * workaround for yet another testament bug
fix #16693