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

Add unit test checking to cargo check #4592

Merged
merged 6 commits into from
Oct 30, 2017
Merged
Show file tree
Hide file tree
Changes from 3 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
24 changes: 20 additions & 4 deletions src/bin/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::env;

use cargo::core::Workspace;
use cargo::ops::{self, CompileOptions, MessageFormat, Packages};
use cargo::util::{CliResult, Config};
use cargo::util::{CliResult, CliError, Config};
use cargo::util::important_paths::find_root_manifest_for_wd;

pub const USAGE: &'static str = "
Expand All @@ -28,6 +28,7 @@ Options:
--benches Check all benches
--all-targets Check all targets (lib and bin targets by default)
--release Check artifacts in release mode, with optimizations
--profile PROFILE Profile to build the selected target for
--features FEATURES Space-separated list of features to also check
--all-features Check all available features
--no-default-features Do not check the `default` feature
Expand All @@ -53,6 +54,9 @@ Note that `--exclude` has to be specified in conjunction with the `--all` flag.
Compilation can be configured via the use of profiles which are configured in
the manifest. The default profile for this command is `dev`, but passing
the --release flag will use the `release` profile instead.

The `--profile test` flag can be used to check unit tests with the
`#[cfg(test)]` attribute.
";

#[derive(Deserialize)]
Expand Down Expand Up @@ -83,6 +87,7 @@ pub struct Options {
flag_frozen: bool,
flag_all: bool,
flag_exclude: Vec<String>,
flag_profile: Option<String>,
#[serde(rename = "flag_Z")]
flag_z: Vec<String>,
}
Expand All @@ -106,6 +111,17 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult {
&options.flag_exclude,
&options.flag_package)?;

let test = options.flag_tests ||
Copy link
Member

Choose a reason for hiding this comment

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

This logic may cause subtle oddities, although that's maybe ok for now? but for example:

cargo check --tests --bin foo

I think that'll check the binary foo in "test mode" rather than "do you compile mode", right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. The more I think about it, it is probably wrong. check --tests should not check bins with the test profile. I'll fix it.

match options.flag_profile.as_ref().map(|t| &t[..]) {
Some("test") => true,
None => false,
Some(profile) => {
let err = format!("unknown profile: `{}`, only `test` is currently supported",
profile).into();
return Err(CliError::new(err, 101))
}
};

let opts = CompileOptions {
config: config,
jobs: options.flag_jobs,
Expand All @@ -114,10 +130,10 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult {
all_features: options.flag_all_features,
no_default_features: options.flag_no_default_features,
spec: spec,
mode: ops::CompileMode::Check,
mode: ops::CompileMode::Check{test:test},
release: options.flag_release,
filter: ops::CompileFilter::new(options.flag_lib,
&options.flag_bin, options.flag_bins,
filter: ops::CompileFilter::new(options.flag_lib || options.flag_tests,
&options.flag_bin, options.flag_bins || options.flag_tests,
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment to why these two || conditions were added?

&options.flag_test, options.flag_tests,
&options.flag_example, options.flag_examples,
&options.flag_bench, options.flag_benches,
Expand Down
2 changes: 1 addition & 1 deletion src/bin/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult {
Some("dev") | None => CompileMode::Build,
Some("test") => CompileMode::Test,
Some("bench") => CompileMode::Bench,
Some("check") => CompileMode::Check,
Some("check") => CompileMode::Check {test: false},
Some(mode) => {
let err = format!("unknown profile: `{}`, use dev,
test, or bench", mode).into();
Expand Down
9 changes: 9 additions & 0 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ pub struct Profiles {
pub doc: Profile,
pub custom_build: Profile,
pub check: Profile,
pub check_test: Profile,
pub doctest: Profile,
}

Expand Down Expand Up @@ -661,6 +662,14 @@ impl Profile {
}
}

pub fn default_check_test() -> Profile {
Profile {
check: true,
test: true,
..Profile::default_dev()
}
}

pub fn default_doctest() -> Profile {
Profile {
doc: true,
Expand Down
1 change: 1 addition & 0 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@ impl<'cfg> Workspace<'cfg> {
doc: Profile::default_doc(),
custom_build: Profile::default_custom_build(),
check: Profile::default_check(),
check_test: Profile::default_check_test(),
doctest: Profile::default_doctest(),
};

Expand Down
4 changes: 2 additions & 2 deletions src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ pub fn clean(ws: &Workspace, opts: &CleanOptions) -> CargoResult<()> {
let Profiles {
ref release, ref dev, ref test, ref bench, ref doc,
ref custom_build, ref test_deps, ref bench_deps, ref check,
ref doctest,
ref check_test, ref doctest,
} = *profiles;
let profiles = [release, dev, test, bench, doc, custom_build,
test_deps, bench_deps, check, doctest];
test_deps, bench_deps, check, check_test, doctest];
for profile in profiles.iter() {
units.push(Unit {
pkg,
Expand Down
30 changes: 23 additions & 7 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ impl<'a> CompileOptions<'a> {
pub enum CompileMode {
Test,
Build,
Check,
Check { test: bool },
Bench,
Doc { deps: bool },
Doctest,
Expand Down Expand Up @@ -478,7 +478,7 @@ fn generate_auto_targets<'a>(mode: CompileMode, targets: &'a [Target],
}
base
}
CompileMode::Build | CompileMode::Check => {
CompileMode::Build | CompileMode::Check{..} => {
targets.iter().filter(|t| {
t.is_bin() || t.is_lib()
}).map(|t| BuildProposal {
Expand Down Expand Up @@ -603,11 +603,28 @@ fn generate_targets<'a>(pkg: &'a Package,
CompileMode::Test => test,
CompileMode::Bench => &profiles.bench,
CompileMode::Build => build,
CompileMode::Check => &profiles.check,
CompileMode::Check {test: false} => &profiles.check,
CompileMode::Check {test: true} => &profiles.check_test,
CompileMode::Doc { .. } => &profiles.doc,
CompileMode::Doctest => &profiles.doctest,
};

let test_profile = if profile.check {
&profiles.check_test
} else if mode == CompileMode::Build {
test
} else {
profile
};

let bench_profile = if profile.check {
&profiles.check_test
} else if mode == CompileMode::Build {
&profiles.bench
} else {
profile
};

let targets = match *filter {
CompileFilter::Default { required_features_filterable } => {
let deps = if release {
Expand All @@ -631,15 +648,14 @@ fn generate_targets<'a>(pkg: &'a Package,
bail!("no library targets found")
}
}

targets.append(&mut propose_indicated_targets(
pkg, bins, "bin", Target::is_bin, profile)?);
targets.append(&mut propose_indicated_targets(
pkg, examples, "example", Target::is_example, build)?);
pkg, examples, "example", Target::is_example, profile)?);
targets.append(&mut propose_indicated_targets(
pkg, tests, "test", Target::is_test, test)?);
pkg, tests, "test", Target::is_test, test_profile)?);
targets.append(&mut propose_indicated_targets(
pkg, benches, "bench", Target::is_bench, &profiles.bench)?);
pkg, benches, "bench", Target::is_bench, bench_profile)?);
targets
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_rustc/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
ret.extend(self.maybe_lib(unit));

// Integration tests/benchmarks require binaries to be built
if unit.profile.test &&
if unit.profile.test && !unit.profile.check &&
Copy link
Member

Choose a reason for hiding this comment

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

Could you expand the above comment to explain this negation?

(unit.target.is_test() || unit.target.is_bench()) {
ret.extend(unit.pkg.targets().iter().filter(|t| {
let no_required_features = Vec::new();
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,8 @@ fn build_profiles(profiles: &Option<TomlProfiles>) -> Profiles {
custom_build: Profile::default_custom_build(),
check: merge(Profile::default_check(),
profiles.and_then(|p| p.dev.as_ref())),
check_test: merge(Profile::default_check_test(),
profiles.and_then(|p| p.dev.as_ref())),
doctest: Profile::default_doctest(),
};
// The test/bench targets cannot have panic=abort because they'll all get
Expand Down
56 changes: 56 additions & 0 deletions tests/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3902,3 +3902,59 @@ fn uplift_dsym_of_bin_on_mac() {
assert_that(&p.bin("c.dSYM"), is_not(existing_dir()));
assert_that(&p.bin("d.dSYM"), is_not(existing_dir()));
}

// Make sure that `cargo build` chooses the correct profile for building
// targets based on filters (assuming --profile is not specified).
#[test]
fn build_filter_infer_profile() {
let p = project("foo")
.file("Cargo.toml", r#"
[package]
name = "foo"
version = "0.1.0"
authors = []
"#)
.file("src/lib.rs", "")
.file("src/main.rs", "fn main() {}")
.file("tests/t1.rs", "")
.file("benches/b1.rs", "")
.file("examples/ex1.rs", "fn main() {}")
.build();

assert_that(p.cargo("build").arg("-v"),
execs().with_status(0)
.with_stderr_contains("\
[RUNNING] `rustc --crate-name foo src[/]lib.rs --crate-type lib \
--emit=dep-info,link[..]")
.with_stderr_contains("\
[RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \
--emit=dep-info,link[..]")
);

p.root().join("target").rm_rf();
assert_that(p.cargo("build").arg("-v").arg("--test=t1"),
execs().with_status(0)
.with_stderr_contains("\
[RUNNING] `rustc --crate-name foo src[/]lib.rs --crate-type lib \
--emit=dep-info,link[..]")
.with_stderr_contains("\
[RUNNING] `rustc --crate-name t1 tests[/]t1.rs --emit=dep-info,link[..]")
.with_stderr_contains("\
[RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \
--emit=dep-info,link[..]")
);

p.root().join("target").rm_rf();
assert_that(p.cargo("build").arg("-v").arg("--bench=b1"),
execs().with_status(0)
.with_stderr_contains("\
[RUNNING] `rustc --crate-name foo src[/]lib.rs --crate-type lib \
--emit=dep-info,link[..]")
.with_stderr_contains("\
[RUNNING] `rustc --crate-name b1 benches[/]b1.rs --emit=dep-info,link \
-C opt-level=3[..]")
.with_stderr_contains("\
[RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \
--emit=dep-info,link[..]")
);
}
2 changes: 1 addition & 1 deletion tests/cargotest/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub fn cargo_home() -> PathBuf {

pub struct InstalledExe(pub &'static str);

fn exe(name: &str) -> String {
pub fn exe(name: &str) -> String {
if cfg!(windows) {format!("{}.exe", name)} else {name.to_string()}
}

Expand Down
Loading