Fix compiletest so it respects warnings for run-pass. #35674
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
Fixes #35165 |
|
#35165 (comment) claims that some other PRs must land before this can go in. Which PRs did you have in mind? |
| @@ -992,7 +997,7 @@ actual:\n\ | |||
| fn check_expected_errors(&self, | |||
| expected_errors: Vec<errors::Error>, | |||
| proc_res: &ProcRes) { | |||
| if proc_res.status.success() { | |||
| if proc_res.status.success() && expected_errors.iter().any(|x| x.kind == Some(ErrorKind::Error)) { | |||
There was a problem hiding this comment.
I think we may want to thread through some more information here, this means a compile-fail test which only checked for warnings would pass if compilation succeeded, I believe.
Perhaps whether or not proc_res.status.success() should be checked could be passed in or lifted out?
There was a problem hiding this comment.
This change is technically correct, in that check_expected_errors is only called from one place before the change, run_cfail_test and it checks for compile failure explicitly.
The code currently requires a compilation failure only if there is an error comment in the file. Note, this isn't currently useful other than ensuring that run-pass tests don't require that errors happen. And in that sense, it's a short-circuit, since it would probably result in the match failing below. Though, it does catch the unlikely failure mode where the compiler reports an error but doesn't return a failure code.
|
Looks like tidy has an error on travis as well. Thanks for the PR though, looks good to me! |
|
The tidy is fixed. |
|
This means that we could check in a compile-fail tests with only |
|
Er, in that it would pass if the compiler succeeded to compile. I would expect all compile-fail tests to assert that the compile fails. |
|
The Since I'm having a hard time explaining this, I'll point to the relevant code: https://github.com/rust-lang/rust/blob/master/src/tools/compiletest/src/runtest.rs#L132-L136 This is what ensures that cfail tests require a compilation failure. It's the first thing that's checked after running the compiler. It won't call |
Fix compiletest so it respects warnings for run-pass.
|
💔 Test failed - auto-win-gnu-64-opt |
|
I can't repro the failure locally. Is this test only run on a specific target or is it run on all targets but only fails on this one? |
|
@alexcrichton How do I try this again? |
|
Can I ask bors to try again or only approved reviewers? |
|
Ah no only approved reviewers can |
Fix compiletest so it respects warnings for run-pass.
No description provided.