UI tests should fail if they ICE

[varkor]

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 .stderr file.

src/test/ui/issue-50585.rs currently fails with an ICE, but this is not reflected anywhere, for example.

[euclio]

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 the compiler unexpectedly panicked, etc., but this is more fragile.

[varkor]

@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?

[euclio]

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> .

[varkor]

Oh, that's a much better situation than I expected, thanks!