UI tests should fail if they ICE #51971
Comments
estebank
added
the
A-testsuite
|
This is because the status code for an ICE is the same as the status code for a regular compilation failure. When a UI test ICEs, as long as the correct error output is still present, it'll pass. I think the cleanest solution would be to change the status code of rustc in the event of an ICE, but that probably warrants a wider conversation. There's precedent for doing this in gcc. An alternative solution would be to scrape the output for |
This commit changes the exit status of rustc to 1 in the presence of compilation errors. In the event of an unexpected panic (ICE) the standard panic error exit status of 101 remains. A run-make test is added to ensure that the exit code does not regress, and compiletest is updated to check for an exit status of 1 or 101, depending on the mode and suite. This is a breaking change for custom drivers. Fixes rust-lang#51971.
overhaul exit codes for rustc and rustdoc This commit changes the exit status of rustc to 1 in the presence of compilation errors. In the event of an unexpected panic (ICE) the standard panic error exit status of 101 remains. A run-make test is added to ensure that the exit code does not regress, and compiletest is updated to check for an exit status of 1 or 101, depending on the mode and suite. This is a breaking change for custom drivers. Note that while changes were made to the rustdoc binary, there is no intended behavior change. rustdoc errors (i.e., failed lints) will still report 101. While this could *also* hide potential ICEs, I will leave that work to a future PR. Fixes #51971.
|
@euclio: is this issue actually fixed? The pull request #52197 claims to fix it, but it doesn't modify any tests — and I'd be very surprised if none of the existing test suite ICEs (I've recently fixed one, and I can't believe that was the only one). Am I right in thinking #52197 makes it possible to distinguish, but doesn't actually make ICEs fail tests? |
|
Yes, notice src/test/ui/issue-20801.rs
<https://github.com/rust-lang/rust/pull/52197/files#diff-6732283260b31d2c3fcb03177c2e9508>
currently ICEs under NLL, so I ignored it in my PR. I believe that's the
only test that ICEs currently.
…On Thu, Jul 19, 2018 at 3:42 PM varkor ***@***.***> wrote:
@euclio <https://github.com/euclio>: is this issue actually fixed? The
pull request #52197 <#52197> claims
to fix it, but it doesn't modify any tests — and I'd be very surprised if
none of the existing test suite ICEs (I've recently fixed one, and I can't
believe that was the only one). Am I right in thinking #52197
<#52197> makes it *possible* to
distinguish, but doesn't actually make ICEs fail tests?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#51971 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABTxFkbzehNocJ6VbKldbrANbXBLzouZks5uIOElgaJpZM4U-fNl>
.
|
|
Oh, that's a much better situation than I expected, thanks! |
Currently, ICEs are effectively ignored by UI tests. This means both that we can miss ICEs and that tests can change confusingly — it's only obvious that an ICE has occurred if the test is run manually. The output for ICEs in UI tests should be more obvious, and should cause the test to fail, regardless of the
.stderrfile.src/test/ui/issue-50585.rscurrently fails with an ICE, but this is not reflected anywhere, for example.The text was updated successfully, but these errors were encountered: