Skip to content

Commit

Permalink
tests: enable the exceptions category for all targets (nim-works#1082)
Browse files Browse the repository at this point in the history
## Summary

Clean up the category and enable it for all targets by default. This
significantly increases test coverage of exceptions for the JS and VM
targets.

## Details

**General changes/cleanup:**
* references to issues like `nim-works#1234` are replaced with proper GitHub
  links
* the `targets` key is removed for tests that should be run on all
  targets
* usage of `cmd` is replaced with usage of `matrix` (allowing for
  multi-target testing)
* `write(stdout, ...)` is replaced with `echo` in order to support the
  JS and VM targets
* execution of `static:` blocks is replaced with using the VM target,
  when the test is not specific to compile-time execution
* tests that don't work but should are marked as `knownIssue`

**Specific changes:**
* where not necessary for the test, `getCurrentExceptionMsg` is
  replaced with `getCurrentException().msg`, as the former is not yet
  supported by the VM
* `texceptions2.nim` is removed -- it's redundant with `texception.nim`
* `texception_message_null_byte.nim` is turned into a normal test that
  doesn't rely on self-execution
  • Loading branch information
zerbina authored Jan 10, 2024
1 parent 6ebe027 commit 96a90f3
Show file tree
Hide file tree
Showing 22 changed files with 59 additions and 211 deletions.
2 changes: 1 addition & 1 deletion testament/testament.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1110,7 +1110,7 @@ func nativeTarget(): TTarget {.inline.} =
func defaultTargets(category: Category): set[TTarget] =
const standardTargets = {nativeTarget()}
case category.string
of "lang", "lang_callable":
of "lang", "lang_callable", "exception":
{targetC, targetJs, targetVM}
of "arc", "avr", "destructor", "distros", "dll", "gc", "osproc", "parallel",
"realtimeGC", "threads", "views", "valgrind":
Expand Down
1 change: 0 additions & 1 deletion tests/exception/tassignment.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
discard """
targets: "c js vm"
description: '''
Tests for assignments where an exception is raised during evaluation of
the right-hand side
Expand Down
2 changes: 1 addition & 1 deletion tests/exception/tasync_stacktrace_gc.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
discard """
matrix: "--gc:arc"
description: '''
. From https://github.com/nim-lang/Nim/issues/18620
Stacktrace when using async with arc/orc is different than
Expand All @@ -9,6 +8,7 @@ discard """
the stacktrace with arc/orc gives little help in finding out
where the problem is.
'''
knownIssue.js vm: "`getStackTraceEntries` is not yet supported"
"""

proc hello() =
Expand Down
8 changes: 5 additions & 3 deletions tests/exception/tcontinuexc.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
discard """
outputsub: "ECcaught"
outputsub: '''E
C
caught'''
exitcode: "1"
"""
type
Expand All @@ -17,8 +19,8 @@ try:
try:
genErrors("error!")
except ESomething:
stdout.write("E")
stdout.write("C")
echo "E"
echo "C"
raise newException(EsomeotherErr, "bla")
finally:
echo "caught"
Expand Down
2 changes: 1 addition & 1 deletion tests/exception/tdefer1.nim
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ B
A'''
"""

# bug #1742
# bug https://github.com/nim-lang/nim/issues/1742

import strutils
let x = try: parseInt("133a")
Expand Down
3 changes: 1 addition & 2 deletions tests/exception/tdont_overwrite_typename.nim
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
discard """
targets: "c"
output: '''Check passed
Check passed'''
"""

# bug #5628
# bug https://github.com/nim-lang/nim/issues/5628

proc checkException(ex: ref Exception) =
doAssert(ex.name == cstring"ValueError")
Expand Down
3 changes: 1 addition & 2 deletions tests/exception/texcas.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
discard """
targets: "c"
output: '''
Hello
Hello
Expand Down Expand Up @@ -34,7 +33,7 @@ test[Exception]()
test2()
testTryAsExpr(5)

# see bug #7115
# bug https://github.com/nim-lang/nim/issues/7115
doAssert(not compiles(
try:
echo 1
Expand Down
3 changes: 1 addition & 2 deletions tests/exception/texception_inference.nim
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
discard """
output: '''good'''
joinable: false
cmd: "nim c --gc:orc -d:release $file"
matrix: "-d:release"
"""

type
Expand Down
46 changes: 7 additions & 39 deletions tests/exception/texception_message_null_byte.nim
Original file line number Diff line number Diff line change
Expand Up @@ -5,46 +5,14 @@ description: '''
. Error output produced by raise newException is inconsistent with other
IO such as echo or stdout.write.
'''
joinable: false
matrix: ";-d:debug;-d:danger"
outputsub: "` and works fine! [Exception]"
exitcode: 1
"""

# marked as not joinable because this test self-executes

# `\0` not preserved on windows, so only the trailing part can be reliably
# tested for
const msg = "This char is `" & '\0' & "` and works fine!"

when defined nim_t13115:
# bug #13115
template fn =
raise newException(Exception, msg)
when defined nim_t13115_static:
static: fn()
fn()
else:
import std/[osproc,strformat,os,strutils]
proc main =
const nim = getCurrentCompilerExe()
const file = currentSourcePath
for b in "c js".split:
when defined(openbsd):
if b == "js":
# xxx bug: pending #13115
# remove special case once nodejs updated >= 12.16.2
# refs https://github.com/nim-lang/Nim/pull/16167#issuecomment-738270751
continue

# save CI time by avoiding mostly redundant combinations as far as this bug is concerned
var opts = case b
of "c": @["", "-d:nim_t13115_static", "-d:danger", "-d:debug"]
of "js": @["", "-d:nim_t13115_static"]
else: @[""]

for opt in opts:
let cmd = fmt"{nim} r -b:{b} -d:nim_t13115 {opt} --hints:off {file}"
let (outp, exitCode) = execCmdEx(cmd)
when defined windows:
# `\0` not preserved on windows
doAssert "` and works fine!" in outp, cmd & "\n" & msg
else:
doAssert msg in outp, cmd & "\n" & msg
doAssert exitCode == 1
main()
# bug https://github.com/nim-lang/nim/issues/13115
raise newException(Exception, msg)
13 changes: 6 additions & 7 deletions tests/exception/texception_name.nim
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
discard """
description: "Tests to make sure that the Exception.name field is set"
targets: "c js"
"""

template test(desc, body) {.dirty.} =
Expand All @@ -9,8 +8,6 @@ template test(desc, body) {.dirty.} =
proc wrapper() =
body

# TODO: once it's available in testament, use the VM target instead
static: wrapper()
wrapper()


Expand Down Expand Up @@ -46,10 +43,12 @@ test "Name is set on raise if it was unset":
except IOError as e:
doAssert e.name == "IOError"


# fails for the VM. See ``texception_name_vm_issue.nim``
block no_override_empty:
test "Don't override an explicitly empty name":
try:
raise (ref IOError)(name: "") # explicitly set name to an empty string
except IOError as e:
doAssert e.name == ""
when defined(vm):
# see ``texception_name_vm_issue.nim``
doAssert e.name != "", "works now"
else:
doAssert e.name == ""
11 changes: 5 additions & 6 deletions tests/exception/texception_name_vm_issue.nim
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
discard """
action: compile
target: "vm"
description: '''An empty exception name is overridden on raise when run in
the VM'''
knownIssue
"""

# The VM treats `cstring(nil)` and `cstring("")` as the same thing

static:
try:
raise (ref CatchableError)(name: "")
except CatchableError as e:
doAssert e.name == ""
try:
raise (ref CatchableError)(name: "")
except CatchableError as e:
doAssert e.name == ""
13 changes: 8 additions & 5 deletions tests/exception/texceptions.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
discard """
matrix: "--exceptions:goto"
output: '''
BEFORE
Expand All @@ -14,6 +13,9 @@ BEFORE
EXCEPT: IOError: hi
FINALLY
'''
knownIssue.vm: '''
Exception/finally handling is largely disfunctional in the VM
'''
"""

echo ""
Expand Down Expand Up @@ -57,7 +59,8 @@ proc return_in_except =
raise newException(IOError, "hi")

except:
echo "EXCEPT: ", getCurrentException().name, ": ", getCurrentExceptionMsg()
let e = getCurrentException()
echo "EXCEPT: ", e.name, ": ", e.msg
return

finally:
Expand All @@ -66,7 +69,7 @@ proc return_in_except =
try: return_in_except()
except: echo "RECOVER"

block: #10417
block: # https://github.com/nim-lang/nim/issues/10417
proc moo() {.noreturn.} = discard

let bar =
Expand All @@ -84,10 +87,10 @@ block:
try:
raise newException(ValueError, "xx")
except:
doAssert("xx" == getCurrentExceptionMsg())
doAssert("xx" == getCurrentException().msg)
raise newException(KeyError, "yy")
except:
doAssert("yy" == getCurrentExceptionMsg())
doAssert("yy" == getCurrentException().msg)
result.add(1212)
try:
try:
Expand Down
129 changes: 0 additions & 129 deletions tests/exception/texceptions2.nim

This file was deleted.

2 changes: 1 addition & 1 deletion tests/exception/texcpt1.nim
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ proc blah(): int =

echo blah()

# Issue #7845, raise generic exception
# https://github.com/nim-lang/nim/issues/7845, raise generic exception
var x: ref ESomethingGen[int]
new(x)
try:
Expand Down
Loading

0 comments on commit 96a90f3

Please sign in to comment.