-
Notifications
You must be signed in to change notification settings - Fork 13.5k
tests: Require run-fail
ui tests to have an exit code (SIGABRT
not ok)
#143002
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
base: master
Are you sure you want to change the base?
Conversation
rustbot has assigned @petrochenkov. Use |
Some changes occurred in src/tools/compiletest cc @jieyouxu |
This comment has been minimized.
This comment has been minimized.
why not simply also would accept |
@bors2 delegate=try |
@Enselic can now perform try builds on this pull request |
6ad1973
to
17be091
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
@bors2 try jobs=x86_64-msvc-1,x86_64-msvc-2 |
…try> tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok) Normally a `run-fail` ui test shall not be terminated by a signal like `SIGABRT`. So begin having that as a hard requirement. Some of our current tests do terminate by a signal however. Introduce and use `run-fail-without-exit-code` for those tests. This adds further (cc #142304, #142886) protection against the regression in #123733 since that bug also manifested as `SIGABRT` in `tests/ui/panics/panic-main.rs` (shown as `Aborted (core dumped)` in the logs attached to that issue, and I have also been able to reproduce this locally). ### TODO - [ ] what about on Windows? - [ ] also update docs at https://rustc-dev-guide.rust-lang.org/tests/directives.html#controlling-outcome-expectations - [ ] clean up the code ### Zulip discussion See https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/compiletest.3A.20terminate.20by.20signal.20vs.20exit.20with.20error/with/525611235 try-job: x86_64-msvc-1 try-job: x86_64-msvc-2
This comment was marked as resolved.
This comment was marked as resolved.
17be091
to
ad4e082
Compare
An only-windows test that was run-fail now must be run-crash. Fixed now. Trying again: @bors2 try jobs=x86_64-msvc-1,x86_64-msvc-2 |
…try> tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok) Normally a `run-fail` ui test shall not be terminated by a signal like `SIGABRT`. So begin having that as a hard requirement. Some of our current tests do terminate by a signal however. Introduce and use `run-fail-without-exit-code` for those tests. This adds further (cc #142304, #142886) protection against the regression in #123733 since that bug also manifested as `SIGABRT` in `tests/ui/panics/panic-main.rs` (shown as `Aborted (core dumped)` in the logs attached to that issue, and I have also been able to reproduce this locally). ### TODO - [ ] **Q:** what about on Windows? **A:** we'll treat any exit code outside of 1 - 127 as "crashed", and we'll do the same on unix. - [ ] also update docs at https://rustc-dev-guide.rust-lang.org/tests/directives.html#controlling-outcome-expectations - [ ] clean up the code - [ ] test all permutations of actual vs expected ### Zulip discussion See https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/compiletest.3A.20terminate.20by.20signal.20vs.20exit.20with.20error/with/525611235 try-job: x86_64-msvc-1 try-job: x86_64-msvc-2
This comment was marked as resolved.
This comment was marked as resolved.
ad4e082
to
2499dac
Compare
Some changes occurred in tests/ui/sanitizer cc @rcvalle |
@bors2 try jobs=aarch64-apple |
…try> tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok) Normally a `run-fail` ui test shall not be terminated by a signal like `SIGABRT`. So begin having that as a hard requirement. Some of our current tests do terminate by a signal however. Introduce and use ~`run-fail-without-exit-code`~ `run-crash` for those tests. This adds further (cc #142304, #142886) protection against the regression in #123733 since that bug also manifested as `SIGABRT` in `tests/ui/panics/panic-main.rs` (shown as `Aborted (core dumped)` in the logs attached to that issue, and I have also been able to reproduce this locally). ### TODO - [x] **Q:** what about on Windows? **A:** we'll treat any exit code outside of 1 - 127 as "crashed", and we'll do the same on unix. - [x] test all permutations of actual vs expected **Done:** See #143002 (comment). - [x] Handle targets without unwind support - [ ] Add `run-fail-or-crash` for some sanitizer tests ### Zulip discussion See https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/compiletest.3A.20terminate.20by.20signal.20vs.20exit.20with.20error/with/525611235 try-job: aarch64-apple // try-job: x86_64-msvc-1 // try-job: x86_64-gnu // try-job: dist-i586-gnu-i586-i686-musl // try-job: test-various try-job: aarch64-apple
b210f9a
to
d0fc6d2
Compare
Now running all @bors2 try |
…try> tests: Require `run-fail` ui tests to have an exit code (`SIGABRT` not ok) Normally a `run-fail` ui test shall not be terminated by a signal like `SIGABRT`. So begin having that as a hard requirement. Some of our current tests do terminate by a signal however. Introduce and use ~`run-fail-without-exit-code`~ `run-crash` for those tests. This adds further (cc #142304, #142886) protection against the regression in #123733 since that bug also manifested as `SIGABRT` in `tests/ui/panics/panic-main.rs` (shown as `Aborted (core dumped)` in the logs attached to that issue, and I have also been able to reproduce this locally). ### TODO - [x] **Q:** what about on Windows? **A:** we'll treat any exit code outside of 1 - 127 as "crashed", and we'll do the same on unix. - [x] test all permutations of actual vs expected **Done:** See #143002 (comment). - [x] Handle targets without unwind support - [ ] Add `run-fail-or-crash` for some sanitizer tests ### Zulip discussion See https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/compiletest.3A.20terminate.20by.20signal.20vs.20exit.20with.20error/with/525611235 try-job: aarch64-apple try-job: x86_64-msvc-1 try-job: x86_64-gnu try-job: dist-i586-gnu-i586-i686-musl try-job: test-various
c19e167
to
7914e18
Compare
The problems found by bors in the approved commit 4573c84 should be fixed now. Changes in the current commit compared to that commit:
Since the @rustbot ready Updated examples of failure messages:
How exit code 0 looks like for a
|
This comment was marked as resolved.
This comment was marked as resolved.
7914e18
to
5334e8a
Compare
This comment has been minimized.
This comment has been minimized.
5334e8a
to
7f2f79f
Compare
r=me with #143002 (comment) addressed. |
✌️ @Enselic, you can now approve this pull request! If @petrochenkov told you to " |
…t ok) And introduce two new directives for ui tests: * `run-crash` * `run-fail-or-crash` Normally a `run-fail` ui test like tests that panic shall not be terminated by a signal like `SIGABRT`. So begin having that as a hard requirement. Some of our current tests do terminate by a signal/crash however. Introduce and use `run-crash` for those tests. Note that Windows crashes are not handled by signals but by certain high bits set on the process exit code. Example exit code for crash on Windows: `0xc000001d`. Because of this, we define "crash" on all platforms as "not exit with success and not exit with a regular failure code in the range 1..=127". Some tests behave differently on different targets: * Targets without unwind support will abort (crash) instead of exit with failure code 101 after panicking. As a special case, allow crashes for `run-fail` tests for such targets. * Different sanitizer implementations handle detected memory problems differently. Some abort (crash) the process while others exit with failure code 1. Introduce and use `run-fail-or-crash` for such tests.
7f2f79f
to
6950daf
Compare
And introduce two new directives for ui tests:
run-crash
run-fail-or-crash
Normally a
run-fail
ui test like tests that panic shall not be terminated by a signal likeSIGABRT
. So begin having that as a hard requirement.Some of our current tests do terminate by a signal/crash however. Introduce and use
run-crash
for those tests. Note that Windows crashes are not handled by signals but by certain high bits set on the process exit code. Example exit code for crash on Windows:0xc000001d
(STATUS_ILLEGAL_INSTRUCTION
). Because of this, we define "crash" on all platforms as "not exit with success and not exit with a regular failure code in the range 1..=127".Some tests behave differently on different targets:
run-fail
tests for such targets.run-fail-or-crash
for such tests.This adds further (cc #142304, #142886) protection against the regression in #123733 since that bug also manifested as
SIGABRT
intests/ui/panics/panic-main.rs
(shown asAborted (core dumped)
in the logs attached to that issue, and I have also been able to reproduce this locally).TODO
A: we'll treat any exit code outside of 1 - 127 as "crashed", and we'll do the same on unix.
Done: See tests: Require
run-fail
ui tests to have an exit code (SIGABRT
not ok) #143002 (comment).run-fail-or-crash
for some sanitizer testsZulip discussion
See https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/compiletest.3A.20terminate.20by.20signal.20vs.20exit.20with.20error/with/525611235
try-job: aarch64-apple
try-job: x86_64-msvc-1
try-job: x86_64-gnu
try-job: dist-i586-gnu-i586-i686-musl
try-job: test-various