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

testament: line spec silently ignored in tests where errormsg is not specified #17636

Closed
timotheecour opened this issue Apr 4, 2021 · 0 comments · Fixed by #17643
Closed
Assignees

Comments

@timotheecour
Copy link
Member

timotheecour commented Apr 4, 2021

Example

in tests/init/tuninit1.nim:

discard """
  nimout: "Warning: use explicit initialization of 'y' for clarity [Uninit]"
  line:34
  action: compile
"""
...

change line:34 to line:1234

Current Output

test still passes

Expected Output

give error saying that line is ignored in presence of nimout and shouldn't be specified.

note also that specifying line and col and errormsg is IMO bad (because it's more work), instead, the test should specify if there's an error by adding relevant line to nimout, eg:

twrongcolon.nim(11, 12) Error: in expression ' do:

=> less work and less maintenance (just copy paste error)
=> test is in no way weaker

Additional Information

EDIT: line is honored when errormsg is specified, and silently ignored when errormsg is not specified.
See #17643 (comment) for more details on this and for suggested fix.

ringabout added a commit to ringabout/Nim that referenced this issue Apr 5, 2021
@ringabout ringabout self-assigned this Apr 5, 2021
@timotheecour timotheecour changed the title testament: line spec silently ignored in tests testament: line spec silently ignored in tests where errormsg is not specified Apr 5, 2021
Araq pushed a commit that referenced this issue Apr 6, 2021
PMunch pushed a commit to PMunch/Nim that referenced this issue Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants