-
Notifications
You must be signed in to change notification settings - Fork 61
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
ui test with ICE does not fail #169
Comments
Okay, so the reason that "normalized stderr" and "stderr" are so different is about something with JSON stuff... if there is no So, when there is a test that expects empty stderr, and that test ICEs, compiletest normalizes the stderr to empty and considers the test passed. |
@laumann looks like I am still not sure though if the behavior around normalization and JSON is intended. This all looks terribly fragile... |
FWIW, it is only due to this bug that Miri currently looks green in the toolstate tracking. Actually, some of the tests fail with an ICE since rust-lang/rust#59500 landed. |
I'm not sure if that works for Miri, but in Clippy I'm currently using |
I guess Miri could do that, but we don't actually want to run these things so that seems like a waste of test time. See #172 / rust-lang/rust#59769 for a possible fix. |
Handling the integer parsing properly instead of just unwrapping. Note that the test is not catching the ICE because plain UI tests [currently hide ICEs][compiletest_issue]. Once that issue is fixed, this test would fail properly again. [compiletest_issue]: Manishearth/compiletest-rs#169
Fix ICE in decimal_literal_representation lint Handling the integer parsing properly instead of just unwrapping. Note that the test is not catching the ICE because plain UI tests [currently hide ICEs][compiletest_issue]. Once that issue is fixed, this test would fail properly again. Fixes #3891 [compiletest_issue]: Manishearth/compiletest-rs#169
… r=alexcrichton compiletest normalization: preserve non-JSON lines such as ICEs Currently, every non-JSON line from stderr gets normalized away when compiletest normalizes the output. In particular, ICEs get normalized to the empty output. That does not seem desirable, so this changes normalization to preserve non-JSON lines instead. Also see Manishearth/compiletest-rs#169: because of that bug, Miri currently *looks* green in the toolstate, but some tests ICE. That same bug is likely no longer present in latest compiletest because the error code gets checked separately, but it still seems like a good idea to also make sure that ICEs are considered stderr output: This change found an accidental user-visible `error!` in CTFE validation (fixed), and a non-deterministic panic when there are two `main` symbols (not fixed, no idea where this comes from). Both got missed before because non-JSON output got ignored.
… r=alexcrichton compiletest normalization: preserve non-JSON lines such as ICEs Currently, every non-JSON line from stderr gets normalized away when compiletest normalizes the output. In particular, ICEs get normalized to the empty output. That does not seem desirable, so this changes normalization to preserve non-JSON lines instead. Also see Manishearth/compiletest-rs#169: because of that bug, Miri currently *looks* green in the toolstate, but some tests ICE. That same bug is likely no longer present in latest compiletest because the error code gets checked separately, but it still seems like a good idea to also make sure that ICEs are considered stderr output: This change found an accidental user-visible `error!` in CTFE validation (fixed), and a non-deterministic panic when there are two `main` symbols (not fixed, no idea where this comes from). Both got missed before because non-JSON output got ignored.
…hton compiletest normalization: preserve non-JSON lines such as ICEs Currently, every non-JSON line from stderr gets normalized away when compiletest normalizes the output. In particular, ICEs get normalized to the empty output. That does not seem desirable, so this changes normalization to preserve non-JSON lines instead. Also see Manishearth/compiletest-rs#169: because of that bug, Miri currently *looks* green in the toolstate, but some tests ICE. That same bug is likely no longer present in latest compiletest because the error code gets checked separately, but it still seems like a good idea to also make sure that ICEs are considered stderr output: This change found an accidental user-visible `error!` in CTFE validation (fixed), and a non-deterministic panic when there are two `main` symbols (not fixed, no idea where this comes from). Both got missed before because non-JSON output got ignored.
…hton compiletest normalization: preserve non-JSON lines such as ICEs Currently, every non-JSON line from stderr gets normalized away when compiletest normalizes the output. In particular, ICEs get normalized to the empty output. That does not seem desirable, so this changes normalization to preserve non-JSON lines instead. Also see Manishearth/compiletest-rs#169: because of that bug, Miri currently *looks* green in the toolstate, but some tests ICE. That same bug is likely no longer present in latest compiletest because the error code gets checked separately, but it still seems like a good idea to also make sure that ICEs are considered stderr output: This change found an accidental user-visible `error!` in CTFE validation (fixed), and a non-deterministic panic when there are two `main` symbols (not fixed, no idea where this comes from). Both got missed before because non-JSON output got ignored.
Awesome. :) How much work would it be to make a release with this fix? |
…hton compiletest normalization: preserve non-JSON lines such as ICEs Currently, every non-JSON line from stderr gets normalized away when compiletest normalizes the output. In particular, ICEs get normalized to the empty output. That does not seem desirable, so this changes normalization to preserve non-JSON lines instead. Also see Manishearth/compiletest-rs#169: because of that bug, Miri currently *looks* green in the toolstate, but some tests ICE. That same bug is likely no longer present in latest compiletest because the error code gets checked separately, but it still seems like a good idea to also make sure that ICEs are considered stderr output: This change found an accidental user-visible `error!` in CTFE validation (fixed), and a non-deterministic panic when there are two `main` symbols (not fixed, no idea where this comes from). Both got missed before because non-JSON output got ignored.
We are using this in Miri, and I just have the situation that one of our UI tests ICEs when I call it directly but the test is considered passing by compiletest. I have copy-pasted the command-line (as it gets printed when I make the test really fail, e.g. by introducing a syntax error) to make sure it is not some weird extra flag.
On a possibly related note, when I run the test binary directly (
target/debug/deps/compiletest-edd6bd72083f7b79
), all tests should fail because Miri complains about a missing shared library. Instead, only those tests with a non-emptystdout
/stderr
reference file fail, complaining thatstdout
/stderr
was empty instead. This is particularly curious sincestderr
is not actually empty:Compare "normalized stderr" and "stderr".
I have no idea how this is possible.
The text was updated successfully, but these errors were encountered: