Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Open
pnkfelix opened this issue Apr 10, 2018 · 3 comments
Open

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

pnkfelix opened this issue Apr 10, 2018 · 3 comments
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. E-needs-design This issue needs exploration and design to see how and if we can fix/implement it E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Apr 10, 2018 •
edited
Loading

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 pnkfelix mentioned this issue Apr 10, 2018
Closed
pnkfelix added a commit to pnkfelix/rust that referenced this issue Apr 10, 2018
@pnkfelix
Workaround rust-lang#49855 by forcing rustc_error in any mode, includ…
1c8d2bd 
…ing NLL.

NOTE: I was careful to make each change in a manner that preserves the
existing diagnostic output (usually by ensuring that no lines were
added or removed). This means that the resulting source files are not
as nice to read as they were at the start. But we will have to review
these cases by hand anyway as follow-up work, so cleanup could
reasonably happen then (or not at all).
This was referenced Apr 10, 2018
Merged
Closed
bors added a commit that referenced this issue Apr 11, 2018
@bors
Auto merge of #49861 - pnkfelix:compare-mode-nll-followup-2, r=nikoma…
3242880 
…tsakis

Blindly checkpoint status of NLL mode ui tests

This takes the next (and potentially final?) step with #48879.

Namely, this PR got things to the point where I can successfully run `compiletest` on `src/test/ui` with `--compile-mode=nll`.

Here are the main pieces of it:

 1. To figure out how to even run `compiletest` normally on the ui directory, I ran `x.py test -vv`, and then looked for the `compiletest` invocation that mentioned `src/test/ui`.
 2. I took the aforementioned `compiletest` invocation and used it, adding `--compile-mode=nll` to the end. It had 170 failing cases.
 3. Due to #49855, I had to edit some of the tests so that they fail even under NLL, via `#[rustc_error]`. That's the first commit. (Then goto 2 to double-check no such tests remain.)
 4. I took the generated `build/target/test/foo.stderr` file for every case that failed, and blindly copied it to `src/test/foo.nll.stderr`. That's the second commit.
 5. Goto 2 until there were no failing cases.
 6. Remove any stamp files, and re-run `x.py test` to make sure that the edits and new `.nll.stderr` files haven't broken the pre-existing test suite.
@pietroalbini pietroalbini added A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) WG-infra-rustbuild labels Apr 11, 2018
@pnkfelix pnkfelix mentioned this issue Aug 28, 2018
Closed
43 tasks
@pnkfelix
Copy link
Member Author

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
Copy link
Member

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

@jieyouxu jieyouxu added the A-compiletest Area: The compiletest test runner label May 31, 2024
@jieyouxu jieyouxu moved this from Needs Triage / Backlog to Ready in compiletest maintenance and improvements Jun 9, 2024
@jieyouxu
Copy link
Member

compiletest triage: unclear what the current situation is, will need better understanding of compiler/types contributor workflows.

@jieyouxu jieyouxu added E-needs-design This issue needs exploration and design to see how and if we can fix/implement it E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status labels Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. E-needs-design This issue needs exploration and design to see how and if we can fix/implement it E-needs-investigation Call for partcipation: This issues needs some investigation to determine current status T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
compiletest maintenance and improvements
Status: Backlog
Milestone
No milestone
Development

No branches or pull requests
5 participants
@steveklabnik @pnkfelix @pietroalbini @Mark-Simulacrum @jieyouxu