Skip to content

tests: refactor testsuite cargo build --target #14843

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions crates/cargo-test-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1123,6 +1123,21 @@ impl Execs {
}
Ok(())
}

pub fn target(&mut self, target: &str) -> &mut Self {
self.arg("--target").arg(target);
self
}

pub fn target_host(&mut self) -> &mut Self {
self.target(rustc_host());
self
}

pub fn target_dir(&mut self, dir: &str) -> &mut Self {
self.arg("--target-dir").arg(dir);
self
}
}

impl Drop for Execs {
Expand Down
10 changes: 3 additions & 7 deletions tests/build-std/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ fn enable_build_std(e: &mut Execs, arg: Option<&str>) {
trait BuildStd: Sized {
fn build_std(&mut self) -> &mut Self;
fn build_std_arg(&mut self, arg: &str) -> &mut Self;
fn target_host(&mut self) -> &mut Self;
}

impl BuildStd for Execs {
Expand All @@ -55,11 +54,6 @@ impl BuildStd for Execs {
enable_build_std(self, Some(arg));
self
}

fn target_host(&mut self) -> &mut Self {
self.arg("--target").arg(rustc_host());
self
}
}

#[cargo_test(build_std_real)]
Expand Down Expand Up @@ -192,7 +186,9 @@ fn cross_custom() {
)
.build();

p.cargo("build --target custom-target.json -v")
p.cargo("build")
.target("custom-target.json")
.arg("-v")
.build_std_arg("core")
.run();
}
Expand Down
11 changes: 6 additions & 5 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3991,7 +3991,7 @@ fn custom_target_dir_line_parameter() {

let exe_name = format!("foo{}", env::consts::EXE_SUFFIX);

p.cargo("build --target-dir foo/target").run();
p.cargo("build").target_dir("foo/target").run();
assert!(p.root().join("foo/target/debug").join(&exe_name).is_file());
assert!(!p.root().join("target/debug").join(&exe_name).is_file());

Expand All @@ -4006,12 +4006,13 @@ fn custom_target_dir_line_parameter() {
target-dir = "foo/target"
"#,
);
p.cargo("build --target-dir bar/target").run();
p.cargo("build").target_dir("bar/target").run();
assert!(p.root().join("bar/target/debug").join(&exe_name).is_file());
assert!(p.root().join("foo/target/debug").join(&exe_name).is_file());
assert!(p.root().join("target/debug").join(&exe_name).is_file());

p.cargo("build --target-dir foobar/target")
p.cargo("build")
.target_dir("foobar/target")
.env("CARGO_TARGET_DIR", "bar/target")
.run();
assert!(p
Expand Down Expand Up @@ -4456,8 +4457,8 @@ fn cargo_build_empty_target() {
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("build --target")
.arg("")
p.cargo("build")
.target("")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] target was empty
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,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("build").target(&target).run();
}

// Only run this test on linux, since it's difficult to construct
Expand Down Expand Up @@ -494,7 +494,7 @@ fn custom_build_env_var_rustc_linker_with_target_cfg() {

// 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("build").target(&target).run();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of dynamic one is something in my mind could benefit from this change, though I haven't seen other 100+ occurrences changed in this pull request.

Upon this point, maybe this refactor might not be worthy with this scale of churn on your side?
(updating 100+ occurrences of --target sounds tedious)

}

#[cargo_test]
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/cross_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ fn simple_deps() {
.build();

let target = cross_compile::alternate();
p.cargo("build --target").arg(&target).run();
p.cargo("build").target(&target).run();
assert!(p.target_bin(target, "foo").is_file());

if cross_compile::can_run_on_host() {
Expand Down
14 changes: 11 additions & 3 deletions tests/testsuite/custom_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,13 @@ fn custom_target_minimal() {
.file("custom-target.json", SIMPLE_SPEC)
.build();

p.cargo("build --lib --target custom-target.json -v").run();
p.cargo("build --lib --target src/../custom-target.json -v")
p.cargo("build --lib")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So does this one. The original one is more readable IMO.

.target("custom-target.json")
.arg("-v")
.run();
p.cargo("build --lib")
.target("src/../custom-target.json")
.arg("-v")
.run();

// Ensure that the correct style of flag is passed to --target with doc tests.
Expand Down Expand Up @@ -140,7 +145,10 @@ fn custom_bin_target() {
.file("custom-bin-target.json", SIMPLE_SPEC)
.build();

p.cargo("build --target custom-bin-target.json -v").run();
p.cargo("build")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One drawback of it is that the argument list is spliited into multiple lines of indirection, making it harder to read at first glance.

.target("custom-bin-target.json")
.arg("-v")
.run();
}

#[cargo_test(nightly, reason = "requires features no_core, lang_items")]
Expand Down
6 changes: 0 additions & 6 deletions tests/testsuite/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ fn enable_build_std(e: &mut Execs, setup: &Setup) {
trait BuildStd: Sized {
fn build_std(&mut self, setup: &Setup) -> &mut Self;
fn build_std_arg(&mut self, setup: &Setup, arg: &str) -> &mut Self;
fn target_host(&mut self) -> &mut Self;
}

impl BuildStd for Execs {
Expand All @@ -178,11 +177,6 @@ impl BuildStd for Execs {
self.arg(format!("-Zbuild-std={}", arg));
self
}

fn target_host(&mut self) -> &mut Self {
self.arg("--target").arg(rustc_host());
self
}
}

#[cargo_test(build_std_mock)]
Expand Down