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.

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

Cleanup: Migrate run-fail tests in src/test/ui from error-pattern to check-run-results #65865

Closed
3 tasks
tmiasko opened this issue Oct 27, 2019 · 4 comments
Closed
3 tasks
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tmiasko
Copy link
Contributor

tmiasko commented Oct 27, 2019

As tests are moved from src/test/run-fail to src/test/ui #65440 the meaning
of // error-pattern in ui tests becomes overloaded , since tests in
src/test/run-fail match error patterns to executable output while
src/test/ui tests match them to compiler output.

The changes in #65759 enable such overloaded use: if test is expected to
compile successfully and will be run, the patterns are matched to executable
output, otherwise patterns are matched to the compiler output.

@petrochenkov suggested that it might be preferable to migrate those test to use check-run-result instead.

  1. Decide if check-run-result is preferable over error-pattern for run-fail tests in ui.
  2. Migrate run fail tests to use check-run-result.
  3. Remove support for overloaded meaning of error-pattern.
@jonas-schievink jonas-schievink added A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. labels Oct 27, 2019
@JohnTitor JohnTitor added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 16, 2020
@petrochenkov
Copy link
Contributor

Another option is to do "check-run-results" automatically for all run-* tests and create the .run.stderr / .run.stdout files only if the output is not empty.

RalfJung added a commit to RalfJung/rust that referenced this issue May 9, 2020
Zalathar added a commit to Zalathar/rust that referenced this issue Dec 29, 2024
Use `check-run-results` for `run-fail` test stderr

Fixes rust-lang#65865
Zalathar added a commit to Zalathar/rust that referenced this issue Dec 29, 2024
Use `check-run-results` for `run-fail` test stderr

Fixes rust-lang#65865
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 29, 2024
Use `check-run-results` for `run-fail` test stderr

Fixes rust-lang#65865
Zalathar added a commit to Zalathar/rust that referenced this issue Dec 29, 2024
Use `check-run-results` for `run-fail` test stderr

Fixes rust-lang#65865
@jieyouxu jieyouxu added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-compiletest Area: The compiletest test runner labels Dec 29, 2024
@clubby789
Copy link
Contributor

Based on the results of #133460, I'm not sure how worth it this is, as I think tests where we care about the exact stderr are fairly rare. We would definitely need a lot more normalization before doing this automatically

@jieyouxu
Copy link
Member

compiletest triage: I'm not convinced switching error-pattern run-fail ui tests to use only the current form of check-run-results is an improvement over error-pattern unconditionally. If anything, it would be a regression to flaky tests in many cases that are sensitive to exact run stdout/stderr (line numbers, backtraces that can depend on platform-specific unwind mechanism, concrete addresses, version numbers, etc.).

I would be in favor of auditing some of these run-fail tests and sparingly add check-run-results if the exact run stderr/stdout is critical for the intention of the test, but not in a blanket fashion. As such, I'm going to close this issue in favor of a more concrete audit issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Development

Successfully merging a pull request may close this issue.

6 participants