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

--exclude is broken for individual files #96342

Closed
jyn514 opened this issue Apr 23, 2022 · 0 comments · Fixed by #100004
Closed

--exclude is broken for individual files #96342

jyn514 opened this issue Apr 23, 2022 · 0 comments · Fixed by #100004
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented Apr 23, 2022

$ x t src/test/ui --exclude src/test/ui
Skipping Suite(test::src/test/ui) because it is excluded
Skipping Suite(test::src/test/ui) because it is excluded
$ x t src/test/ui/attr-start.rs --exclude src/test/ui/attr-start.rs
Check compiletest suite=ui mode=ui (x86_64-unknown-linux-gnu(x86_64-unknown-linux-gnu) -> x86_64-unknown-linux-gnu(x86_64-unknown-linux-gnu))

running 1 test
.
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 12951 filtered out; finished in 0.09s

This is harder to fix than it looks - if you try something simple like

diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs
index 0c4f3265dbf..e0acf20ae48 100644
--- a/src/bootstrap/builder.rs
+++ b/src/bootstrap/builder.rs
@@ -225,8 +225,8 @@ fn maybe_run(&self, builder: &Builder<'_>, pathset: &PathSet) {
         }
     }
 
-    fn is_excluded(&self, builder: &Builder<'_>, pathset: &PathSet) -> bool {
-        if builder.config.exclude.iter().any(|e| pathset.has(&e.path, e.kind)) {
+    fn is_excluded(&self, builder: &Builder<'_>, pathset: &PathSet, should_run: &ShouldRun) -> bool {
+        if builder.config.exclude.iter().any(|e| pathset.has(&e.path, e.kind) || should_run.is_suite_path(&e.path).is_some()) {
             eprintln!("Skipping {:?} because it is excluded", pathset);
             return true;
         }

you end up skipping the whole suite:

Skipping Suite(test::src/test/ui) because it is excluded
Skipping Suite(test::src/test/ui) because it is excluded

I think the proper fix for this involves adding support in compiletest itself :( by passing --exclude p for each is_valid_test_suite_arg in config.exclude.

@rustbot label +A-rustbuild +A-testsuite +C-bug +E-hard

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. labels Apr 23, 2022
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue May 12, 2022
…mulacrum

Add compiletest and bootstrap "--skip" option forwarded to libtest

With this PR,  "x.py test --skip SKIP ..." will run the specified test suite, but forward "--skip SKIP" to the test tool. libtest already supports this option. The PR also adds it to compiletest which itself just forwards it to libtest.

Adds the functionality requested in rust-lang#96342. This is useful to work around tests broken upstream.

rust-lang#96362 (comment) is the specific test issue my project is trying to work around.
bors added a commit to rust-lang-ci/rust that referenced this issue May 13, 2022
…lacrum

Add compiletest and bootstrap "--skip" option forwarded to libtest

With this PR,  "x.py test --skip SKIP ..." will run the specified test suite, but forward "--skip SKIP" to the test tool. libtest already supports this option. The PR also adds it to compiletest which itself just forwards it to libtest.

Adds the functionality requested in rust-lang#96342. This is useful to work around tests broken upstream.

rust-lang#96362 (comment) is the specific test issue my project is trying to work around.
@bors bors closed this as completed in 24cf45a Aug 7, 2022
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 20, 2024
Move `x test --skip` to be part of `--exclude`

`--skip` is inconsistent with the rest of the interface and redundant with `--exclude`.
Fix --exclude to work properly for files and directories rather than having a separate flag.

Fixes rust-lang/rust#96342. cc rust-lang/rust#96493 (comment)

r? `@Mark-Simulacrum`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Move `x test --skip` to be part of `--exclude`

`--skip` is inconsistent with the rest of the interface and redundant with `--exclude`.
Fix --exclude to work properly for files and directories rather than having a separate flag.

Fixes rust-lang/rust#96342. cc rust-lang/rust#96493 (comment)

r? `@Mark-Simulacrum`
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 C-bug Category: This is a bug. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants