Skip to content

Removed outdated ui test suite README, give reasons for disabled tests #139705

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

Merged
merged 2 commits into from
Apr 13, 2025

Conversation

jieyouxu
Copy link
Member

Changes

  • tests/ui/README.md is very outdated, test suite docs should consistently live in rustc-dev-guide.
  • Add reasons for //@ ignore-test tests that don't have one.1

This is a precursor change to make follow-up changes easier (possibly more specialized directives or converting some auxiliaries into the canonical auxiliary/ form).

Footnotes

  1. searched via rg --no-ignore -F -e "ignore-test" tests/.

Verified

This commit was signed with the committer’s verified signature.
jieyouxu 许杰友 Jieyou Xu (Joe)
We should consolidate our test suite docs in rustc-dev-guide, and this
README is very outdated.

Verified

This commit was signed with the committer’s verified signature.
jieyouxu 许杰友 Jieyou Xu (Joe)
@jieyouxu
Copy link
Member Author

Hm, is triagebot down 🤔
@rustbot label +T-compiler +A-testsuite
@rustbot ready
r? compiler

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 12, 2025
@Zalathar
Copy link
Contributor

Note that compiletest will also ignore any directory containing a file named compiletest-ignore-dir, which could be useful in some cases.

// Ignore directories that contain a file named `compiletest-ignore-dir`.
if dir.join("compiletest-ignore-dir").exists() {
return Ok(());
}

@compiler-errors
Copy link
Member

I think it's fine to mark precondition-checks files individually as unimplemented, b/c they make become implemented separately. Surprised we even have compiletest-ignore-dir 🤔 probably for different purposes than ignoring test failures (but instead, e.g., for file layout reasons).

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 12, 2025

📌 Commit db6e3aa has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 12, 2025
@jieyouxu
Copy link
Member Author

jieyouxu commented Apr 12, 2025

Oh I should've mentioned, I saw some of the precondition-check tests under the same dir active, which is why I didn't ignore that whole directory. (I thought the ignore dir comment was a general remark.)

See https://github.com/rust-lang/rust/tree/master/tests%2Fui%2Fprecondition-checks, only these few precondition-check tests (that is, the ones changed in this PR) aren't implemented yet.

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 13, 2025
…enton

Rollup of 8 pull requests

Successful merges:

 - rust-lang#139163 (indirect-const-stabilize the `exact_div` intrinsic)
 - rust-lang#139276 (Revert "Disable `f16` on Aarch64 without `neon`")
 - rust-lang#139315 (Switch `time` to `jiff` for time formatting in ICE dumps)
 - rust-lang#139382 (Update windows-bindgen to 0.61.0)
 - rust-lang#139688 (rustdoc-search: add unbox flag to Result aliases)
 - rust-lang#139701 (docs: clarify uint exponent for `is_power_of_two`)
 - rust-lang#139705 (Removed outdated ui test suite README, give reasons for disabled tests)
 - rust-lang#139713 (Fix typo in documentation)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 13, 2025
…enton

Rollup of 8 pull requests

Successful merges:

 - rust-lang#139163 (indirect-const-stabilize the `exact_div` intrinsic)
 - rust-lang#139276 (Revert "Disable `f16` on Aarch64 without `neon`")
 - rust-lang#139315 (Switch `time` to `jiff` for time formatting in ICE dumps)
 - rust-lang#139382 (Update windows-bindgen to 0.61.0)
 - rust-lang#139688 (rustdoc-search: add unbox flag to Result aliases)
 - rust-lang#139701 (docs: clarify uint exponent for `is_power_of_two`)
 - rust-lang#139705 (Removed outdated ui test suite README, give reasons for disabled tests)
 - rust-lang#139713 (Fix typo in documentation)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0e50fda into rust-lang:master Apr 13, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 13, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 13, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Rollup merge of rust-lang#139705 - jieyouxu:tests-precursor, r=compiler-errors

Removed outdated ui test suite README, give reasons for disabled tests

### Changes

- `tests/ui/README.md` is very outdated, test suite docs should consistently live in rustc-dev-guide.
- Add reasons for `//@ ignore-test` tests that don't have one.[^query]

This is a precursor change to make follow-up changes easier (possibly more specialized directives or converting some auxiliaries into the canonical `auxiliary/` form).

[^query]: searched via `rg --no-ignore -F -e "ignore-test" tests/`.
@jieyouxu jieyouxu deleted the tests-precursor branch April 13, 2025 07:25
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 14, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Use `compiletest-ignore-dir` for bootstrap self-tests

Follow-up to rust-lang#139705 and rust-lang#139740.

I did another survey pass over `//@ ignore-test` under `tests/`, and this is the only 2 non-tests that should use `compiletest-ignore-dir`.

r? `@Zalathar` (or compiler/bootstrap)
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 15, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Use `compiletest-ignore-dir` for bootstrap self-tests

Follow-up to rust-lang#139705 and rust-lang#139740.

I did another survey pass over `//@ ignore-test` under `tests/`, and this is the only 2 non-tests that should use `compiletest-ignore-dir`.

r? ``@Zalathar`` (or compiler/bootstrap)
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Apr 15, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Use `compiletest-ignore-dir` for bootstrap self-tests

Follow-up to rust-lang#139705 and rust-lang#139740.

I did another survey pass over `//@ ignore-test` under `tests/`, and this is the only 2 non-tests that should use `compiletest-ignore-dir`.

r? ```@Zalathar``` (or compiler/bootstrap)
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Rollup merge of rust-lang#139783 - jieyouxu:ignore-dir, r=Zalathar

Use `compiletest-ignore-dir` for bootstrap self-tests

Follow-up to rust-lang#139705 and rust-lang#139740.

I did another survey pass over `//@ ignore-test` under `tests/`, and this is the only 2 non-tests that should use `compiletest-ignore-dir`.

r? `@Zalathar` (or compiler/bootstrap)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 17, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Introduce and use specialized `//@ ignore-auxiliary` for test support files instead of using `//@ ignore-test`

### Summary

Add a semantically meaningful directive for ignoring test *auxiliary* files. This is for auxiliary files that *participate* in actual tests but should not be built by `compiletest` (i.e. these files are involved through `mod xxx;` or `include!()` or `#[path = "xxx"]`, etc.).

### Motivation

A specialized directive like `//@ ignore-auxiliary` makes it way easier to audit disabled tests via `//@ ignore-test`.
  - These support files cannot use the canonical `auxiliary/` dir because they participate in module resolution or are included, or their relative paths can be important for test intention otherwise.

Follow-up to:
- rust-lang#139705
- rust-lang#139783
- rust-lang#139740

See also discussions in:

- [#t-compiler > Directive name for non-test aux files?](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Directive.20name.20for.20non-test.20aux.20files.3F/with/512773817)
- [#t-compiler > Handling disabled &rust-lang#96;//@ ignore-test&rust-lang#96; tests](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Handling.20disabled.20.60.2F.2F.40.20ignore-test.60.20tests/with/512005974)
- [#t-compiler/meetings > &rust-lang#91;steering&rust-lang#93; 2025-04-11 Dealing with disabled tests](https://rust-lang.zulipchat.com/#narrow/channel/238009-t-compiler.2Fmeetings/topic/.5Bsteering.5D.202025-04-11.20Dealing.20with.20disabled.20tests/with/511717981)

### Remarks on remaining unconditionally disabled tests under `tests/`

After this PR, against commit 79a272c, only **14** remaining test files are disabled through `//@ ignore-test`:

<details>
<summary>Remaining `//@ ignore-test` files under `tests/`</summary>

```
tests/debuginfo/drop-locations.rs
4://@ ignore-test (broken, see rust-lang#128971)

tests/rustdoc/macro-document-private-duplicate.rs
1://@ ignore-test (fails spuriously, see issue rust-lang#89228)

tests/rustdoc/inline_cross/assoc-const-equality.rs
3://@ ignore-test (FIXME: rust-lang#125092)

tests/ui/match/issue-27021.rs
7://@ ignore-test (rust-lang#54987)

tests/ui/match/issue-26996.rs
7://@ ignore-test (rust-lang#54987)

tests/ui/issues/issue-49298.rs
9://@ ignore-test (rust-lang#54987)

tests/ui/issues/issue-59756.rs
2://@ ignore-test (rustfix needs multiple suggestions)

tests/ui/precondition-checks/write.rs
5://@ ignore-test (unimplemented)

tests/ui/precondition-checks/read.rs
5://@ ignore-test (unimplemented)

tests/ui/precondition-checks/write_bytes.rs
5://@ ignore-test (unimplemented)

tests/ui/explicit-tail-calls/drop-order.rs
2://@ ignore-test: tail calls are not implemented in rustc_codegen_ssa yet, so this causes 🧊

tests/ui/panics/panic-short-backtrace-windows-x86_64.rs
3://@ ignore-test (rust-lang#92000)

tests/ui/json/json-bom-plus-crlf-multifile-aux.rs
3://@ ignore-test Not a test. Used by other tests

tests/ui/traits/next-solver/object-soundness-requires-generalization.rs
2://@ ignore-test (see rust-lang#114196)
```
</details>

Of these, most are either **unimplemented**, or **spurious**, or **known-broken**. The outstanding one is `tests/ui/json/json-bom-plus-crlf-multifile-aux.rs` which I did not want to touch in *this* PR -- that aux file has load-bearing BOM and carriage returns and byte offset matters. I think those test files that require special encoding / BOM probably are better off as `run-make` tests. See rust-lang#139968 for that aux file.

### Review advice

- Best reviewed commit-by-commit.
- The directive name diverged from the most voted `//@ auxiliary` because I think that's easy to confuse with `//@ aux-{crate,dir}`.

r? compiler
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 18, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Rollup merge of rust-lang#139967 - jieyouxu:auxiliary, r=wesleywiser

Introduce and use specialized `//@ ignore-auxiliary` for test support files instead of using `//@ ignore-test`

### Summary

Add a semantically meaningful directive for ignoring test *auxiliary* files. This is for auxiliary files that *participate* in actual tests but should not be built by `compiletest` (i.e. these files are involved through `mod xxx;` or `include!()` or `#[path = "xxx"]`, etc.).

### Motivation

A specialized directive like `//@ ignore-auxiliary` makes it way easier to audit disabled tests via `//@ ignore-test`.
  - These support files cannot use the canonical `auxiliary/` dir because they participate in module resolution or are included, or their relative paths can be important for test intention otherwise.

Follow-up to:
- rust-lang#139705
- rust-lang#139783
- rust-lang#139740

See also discussions in:

- [#t-compiler > Directive name for non-test aux files?](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Directive.20name.20for.20non-test.20aux.20files.3F/with/512773817)
- [#t-compiler > Handling disabled &rust-lang#96;//@ ignore-test&rust-lang#96; tests](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Handling.20disabled.20.60.2F.2F.40.20ignore-test.60.20tests/with/512005974)
- [#t-compiler/meetings > &rust-lang#91;steering&rust-lang#93; 2025-04-11 Dealing with disabled tests](https://rust-lang.zulipchat.com/#narrow/channel/238009-t-compiler.2Fmeetings/topic/.5Bsteering.5D.202025-04-11.20Dealing.20with.20disabled.20tests/with/511717981)

### Remarks on remaining unconditionally disabled tests under `tests/`

After this PR, against commit 79a272c, only **14** remaining test files are disabled through `//@ ignore-test`:

<details>
<summary>Remaining `//@ ignore-test` files under `tests/`</summary>

```
tests/debuginfo/drop-locations.rs
4://@ ignore-test (broken, see rust-lang#128971)

tests/rustdoc/macro-document-private-duplicate.rs
1://@ ignore-test (fails spuriously, see issue rust-lang#89228)

tests/rustdoc/inline_cross/assoc-const-equality.rs
3://@ ignore-test (FIXME: rust-lang#125092)

tests/ui/match/issue-27021.rs
7://@ ignore-test (rust-lang#54987)

tests/ui/match/issue-26996.rs
7://@ ignore-test (rust-lang#54987)

tests/ui/issues/issue-49298.rs
9://@ ignore-test (rust-lang#54987)

tests/ui/issues/issue-59756.rs
2://@ ignore-test (rustfix needs multiple suggestions)

tests/ui/precondition-checks/write.rs
5://@ ignore-test (unimplemented)

tests/ui/precondition-checks/read.rs
5://@ ignore-test (unimplemented)

tests/ui/precondition-checks/write_bytes.rs
5://@ ignore-test (unimplemented)

tests/ui/explicit-tail-calls/drop-order.rs
2://@ ignore-test: tail calls are not implemented in rustc_codegen_ssa yet, so this causes 🧊

tests/ui/panics/panic-short-backtrace-windows-x86_64.rs
3://@ ignore-test (rust-lang#92000)

tests/ui/json/json-bom-plus-crlf-multifile-aux.rs
3://@ ignore-test Not a test. Used by other tests

tests/ui/traits/next-solver/object-soundness-requires-generalization.rs
2://@ ignore-test (see rust-lang#114196)
```
</details>

Of these, most are either **unimplemented**, or **spurious**, or **known-broken**. The outstanding one is `tests/ui/json/json-bom-plus-crlf-multifile-aux.rs` which I did not want to touch in *this* PR -- that aux file has load-bearing BOM and carriage returns and byte offset matters. I think those test files that require special encoding / BOM probably are better off as `run-make` tests. See rust-lang#139968 for that aux file.

### Review advice

- Best reviewed commit-by-commit.
- The directive name diverged from the most voted `//@ auxiliary` because I think that's easy to confuse with `//@ aux-{crate,dir}`.

r? compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants