2025-04-11 Dealing with disabled tests

2025-04-11 Dealing with disabled tests Meeting proposal: https://github.com/rust-lang/compiler-team/issues/540 We sometimes disable tests because some other component we don't control (e.g. LLVM) is causing them to fail, for example: - powerpc64 [rust#99853](https://github.com/rust-lang/rust/issues/99853), then re-enabled in [compiler-bultins#792](compiler-builtins/pull/792) - ppc64le [rust#88520](https://github.com/rust-lang/rust/issues/88520), then re-anabled in [compiler-bultins#793](compiler-builtins/pull/793) - Disabled tests - find in `rustc` test disabled under ./tests - tests disabled in `compiler-builtins` under `./testcrate`: - [compiler-builtins#616](https://github.com/rust-lang/compiler-builtins/issues/616), ([comment](https://github.com/rust-lang/compiler-builtins/blob/9978a8b06b7c1b53a6c503a2bfe7aea9ba6ca98b/testcrate/tests/mul.rs#L126)) - From the [CHANGELOG.md](https://github.com/rust-lang/compiler-builtins/blob/master/CHANGELOG.md) > - Disable `f16` on AArch64 without the `neon` feature > - Disable f128 for amdgpu ([#737](https://github.com/rust-lang/compiler-builtins/pull/737)) > - Disable `f16` on platforms that have recursion problems > - Disable `f16` for LoongArch64 ([#722](https://github.com/rust-lang/compiler-builtins/pull/722)) - tests that should be deleted because disabled on all platform? [Comment](https://github.com/rust-lang/compiler-team/issues/540#issuecomment-1256428304) from @**lqd**: > Is the following in scope for that meeting: is it possible to ensure these optional components' tests are indeed executed on CI ? (Or is that more of a t-infra concern). I'm thinking of the needs-lld tests in particular, that we currently can run locally but not on CI. - NOTE(jieyouxu): I'm not actually sure who's maintaining `compiler-builtins`... maybe Trevor? I think `compiler-builtins` has its own CI test suites as well... - (apiraino): I think so by looking at the commit history, the [team assigned to it](https://github.com/rust-lang/team/blob/6ecf3d623a47942e09f4b11322baad38b78e3c3e/repos/rust-lang/compiler-builtins.toml#L8) is [crate-maintainers](https://github.com/rust-lang/team/blob/master/teams/crate-maintainers.toml) [Reporting test suite statistics post-merge](https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/Reporting.20test.20suite.20statistics.20after.20merge/with/500706741): - Jakub implemented a post-merge test metrics report that will show test diffs before and after changes from a PR. Still experimental, subject to improvements on precision. - Had to ignore doctest diffs because they can be very noisy and hard to distinguish between doctests in same file. --- > (jieyouxu: Feel free to rearrange/reword the following) > (apiraino: All the following could be imo great stuff to improve our docs about tests on the [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/tests/running.html)) Notes on test diversity: - We have a lot of tests in `rust-lang/rust` and `rust-lang/$crate`. Many `rust-lang/$crate` have their own test suites that get exercised in per-repo CI. - In `rust-lang/rust`, we run test suites of: - Tidy - compiletest-managed test suites under `tests/` - `cargotest` (tiny-`crater`-like thing that is a smoke test of a few selected crates), not to be confused with `cargo` - Test suites of individual tools (`miri`, `rust-analyzer`, `clippy`, `rustfmt`, `cargo`, ...) - Compiler crate tests - Library crate tests (`coretests`, `alloctests`, library doctests) - `error-index` - Self-tests of above mentioned tools Notes for compiletest-managed test suites under `tests/`: - Re. compiletest-managed test suites, the ones to watch out for are `//@ ignore-test` that are not support files. - Actionable: ensure `//@ known-bug: #123456` is used instead of `//@ ignore-test` if test is broken and thus disabled with a proper issue to track it, instead of `//@ ignore-test` which is sometimes used to ignore non-test/supporting files. - NOTE: I need to double check what `//@ known-debug` actually does in compiletest, I think it has slightly different meaning in crashes tests vs non-crashes tests. - NOTE: some tests are disabled because they can spuriously fail / are flakey. Those are particularly suspicious because they can be failing due to genuine problems. - Actionable: where suitable, introduce dedicated-purpose *capability*/*requirement*-based directives instead of using generic `//@ ignore-$target`s. - Examples: - Good: descriptive capability-based directives: `//@ needs-crate-type: dylib`. - Bad: generic directives, fragile on new targets / removed targets if ignore cause is about capacabity not the actual target: `//@ ignore-wasm (needs dylib)`. - Bad: overly broad `//@ ignore-xxx`: `//@ ignore-windows` when actually `//@ ignore-windows-gnu` is sufficient. - Mixed: depending on test intention, some tests may be unnecessarily overly constrained with `//@ only-xxx` directives (but only if coverage against multiple targets or whatever is actually meaningful regarding test intention). - Actionable (long term): possibly compiletest could be augmented to support queries on ignores (for instance, ask compiletest to report which tests are `//@ ignore-test` and which tests are ignored for `//@ only-$target`). - Actionable (long term): some analysis/checks on "impossible" combinations of test conditions (for instance, both `//@ needs-windows` and `//@ ignore-windows`) that can lead to tests that are never run. - Actionable (long term, very hard): figure out how to extend the `remote-test-{server,client}` scheme to allow running test binaries on the cross-compiled *target* that have more complicated configurations (in qemu/emulators and whatever), so that we don't need as many `//@ ignore-cross-compile`. - Meta: I'm not sure what kind of tooling can be used to track ignored tests... It'd be nice if there's like a dashboard of ignored tests but that's additional implementation work + maintenance so... - Note: there are also specific cases where it's possible to end up with a combination of requirements/ignores that cause the test to never run on *any* of our present CI runners. Those are arguably nasty because they can give a false sense of test coverage. Jakub has a script where they used to check `run-make` tests and found several that were never run in our set of CI jobs. - Some inconsistencies in the `//@ edition` tagging, will be fixed in [rust#139578](https://github.com/rust-lang/rust/pull/139578)