search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
94.93k stars 12.24k forks source link

compiletest: compare-mode cannot handle mixed success + failure #49855

Open pnkfelix opened 6 years ago

pnkfelix commented 6 years ago

Sub-issue of #48879

Currently, if you have a ui test src/test/ui/foo.rs that signals a diagnostic error under the AST borrowck, but has no error under the NLL borrowck, then there is no way to make a foo.nll.stderr or foo.nll.stdout such that running compiletest --compare-mode=nll will accept it.

The reason: compiletest complains that the compilation itself was successful, and by default, all ui tests are assumed to signal a compilation failure.

The usual way such a case (of a test that compiles without an error) is handled in compiletest is to add a comment of the form // run-pass in the test itself. But we do not want the tests to be marked as always passing; they only pass under the special mode (NLL in this case) that is still under development in the compiler itself.

There are a number of reasonable ways of addressing this.

  1. We could modify such tests to ensure that they end with a function their fn main is tagged with #[rustc_error], so that they will never successfully compile.
  2. We could attempt to have some way to encode a mode-predicated run-pass marker, e.g. // [nll] run-pass.

  3. We could have compiletest infer that a test, when running under compare-mode, must have an implicit // run-pass marker, if there exists a foo.mode.stdout file (even an empty one).

    I'm not such a big fan of options 1 or 2 because I like the fact that (so far) compare-mode does not have to impact the existing .rs nor .stderr/.stdout files; you just add your alternative .nll.stderr file and no one else is the wiser.

So currently I would like us to adopt option 3 (or some other option I haven't thought of yet) that allows the .rs/.stderr/.stdout to go on oblivious to the existence of compare-mode.

But, in the short term, to let us get work done, I plan to follow option 1 for all existing ui/ tests. (This is intended to be a temporary hack; all such uses of this hack should point back to this issue.)

pnkfelix commented 5 years ago

GIven that the compiler team is talking about broader revisions to our test suites (e.g. turning them all into ui tests, and more consistently embedding meta-data like // run-pass into the headers of the tests themselves), I am now more inclined towards adopting option 2 above to resolve this in the long term:

  1. We could attempt to have some way to encode a mode-predicated run-pass marker, e.g. // [nll] run-pass.
steveklabnik commented 4 years ago

Triage; not aware of changes here, though this is a part of the repo that I'm not particularly familiar with.