use `--exact` on `--skip` to avoid unintended substring matches #132979

onur-ozkan · 2024-11-13T05:14:41Z

Without the --exact flag, using --skip tests/rustdoc can unintentionally skip other tests that match as substrings such as rustdoc-gui, rustdoc-js, etc.

For debugging, run: ./x.py --stage 2 test rustdoc-ui --skip tests/rustdoc and ./x.py --stage 2 test rustdoc-ui --skip tests/rustdoc -- --exact

Resolves #117721

try-job: x86_64-apple-1

Without the `--exact` flag, using `--skip tests/rustdoc` can unintentionally skip other tests that match as substrings such as `rustdoc-gui`, `rustdoc-js`, etc. For debugging, run: `./x.py --stage 2 test rustdoc-ui --skip tests/rustdoc` and `./x.py --stage 2 test rustdoc-ui --skip tests/rustdoc -- --exact` Signed-off-by: onur-ozkan <work@onurozkan.dev>

rustbot · 2024-11-13T05:14:50Z

r? @Kobzol

rustbot has assigned @Kobzol.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

jieyouxu · 2024-11-13T06:40:02Z

Thanks!
r? jieyouxu
@bors r+ rollup

bors · 2024-11-13T06:40:05Z

📌 Commit 13d59ec has been approved by jieyouxu

It is now in the queue for this repository.

use `--exact` on `--skip` to avoid unintended substring matches Without the `--exact` flag, using `--skip tests/rustdoc` can unintentionally skip other tests that match as substrings such as `rustdoc-gui`, `rustdoc-js`, etc. For debugging, run: `./x.py --stage 2 test rustdoc-ui --skip tests/rustdoc` and `./x.py --stage 2 test rustdoc-ui --skip tests/rustdoc -- --exact` Resolves rust-lang#117721

…llaumeGomez Rollup of 6 pull requests Successful merges: - rust-lang#132709 (optimize char::to_digit and assert radix is at least 2) - rust-lang#132842 (ABI checks: add support for tier2 arches) - rust-lang#132965 (allow CFGuard on windows-gnullvm) - rust-lang#132967 (fix REGISTRY_USERNAME to reuse cache between auto and pr jobs) - rust-lang#132971 (Handle infer vars in anon consts on stable) - rust-lang#132979 (use `--exact` on `--skip` to avoid unintended substring matches) r? `@ghost` `@rustbot` modify labels: rollup

GuillaumeGomez · 2024-11-13T13:24:58Z

Failed in #132989.

@bors r-

jieyouxu · 2024-11-13T13:30:36Z

Unknown flag --exact
The build system of cg_clif.

USAGE:
    ./y.sh prepare [--out-dir DIR] [--download-dir DIR]
    ./y.sh build [--sysroot none|clif|llvm] [--out-dir DIR] [--download-dir DIR] [--no-unstable-features] [--frozen]
    ./y.sh test [--sysroot none|clif|llvm] [--out-dir DIR] [--download-dir DIR] [--no-unstable-features] [--frozen] [--skip-test TESTNAME]
    ./y.sh abi-cafe [--sysroot none|clif|llvm] [--out-dir DIR] [--download-dir DIR] [--no-unstable-features] [--frozen]
    ./y.sh bench [--sysroot none|clif|llvm] [--out-dir DIR] [--download-dir DIR] [--no-unstable-features] [--frozen]
OPTIONS:

Hm, that might've been passed down to cg_clif?

/Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-codegen/x86_64-apple-darwin/release/y test --download-dir /Users/runner/work/rust/rust/build/cg_clif_download --out-dir /Users/runner/work/rust/rust/build/x86_64-apple-darwin/stage1-tools/cg_clif --no-unstable-features --use-backend cranelift --sysroot llvm --skip-test testsuite.extended_sysroot --exact

cc @bjorn3

onur-ozkan · 2024-11-13T13:47:44Z

It seems that this part:

rust/src/bootstrap/src/core/build_steps/test.rs

Line 3408 in 65b3877

cargo.args(builder.config.test_args());

is passing all test arguments to cg_clif which doesn't seem right as we can't know if they are supported or not. We should only pass specific arguments explicitly.

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan · 2024-11-13T13:49:37Z

@bors try

bors · 2024-11-13T13:50:49Z

⌛ Trying commit 1824c7f with merge 8675613...

use `--exact` on `--skip` to avoid unintended substring matches Without the `--exact` flag, using `--skip tests/rustdoc` can unintentionally skip other tests that match as substrings such as `rustdoc-gui`, `rustdoc-js`, etc. For debugging, run: `./x.py --stage 2 test rustdoc-ui --skip tests/rustdoc` and `./x.py --stage 2 test rustdoc-ui --skip tests/rustdoc -- --exact` Resolves rust-lang#117721 try-job: x86_64-apple-1

bors · 2024-11-13T15:58:47Z

💔 Test failed - checks-actions

onur-ozkan · 2024-11-13T20:14:57Z

@bors try

use `--exact` on `--skip` to avoid unintended substring matches Without the `--exact` flag, using `--skip tests/rustdoc` can unintentionally skip other tests that match as substrings such as `rustdoc-gui`, `rustdoc-js`, etc. For debugging, run: `./x.py --stage 2 test rustdoc-ui --skip tests/rustdoc` and `./x.py --stage 2 test rustdoc-ui --skip tests/rustdoc -- --exact` Resolves rust-lang#117721 try-job: x86_64-apple-1

bors · 2024-11-13T20:16:09Z

⌛ Trying commit 11dca9e with merge 505cd65...

src/bootstrap/src/core/build_steps/test.rs

onur-ozkan · 2024-11-25T17:29:12Z

@rustbot author

tgross35 · 2024-11-25T21:24:32Z

src/bootstrap/src/core/build_steps/test.rs

-        cargo_run.arg("--");
-        if builder.config.args().is_empty() {
-            // By default, exclude tests that take longer than ~1m.
-            cargo_run.arg("--skip-huge");
-        } else {
-            cargo_run.args(builder.config.args());
+        if env::var("FLOAT_PARSE_TESTS_SKIP_HUGE").unwrap_or_default() == "1" {
+            cargo_run
+                .arg("--")
+                // By default, exclude tests that take longer than ~1m.
+                .arg("--skip-huge");


I think this might be missing another branch here - with the change change it looks like the tests don't run at all unless FLOAT_PARSE_TESTS_SKIP_HUGE=1.

The logic should be: always run by default with --skip-huge. If the user somehow indicates that slow tests should not be skipped, don't pass --skip-huge. (So if this is done via env it should be something like FLOAT_PARSE_TESTS_NO_SKIP_HUGE).

I think it's quite annoying not to run all tests by default and to have to pass something like NO_SKIP_HUGE. If we don't want to run them by default, why the tool itself doesn't do it by default instead of requiring bootstrap to pass --skip-huge? Anyway, not a big deal — I can do that.

The reasoning is that when running the whole testsuite or in CI, about a minute of tests here is good enough. However, if it is run on its own, you probably don't want to forget to run the more thorough tests because of not passing a flag. I don't know what is best here, maybe the test should skip huge if RUSTC_BOOTSTRAP is set? (If you want to do that here it's easy enough, just set cfg.skip_huge if the env is set).

Anyway the change looks good as-is 👍

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan · 2024-11-26T19:15:10Z

@rustbot ready

onur-ozkan · 2024-11-27T05:17:09Z

@bors r=jieyouxu,tgross35

bors · 2024-11-27T05:17:12Z

📌 Commit 8d404a4 has been approved by jieyouxu,tgross35

It is now in the queue for this repository.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#132979 (use `--exact` on `--skip` to avoid unintended substring matches) - rust-lang#133248 (CI: split x86_64-msvc-ext job) - rust-lang#133449 (std: expose `const_io_error!` as `const_error!`) - rust-lang#133453 (Commit license-metadata.json to git and check it's correct in CI) - rust-lang#133457 (miri: implement `TlsFree`) - rust-lang#133493 (do not constrain infer vars in `find_best_leaf_obligation`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#132979 - onur-ozkan:skip-exact, r=jieyouxu,tgross35 use `--exact` on `--skip` to avoid unintended substring matches Without the `--exact` flag, using `--skip tests/rustdoc` can unintentionally skip other tests that match as substrings such as `rustdoc-gui`, `rustdoc-js`, etc. For debugging, run: `./x.py --stage 2 test rustdoc-ui --skip tests/rustdoc` and `./x.py --stage 2 test rustdoc-ui --skip tests/rustdoc -- --exact` Resolves rust-lang#117721 try-job: x86_64-apple-1

rustbot assigned Kobzol Nov 13, 2024

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Nov 13, 2024

rustbot assigned jieyouxu and unassigned Kobzol Nov 13, 2024

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 Nov 13, 2024

GuillaumeGomez mentioned this pull request Nov 13, 2024

Rollup of 6 pull requests #132989

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 13, 2024

don't pass every test arg to cg_clif

1824c7f

Signed-off-by: onur-ozkan <work@onurozkan.dev>

rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Nov 13, 2024