Switch some tests from `build` to `check` #11725

Muscraft · 2023-02-16T04:15:16Z

#11341 brought up issues with cargo's testsute size and speed. One of the suggested fixes was switching most tests to check instead of build. This PR did that.

Before size on nightly: 4.4GB

After size on nightly: 4.2GB

Regex to find build in tests/testsuite: cargo\(".*build.*\)
Before: 1607
After: 626

Note I did not remove all build I only did the easy ones that required minimal changes. I also tried not to touch systems I was unsure about. There could be other uses of build I missed as well.

I still need to play around with opt-level, debug=0, and a few other tweaks, but there should be more time/memory to drop.

Each test file changed is in a commit of its own, so you should look commit by commit.

rustbot · 2023-02-16T04:15:21Z

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

Muscraft · 2023-02-17T16:47:00Z

I think this is ready for review. I dropped debug=0 from this PR, so the changes switching build to check can be seen alone. There isn't a massive drop in size (or runtime, I haven't had time to retest), but it seems like a set of worthwhile changes. I could not switch all build to check, but I got many of the low-hanging ones.

If it is better to squash all of these commits after review, I would be happy to!

epage · 2023-02-17T20:17:04Z

@bors r+

bors · 2023-02-17T20:17:06Z

📌 Commit b152e10 has been approved by epage

It is now in the queue for this repository.

Switch some tests from `build` to `check` #11341 brought up issues with cargo's `testsute` size and speed. One of the suggested fixes was switching most tests to `check` instead of `build`. This PR did that. Before size on `nightly`: 4.4GB After size on `nightly`: 4.2GB Regex to find `build` in `tests/testsuite`: `cargo\(".*build.*\)` Before: 1607 After: 626 Note I did not remove all `build` I only did the easy ones that required minimal changes. I also tried not to touch systems I was unsure about. There could be other uses of `build` I missed as well. I still need to play around with `opt-level`, `debug=0`, and a few other tweaks, but there should be more time/memory to drop. Each test file changed is in a commit of its own, so you should look commit by commit.

bors · 2023-02-17T20:45:46Z

⌛ Testing commit b152e10 with merge 98b30bb...

ehuss · 2023-02-17T21:03:49Z

Hang on a bit, there are a number of changes I'd like to make before this merges.

bors · 2023-02-17T21:04:08Z

💔 Test failed - checks-actions

ehuss · 2023-02-17T18:07:53Z

tests/testsuite/build_script.rs

@@ -450,7 +450,7 @@ fn custom_build_env_var_rustc_linker() {

    // no crate type set => linker never called => build succeeds if and
    // only if build.rs succeeds, despite linker binary not existing.
-    p.cargo("build --target").arg(&target).run();
+    p.cargo("check --target").arg(&target).run();


I might suggest dropping all of the changes to build_script.rs. Many of these tests are related to linking, and I think there is some risk that using check builds will reduce coverage here.

ehuss · 2023-02-17T18:09:23Z

tests/testsuite/build_script_extra_link_arg.rs

@@ -22,7 +22,7 @@ fn build_script_extra_link_arg_bin() {
        )
        .build();

-    p.cargo("build -v")
+    p.cargo("check -v")


I would also probably not touch anything in build_script_extra_link_arg since it is related to linking.

ehuss · 2023-02-17T18:14:18Z

tests/testsuite/clean.rs

@@ -15,7 +15,7 @@ fn cargo_clean_simple() {
        .file("src/foo.rs", &main_file(r#""i am foo""#, &[]))
        .build();

-    p.cargo("build").run();
+    p.cargo("check").run();


I might avoid changing clean.rs as well, since several of these tests are related to the artifacts produced, and the set of artifacts is different with check.

ehuss · 2023-02-17T21:10:04Z

tests/testsuite/concurrent.rs

@@ -231,8 +231,8 @@ fn git_same_repo_different_tags() {
        );
    let p = p.build();

-    let mut a = p.cargo("build -v").cwd("a").build_command();
-    let mut b = p.cargo("build -v").cwd("b").build_command();
+    let mut a = p.cargo("check -v").cwd("a").build_command();


I think I would probably avoid modifying concurrent.rs. These involve generating artifacts, and cargo has had issues with collisions of artifacts. Since check generates different artifacts, I'd be concerned about the change in test coverage here.

ehuss · 2023-02-17T21:13:14Z

tests/testsuite/tool_paths.rs

@@ -21,10 +21,10 @@ fn pathless_tools() {
        )
        .build();

-    foo.cargo("build --verbose")
+    foo.cargo("check --verbose")


Since these are involved with linker flags, I think tool_paths.rs probably shouldn't be changed.

ehuss · 2023-02-20T18:10:58Z

tests/testsuite/plugins.rs

@@ -94,7 +94,7 @@ fn plugin_to_the_max() {
        .file("src/lib.rs", "pub fn baz() -> i32 { 1 }")
        .build();

-    foo.cargo("build").run();
+    foo.cargo("check").run();


I would avoid any changes to plugins.rs, since these involve linking and such.

ehuss · 2023-02-20T18:13:48Z

tests/testsuite/profile_config.rs

@@ -256,11 +256,11 @@ fn profile_config_all_options() {
        )
        .build();

-    p.cargo("build --release -v")
+    p.cargo("check --release -v")


This test involves codegen, so I don't think it should be modified.

I would probably avoid any changes to profile_config.rs to be safe, since they involve profile settings.

ehuss · 2023-02-20T18:15:12Z

tests/testsuite/profile_custom.rs

@@ -21,7 +21,7 @@ fn inherits_on_release() {
        .file("src/lib.rs", "")
        .build();

-    p.cargo("build")
+    p.cargo("check")


I'd also suggest not modifying profile_custom as any profile-oriented things might involve codgen.

ehuss · 2023-02-20T18:16:17Z

tests/testsuite/profiles.rs

@@ -269,14 +269,14 @@ fn profile_in_non_root_manifest_triggers_a_warning() {
        .file("bar/src/main.rs", "fn main() {}")
        .build();

-    p.cargo("build -v")
+    p.cargo("check -v")


I suggest not modifying anything in profiles.rs since they often involve codegen.

ehuss · 2023-02-20T18:18:00Z

tests/testsuite/rename_deps.rs

@@ -27,7 +27,7 @@ fn rename_dependency() {
        .file("src/lib.rs", "extern crate bar; extern crate baz;")
        .build();

-    p.cargo("build").run();
+    p.cargo("check").run();


Since these involve making sure that the renamed libraries can be linked, I would probably avoid any changes to rename_deps.rs.

ehuss · 2023-02-21T23:27:28Z

Thanks!

@bors r+

bors · 2023-02-21T23:27:31Z

📌 Commit 45c9c8e has been approved by ehuss

It is now in the queue for this repository.

bors · 2023-02-21T23:27:39Z

⌛ Testing commit 45c9c8e with merge 0c33172...

bors · 2023-02-22T00:15:53Z

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 0c33172 to master...

15 commits in 17b3d0de0897e1c6b8ca347bd39f850bb0a5b9f6..9d5b32f503fc099c4064298465add14d4bce11e6 2023-02-17 19:45:09 +0000 to 2023-02-22 23:04:16 +0000 - refactor(job_queue): docs and move types around (rust-lang/cargo#11758) - Scrub more of the test environment (rust-lang/cargo#11757) - Make more reads of environment variables go through the `Config` (rust-lang/cargo#11754) - Revert "Update curl-sys to use libcurl 7.88.1" (rust-lang/cargo#11755) - use consistent case (rust-lang/cargo#11748) - Switch some tests from `build` to `check` (rust-lang/cargo#11725) - Fix typo in sparse-registry warning message (rust-lang/cargo#11753) - reuse url encoding from `url` crate, don't use separate `percent-encoding` (rust-lang/cargo#11750) - Read environment variables through `Config` instead of `std::env::var(_os)` (rust-lang/cargo#11727) - Update curl-sys to use libcurl 7.88.1 (rust-lang/cargo#11749) - mdman: update pretty_assertions to reduce deps (rust-lang/cargo#11747) - Cleanup tests (rust-lang/cargo#11745) - Enhance help texts of position args (rust-lang/cargo#11740) - Fix typo (rust-lang/cargo#11741) - Update comment about cargo-ok (rust-lang/cargo#11724)

Update cargo 15 commits in 17b3d0de0897e1c6b8ca347bd39f850bb0a5b9f6..9d5b32f503fc099c4064298465add14d4bce11e6 2023-02-17 19:45:09 +0000 to 2023-02-22 23:04:16 +0000 - refactor(job_queue): docs and move types around (rust-lang/cargo#11758) - Scrub more of the test environment (rust-lang/cargo#11757) - Make more reads of environment variables go through the `Config` (rust-lang/cargo#11754) - Revert "Update curl-sys to use libcurl 7.88.1" (rust-lang/cargo#11755) - use consistent case (rust-lang/cargo#11748) - Switch some tests from `build` to `check` (rust-lang/cargo#11725) - Fix typo in sparse-registry warning message (rust-lang/cargo#11753) - reuse url encoding from `url` crate, don't use separate `percent-encoding` (rust-lang/cargo#11750) - Read environment variables through `Config` instead of `std::env::var(_os)` (rust-lang/cargo#11727) - Update curl-sys to use libcurl 7.88.1 (rust-lang/cargo#11749) - mdman: update pretty_assertions to reduce deps (rust-lang/cargo#11747) - Cleanup tests (rust-lang/cargo#11745) - Enhance help texts of position args (rust-lang/cargo#11740) - Fix typo (rust-lang/cargo#11741) - Update comment about cargo-ok (rust-lang/cargo#11724)

Muscraft added 3 commits February 15, 2023 15:28

chore: update alt_registry test to use check

1873c33

chore: update build_config test to use check

9174c22

chore: update bad_manifest_path test to use check

7fcb542

rustbot assigned ehuss Feb 16, 2023

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 16, 2023

Muscraft force-pushed the reduce-build-in-tests branch 2 times, most recently from 0396fed to b152e10 Compare February 17, 2023 16:39

Muscraft marked this pull request as ready for review February 17, 2023 16:42

Muscraft changed the title ~~WIP: Reduce testsuite overhead~~ Switch some tests from build to check` Feb 17, 2023

Muscraft changed the title ~~Switch some tests from build to check`~~ Switch some tests from build to check Feb 17, 2023

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 Feb 17, 2023

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 17, 2023

ehuss reviewed Feb 20, 2023

View reviewed changes

Muscraft added 7 commits February 20, 2023 12:20

chore: update build_script_env test to use check

e9838a5

chore: update cache_messages test to use check

e2a6ed7

chore: update cargo_env_config test to use check

9c6a044

chore: update cargo_features tests to use check

c10a4c6

chore: update cfg tests to use check

3845fc9

chore: update check_cfg tests to use check

d89e73c

chore: update config tests to use check

8507c17

Muscraft added 12 commits February 20, 2023 12:21

chore: update proc_macro tests to use check

a726b8d

chore: update profile_overrides tests to use check

8d66a02

chore: update progress tests to use check

41738be

chore: update pub_priv tests to use check

1b9af48

chore: update registry tests to use check

98c7466

chore: update registry_auth tests to use check

ab1794b

chore: update replace tests to use check

6282282

chore: update rust_version tests to use check

43f9b8e

chore: update rustflags tests to use check

866daf4

chore: update update tests to use check

b974cac

chore: update vendor tests to use check

221938a

chore: update workspaces tests to use check

45c9c8e

Muscraft force-pushed the reduce-build-in-tests branch from b152e10 to 45c9c8e Compare February 20, 2023 18:25

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 Feb 21, 2023

ehuss mentioned this pull request Feb 21, 2023

Reduce cargo's testsuite overhead #11341

Open

bors merged commit 0c33172 into rust-lang:master Feb 22, 2023

bors mentioned this pull request Feb 22, 2023

gitoxide integration: fetch #11448

Merged

16 tasks

weihanglo mentioned this pull request Feb 22, 2023

Update cargo rust-lang/rust#108330

Merged

Muscraft deleted the reduce-build-in-tests branch February 22, 2023 00:58

ehuss added this to the 1.69.0 milestone Mar 9, 2023

weihanglo mentioned this pull request Oct 2, 2024

initial version of checksum based freshness #14137

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch some tests from `build` to `check` #11725

Switch some tests from `build` to `check` #11725

Muscraft commented Feb 16, 2023 •

edited

Loading

rustbot commented Feb 16, 2023

Muscraft commented Feb 17, 2023 •

edited

Loading

epage commented Feb 17, 2023

bors commented Feb 17, 2023

bors commented Feb 17, 2023

ehuss commented Feb 17, 2023

bors commented Feb 17, 2023

ehuss Feb 17, 2023

ehuss Feb 17, 2023

ehuss Feb 17, 2023

ehuss Feb 17, 2023

ehuss Feb 17, 2023

ehuss Feb 20, 2023

ehuss Feb 20, 2023

ehuss Feb 20, 2023

ehuss Feb 20, 2023

ehuss Feb 20, 2023

ehuss commented Feb 21, 2023

bors commented Feb 21, 2023

bors commented Feb 21, 2023

bors commented Feb 22, 2023

Switch some tests from build to check #11725

Switch some tests from build to check #11725

Conversation

Muscraft commented Feb 16, 2023 • edited Loading

rustbot commented Feb 16, 2023

Muscraft commented Feb 17, 2023 • edited Loading

epage commented Feb 17, 2023

bors commented Feb 17, 2023

bors commented Feb 17, 2023

ehuss commented Feb 17, 2023

bors commented Feb 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehuss commented Feb 21, 2023

bors commented Feb 21, 2023

bors commented Feb 21, 2023

bors commented Feb 22, 2023

Switch some tests from `build` to `check` #11725

Switch some tests from `build` to `check` #11725

Muscraft commented Feb 16, 2023 •

edited

Loading

Muscraft commented Feb 17, 2023 •

edited

Loading