-
Notifications
You must be signed in to change notification settings - Fork 39
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
tests: enable the exceptions
category for all targets
#1082
tests: enable the exceptions
category for all targets
#1082
Conversation
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
The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change itself looks good, testament issue aside.
/merge |
Merge requested by: @saem Contents after the first section break of the PR description has been removed and preserved below:
|
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:
#1234
are replaced with proper GitHublinks
targets
key is removed for tests that should be run on alltargets
cmd
is replaced with usage ofmatrix
(allowing formulti-target testing)
write(stdout, ...)
is replaced withecho
in order to support theJS and VM targets
static:
blocks is replaced with using the VM target,when the test is not specific to compile-time execution
knownIssue
Specific changes:
getCurrentExceptionMsg
isreplaced with
getCurrentException().msg
, as the former is not yetsupported by the VM
texceptions2.nim
is removed -- it's redundant withtexception.nim
texception_message_null_byte.nim
is turned into a normal test thatdoesn't rely on self-execution