Skip to content

Commit 9d59e52

Browse files
Brezakoli-obkBD103
authored
Switch to ui_test in compile fail tests. (#12810)
# Objective Make compile fail tests less likely to break with new Rust versions. Closes #12627 ## Solution Switch from [`trybuild`](https://github.com/dtolnay/trybuild) to [`ui_test`](https://github.com/oli-obk/ui_test). ## TODO - [x] Update `bevy_ecs_compile_fail_tests` - [x] Update `bevy_macros_compile_fail_tests` - [x] Update `bevy_reflect_compile_fail_tests` --------- Co-authored-by: Oli Scherer <github35764891676564198441@oli-obk.de> Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>
1 parent de9dc9c commit 9d59e52

107 files changed

Lines changed: 885 additions & 416 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/release.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,5 @@ jobs:
5252
base: "main"
5353
title: "Preparing Next Release"
5454
body: |
55-
Preparing next release
56-
This PR has been auto-generated
55+
Preparing next release. This PR has been auto-generated.
56+
UI tests have not been automatically bumped to the latest version, please fix them manually.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ exclude = [
1919
"crates/bevy_ecs_compile_fail_tests",
2020
"crates/bevy_macros_compile_fail_tests",
2121
"crates/bevy_reflect_compile_fail_tests",
22+
"crates/bevy_compile_test_utils",
2223
]
2324
members = [
2425
"crates/*",
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
[package]
2+
name = "bevy_compile_test_utils"
3+
edition = "2021"
4+
description = "Utils for compile tests used in the engine"
5+
homepage = "https://bevyengine.org"
6+
repository = "https://github.com/bevyengine/bevy"
7+
license = "MIT OR Apache-2.0"
8+
publish = false
9+
10+
[dependencies]
11+
ui_test = "0.22.3"
12+
13+
[[test]]
14+
name = "example"
15+
harness = false
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# Helpers for compile fail tests
2+
3+
This crate contains everything needed to set up compile tests for the Bevy repo. It, like all Bevy compile test crates, is excluded from the Bevy workspace. This is done to not fail [`crater` tests](https://github.com/rust-lang/crater) for Bevy. The `CI` workflow executes these tests on the stable rust toolchain see ([tools/ci](../../tools/ci/src/main.rs)).
4+
5+
## Writing new test cases
6+
7+
Test cases are annotated .rs files. These annotations define how the test case should be run and what we're expecting to fail. Please see <https://github.com/oli-obk/ui_test/blob/main/README.md> for more information.
8+
9+
Annotations can roughly be split into global annotations which are prefixed with `//@` and define how tests should be run and error annotations which are prefixed with `//~` and define where errors we expect to happen. From the global annotations, you're only likely to care about `//@check-pass` which will make any compile errors in the test trigger a test failure.
10+
11+
The error annotations are composed of two parts.
12+
An optional location specifier:
13+
14+
- `^` The error happens on the line above.
15+
- `v` The error happens on the line below.
16+
- `|` The error annotation is connected to another one.
17+
- If the location specifier is missing, the error is assumed to happen on the same line as the annotation.
18+
19+
An error matcher:
20+
21+
- `E####` The error we're expecting has the [`####` rustc error code](https://doc.rust-lang.org/error_codes/error-index.html), e.g `E0499`
22+
- `<lint_name>` The given [compiler lint](https://doc.rust-lang.org/rustc/lints/index.html) is triggered, e.g. `dead_code`
23+
- `LEVEL: <substring>` A compiler error of the given level (valid levels are: `ERROR`, `HELP`, `WARN` or `NOTE`) will be raised and it will contain the `substring`. Substrings can contain spaces.
24+
- `LEVEL: /<regex>/` Same as above but a regex is used to match the error message.
25+
26+
An example of an error annotation would be `//~v ERROR: missing trait`. This error annotation will match any error occurring on the line below that contains the substring `missing trait`.
27+
28+
## Adding compile fail tests for a crate that doesn't have them
29+
30+
This will be a rather involved process. You'll have to:
31+
32+
- Create an empty library crate in the [`crates`](..) directory.
33+
- Add this crate as a `dev-dependency`.
34+
- Create a folder called `tests` within the new crate.
35+
- Add a test runner file to this folder. The file should contain a main function calling one of the test functions defined in this crate.
36+
- Add a `[[test]]` table to the `Cargo.toml`. This table will need to contain `harness = false` and `name = <name of the test runner file you defined>`.
37+
- Modify the [`Ci`](../../tools/ci/) tool to run `cargo test` on this crate.
38+
- And finally, write your compile tests.
39+
40+
If you have any questions, don't be scared to ask for help.
41+
42+
## A note about `.stderr` files
43+
44+
We're capable of generating `.stderr` files for all our compile tests. These files contain the error output generated by the test. To create or regenerate them yourself, trigger the tests with the `BLESS` environment variable set to any value (e.g. `BLESS="some symbolic value"`). We currently have to ignore mismatches between these files and the actual stderr output from their corresponding test due to issues with file paths. We attempt to sanitize file paths but for proc-macros, the compiler error messages contain file paths to the current toolchain's copy of the standard library. If we knew of a way to construct a path to the current toolchains folder we could fix this.
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
use std::{
2+
env,
3+
path::{Path, PathBuf},
4+
};
5+
6+
// Re-export ui_test so all the tests use the same version.
7+
pub use ui_test;
8+
9+
use ui_test::{
10+
default_file_filter, default_per_file_config, run_tests_generic,
11+
status_emitter::{Gha, StatusEmitter, Text},
12+
Args, Config, OutputConflictHandling,
13+
};
14+
15+
/// Use this instead of hand rolling configs.
16+
///
17+
/// `root_dir` is the directory your tests are contained in. Needs to be a path from crate root.
18+
fn basic_config(root_dir: impl Into<PathBuf>, args: &Args) -> Config {
19+
let mut config = Config {
20+
dependencies_crate_manifest_path: Some("Cargo.toml".into()),
21+
bless_command: Some("`cargo test` with the BLESS environment variable set to any non empty value".to_string()),
22+
output_conflict_handling: if env::var_os("BLESS").is_some() {
23+
OutputConflictHandling::Bless
24+
} else {
25+
// stderr output changes between rust versions so we just rely on annotations
26+
OutputConflictHandling::Ignore
27+
},
28+
..Config::rustc(root_dir)
29+
};
30+
31+
config.with_args(args);
32+
33+
let bevy_root = "..";
34+
35+
// Don't leak contributor filesystem paths
36+
config.path_stderr_filter(Path::new(bevy_root), b"$BEVY_ROOT");
37+
config.path_stderr_filter(Path::new(env!("RUSTUP_HOME")), b"$RUSTUP_HOME");
38+
39+
// ui_test doesn't compile regex with perl character classes.
40+
// \pL = unicode class for letters, \pN = unicode class for numbers
41+
config.stderr_filter(r"\/home\/[\pL\pN_@#\-\. ]+", "$HOME");
42+
// Paths in .stderr seem to always be normalized to use /. Handle both anyway.
43+
config.stderr_filter(
44+
r"[a-zA-Z]:(?:\\|\/)users(?:\\|\/)[\pL\pN_@#\-\. ]+", // NOTE: [\pL\pN_@#\-\. ] is a poor attempt at handling usernames
45+
"$HOME",
46+
);
47+
48+
config
49+
}
50+
51+
/// Runs ui tests for a single directory.
52+
///
53+
/// `root_dir` is the directory your tests are contained in. Needs to be a path from crate root.
54+
pub fn test(test_root: impl Into<PathBuf>) -> ui_test::Result<()> {
55+
test_multiple([test_root])
56+
}
57+
58+
/// Run ui tests with the given config
59+
pub fn test_with_config(config: Config) -> ui_test::Result<()> {
60+
test_with_multiple_configs([config])
61+
}
62+
63+
/// Runs ui tests for a multiple directories.
64+
///
65+
/// `root_dirs` paths need to come from crate root.
66+
pub fn test_multiple(
67+
test_roots: impl IntoIterator<Item = impl Into<PathBuf>>,
68+
) -> ui_test::Result<()> {
69+
let args = Args::test()?;
70+
71+
let configs = test_roots.into_iter().map(|root| basic_config(root, &args));
72+
73+
test_with_multiple_configs(configs)
74+
}
75+
76+
/// Run ui test with the given configs.
77+
///
78+
/// Tests for configs are run in parallel.
79+
pub fn test_with_multiple_configs(
80+
configs: impl IntoIterator<Item = Config>,
81+
) -> ui_test::Result<()> {
82+
let configs = configs.into_iter().collect();
83+
84+
let emitter: Box<dyn StatusEmitter + Send> = if env::var_os("CI").is_some() {
85+
Box::new((
86+
Text::verbose(),
87+
Gha::<true> {
88+
name: env!("CARGO_PKG_NAME").to_string(),
89+
},
90+
))
91+
} else {
92+
Box::new(Text::quiet())
93+
};
94+
95+
run_tests_generic(
96+
configs,
97+
default_file_filter,
98+
default_per_file_config,
99+
emitter,
100+
)
101+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
fn main() -> bevy_compile_test_utils::ui_test::Result<()> {
2+
// Run all tests in the tests/example_tests folder.
3+
// If we had more tests we could either call this function
4+
// on everysingle one or use test_multiple and past it an array
5+
// of paths.
6+
//
7+
// Don't forget that when running tests the working directory
8+
// is set to the crate root.
9+
bevy_compile_test_utils::test("tests/example_tests")
10+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Compiler warnings also need to be annotated. We don't
2+
// want to annotate all the unused variables so let's instruct
3+
// the compiler to ignore them.
4+
#![allow(unused_variables)]
5+
6+
fn bad_moves() {
7+
let x = String::new();
8+
// Help diagnostics need to be annotated
9+
let y = x;
10+
//~^ HELP: consider cloning
11+
12+
// We expect a failure on this line
13+
println!("{x}"); //~ ERROR: borrow
14+
15+
16+
let x = String::new();
17+
// We expect the help message to mention cloning.
18+
//~v HELP: consider cloning
19+
let y = x;
20+
21+
// Check error message using a regex
22+
println!("{x}");
23+
//~^ ERROR: /(move)|(borrow)/
24+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
error[E0382]: borrow of moved value: `x`
2+
--> tests/example_tests/basic_test.rs:13:15
3+
|
4+
7 | let x = String::new();
5+
| - move occurs because `x` has type `String`, which does not implement the `Copy` trait
6+
8 | // Help diagnostics need to be annotated
7+
9 | let y = x;
8+
| - value moved here
9+
...
10+
13 | println!("{x}");
11+
| ^^^ value borrowed here after move
12+
|
13+
= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
14+
help: consider cloning the value if the performance cost is acceptable
15+
|
16+
9 | let y = x.clone();
17+
| ++++++++
18+
19+
error[E0382]: borrow of moved value: `x`
20+
--> tests/example_tests/basic_test.rs:22:15
21+
|
22+
16 | let x = String::new();
23+
| - move occurs because `x` has type `String`, which does not implement the `Copy` trait
24+
...
25+
19 | let y = x;
26+
| - value moved here
27+
...
28+
22 | println!("{x}");
29+
| ^^^ value borrowed here after move
30+
|
31+
= note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
32+
help: consider cloning the value if the performance cost is acceptable
33+
|
34+
19 | let y = x.clone();
35+
| ++++++++
36+
37+
error: aborting due to 2 previous errors
38+
39+
For more information about this error, try `rustc --explain E0382`.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// You can import anything defined in the dependencies table of the crate.
2+
use ui_test::Config;
3+
4+
fn wrong_type() {
5+
let _ = Config::this_function_does_not_exist();
6+
//~^ E0599
7+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error[E0599]: no function or associated item named `this_function_does_not_exist` found for struct `Config` in the current scope
2+
--> tests/example_tests/import.rs:5:21
3+
|
4+
5 | let _ = Config::this_function_does_not_exist();
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function or associated item not found in `Config`
6+
|
7+
note: if you're trying to build a new `Config` consider using one of the following associated functions:
8+
Config::rustc
9+
Config::cargo
10+
--> $RUSTUP_HOME/.cargo/git/checkouts/ui_test-2b82183a391bb05c/680bb08/src/config.rs:63:5
11+
|
12+
63 | pub fn rustc(root_dir: impl Into<PathBuf>) -> Self {
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
...
15+
108 | pub fn cargo(root_dir: impl Into<PathBuf>) -> Self {
16+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
17+
18+
error: aborting due to 1 previous error
19+
20+
For more information about this error, try `rustc --explain E0599`.

0 commit comments

Comments
 (0)