Skip to content

Commit

Permalink
Auto merge of #2670 - matklad:simplify-tests, r=alexcrichton
Browse files Browse the repository at this point in the history 
Simplify tests

@alexcrichton I think there is one bit of tests that could be simplified.

What do you think about writing this

```Rust
test!(simple {
    pkg("foo", "0.0.1");

    assert_that(cargo_process("install").arg("foo"),
                execs().with_status(0).with_stdout(&format!("\
[UPDATING] registry `[..]`
[DOWNLOADING] foo v0.0.1 (registry file://[..])
[COMPILING] foo v0.0.1 (registry file://[..])
[INSTALLING] {home}[..]bin[..]foo[..]
",
        home = cargo_home().display())));
    assert_that(cargo_home(), has_installed_exe("foo"));

    assert_that(cargo_process("uninstall").arg("foo"),
                execs().with_status(0).with_stdout(&format!("\
[REMOVING] {home}[..]bin[..]foo[..]
",
        home = cargo_home().display())));
    assert_that(cargo_home(), is_not(has_installed_exe("foo")));
});
```

instead of this

```Rust
test!(simple {
    pkg("foo", "0.0.1");

    assert_that(cargo_process("install").arg("foo"),
                execs().with_status(0).with_stdout(&format!("\
{updating} registry `[..]`
{downloading} foo v0.0.1 (registry file://[..])
{compiling} foo v0.0.1 (registry file://[..])
{installing} {home}[..]bin[..]foo[..]
",
        updating = UPDATING,
        downloading = DOWNLOADING,
        compiling = COMPILING,
        installing = INSTALLING,
        home = cargo_home().display())));
    assert_that(cargo_home(), has_installed_exe("foo"));

    assert_that(cargo_process("uninstall").arg("foo"),
                execs().with_status(0).with_stdout(&format!("\
{removing} {home}[..]bin[..]foo[..]
",
        removing = REMOVING,
        home = cargo_home().display())));
    assert_that(cargo_home(), is_not(has_installed_exe("foo")));
});
```

This PR has a proof of concept implementation of this feature applied to a couple of tests.

r? @alexcrichton
  • Loading branch information
@bors
bors committed May 11, 2016
2 parents 935df0f + 29cdad4 commit e3bc030
Show file tree
Hide file tree
Showing 35 changed files with 1,258 additions and 1,504 deletions.
34 changes: 27 additions & 7 deletions tests/support/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ impl Execs {
description: &str, extra: &[u8],
partial: bool) -> ham::MatchResult {
let out = match expected {
Some(out) => out,
Some(out) => substitute_macros(out),
None => return ham::success(),
};
let actual = match str::from_utf8(actual) {
Expand Down Expand Up @@ -657,12 +657,32 @@ pub fn path2url(p: PathBuf) -> Url {
Url::from_file_path(&*p).ok().unwrap()
}

pub static RUNNING: &'static str = " Running";
pub static COMPILING: &'static str = " Compiling";
pub static ERROR: &'static str = "error:";
pub static DOCUMENTING: &'static str = " Documenting";
pub static FRESH: &'static str = " Fresh";
pub static UPDATING: &'static str = " Updating";
fn substitute_macros(input: &str) -> String {
let macros = [
("[RUNNING]", " Running"),
("[COMPILING]", " Compiling"),
("[ERROR]", "error:"),
("[DOCUMENTING]", " Documenting"),
("[FRESH]", " Fresh"),
("[UPDATING]", " Updating"),
("[ADDING]", " Adding"),
("[REMOVING]", " Removing"),
("[DOCTEST]", " Doc-tests"),
("[PACKAGING]", " Packaging"),
("[DOWNLOADING]", " Downloading"),
("[UPLOADING]", " Uploading"),
("[VERIFYING]", " Verifying"),
("[ARCHIVING]", " Archiving"),
("[INSTALLING]", " Installing"),
("[REPLACING]", " Replacing")
];
let mut result = input.to_owned();
for &(pat, subst) in macros.iter() {
result = result.replace(pat, subst)
}
return result;
}

pub static ADDING: &'static str = " Adding";
pub static REMOVING: &'static str = " Removing";
pub static DOCTEST: &'static str = " Doc-tests";
Expand Down
99 changes: 43 additions & 56 deletions tests/test_bad_config.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use support::{project, execs, ERROR};
use support::{project, execs};
use support::registry::Package;
use hamcrest::assert_that;

Expand All @@ -19,11 +19,10 @@ test!(bad1 {
"#);
assert_that(foo.cargo_process("build").arg("-v")
.arg("--target=nonexistent-target"),
execs().with_status(101).with_stderr(&format!("\
{error} expected table for configuration key `target.nonexistent-target`, \
execs().with_status(101).with_stderr("\
[ERROR] expected table for configuration key `target.nonexistent-target`, \
but found string in [..]config
",
error = ERROR)));
"));
});

test!(bad2 {
Expand All @@ -40,8 +39,8 @@ test!(bad2 {
proxy = 3.0
"#);
assert_that(foo.cargo_process("publish").arg("-v"),
execs().with_status(101).with_stderr(&format!("\
{error} Couldn't load Cargo configuration
execs().with_status(101).with_stderr("\
[ERROR] Couldn't load Cargo configuration
Caused by:
failed to load TOML configuration from `[..]config`
Expand All @@ -54,7 +53,7 @@ Caused by:
Caused by:
found TOML configuration value of unknown type `float`
", error = ERROR)));
"));
});

test!(bad3 {
Expand All @@ -71,11 +70,10 @@ test!(bad3 {
proxy = true
"#);
assert_that(foo.cargo_process("publish").arg("-v"),
execs().with_status(101).with_stderr(&format!("\
{error} invalid configuration for key `http.proxy`
execs().with_status(101).with_stderr("\
[ERROR] invalid configuration for key `http.proxy`
expected a string, but found a boolean in [..]config
",
error = ERROR)));
"));
});

test!(bad4 {
Expand All @@ -85,14 +83,13 @@ test!(bad4 {
name = false
"#);
assert_that(foo.cargo_process("new").arg("-v").arg("foo"),
execs().with_status(101).with_stderr(&format!("\
{error} Failed to create project `foo` at `[..]`
execs().with_status(101).with_stderr("\
[ERROR] Failed to create project `foo` at `[..]`
Caused by:
invalid configuration for key `cargo-new.name`
expected a string, but found a boolean in [..]config
",
error = ERROR)));
"));
});

test!(bad5 {
Expand All @@ -106,8 +103,8 @@ test!(bad5 {
foo.build();
assert_that(foo.cargo("new")
.arg("-v").arg("foo").cwd(&foo.root().join("foo")),
execs().with_status(101).with_stderr(&format!("\
{error} Couldn't load Cargo configuration
execs().with_status(101).with_stderr("\
[ERROR] Couldn't load Cargo configuration
Caused by:
failed to merge key `foo` between files:
Expand All @@ -116,8 +113,7 @@ Caused by:
Caused by:
expected integer, but found string
",
error = ERROR)));
"));
});

test!(bad_cargo_config_jobs {
Expand All @@ -134,10 +130,9 @@ test!(bad_cargo_config_jobs {
jobs = -1
"#);
assert_that(foo.cargo_process("build").arg("-v"),
execs().with_status(101).with_stderr(&format!("\
{error} build.jobs must be positive, but found -1 in [..]
",
error = ERROR)));
execs().with_status(101).with_stderr("\
[ERROR] build.jobs must be positive, but found -1 in [..]
"));
});

test!(default_cargo_config_jobs {
Expand Down Expand Up @@ -189,8 +184,8 @@ test!(invalid_global_config {
.file("src/lib.rs", "");

assert_that(foo.cargo_process("build").arg("-v"),
execs().with_status(101).with_stderr(&format!("\
{error} Couldn't load Cargo configuration
execs().with_status(101).with_stderr("\
[ERROR] Couldn't load Cargo configuration
Caused by:
could not parse TOML configuration in `[..]config`
Expand All @@ -199,8 +194,7 @@ Caused by:
could not parse input as TOML
[..]config:1:2 expected `=`, but found eof
",
error = ERROR)));
"));
});

test!(bad_cargo_lock {
Expand All @@ -215,13 +209,12 @@ test!(bad_cargo_lock {
.file("src/lib.rs", "");

assert_that(foo.cargo_process("build").arg("-v"),
execs().with_status(101).with_stderr(&format!("\
{error} failed to parse lock file at: [..]Cargo.lock
execs().with_status(101).with_stderr("\
[ERROR] failed to parse lock file at: [..]Cargo.lock
Caused by:
expected a section for the key `root`
",
error = ERROR)));
"));
});

test!(bad_git_dependency {
Expand All @@ -238,16 +231,15 @@ test!(bad_git_dependency {
.file("src/lib.rs", "");

assert_that(foo.cargo_process("build").arg("-v"),
execs().with_status(101).with_stderr(&format!("\
{error} Unable to update file:///
execs().with_status(101).with_stderr("\
[ERROR] Unable to update file:///
Caused by:
failed to clone into: [..]
Caused by:
[[..]] 'file:///' is not a valid local file URI
",
error = ERROR)));
"));
});

test!(bad_crate_type {
Expand Down Expand Up @@ -285,15 +277,14 @@ test!(malformed_override {
.file("src/lib.rs", "");

assert_that(foo.cargo_process("build"),
execs().with_status(101).with_stderr(&format!("\
{error} failed to parse manifest at `[..]`
execs().with_status(101).with_stderr("\
[ERROR] failed to parse manifest at `[..]`
Caused by:
could not parse input as TOML
Cargo.toml:[..]
",
error = ERROR)));
"));
});

test!(duplicate_binary_names {
Expand All @@ -316,13 +307,12 @@ test!(duplicate_binary_names {
.file("b.rs", r#"fn main() -> () {}"#);

assert_that(foo.cargo_process("build"),
execs().with_status(101).with_stderr(&format!("\
{error} failed to parse manifest at `[..]`
execs().with_status(101).with_stderr("\
[ERROR] failed to parse manifest at `[..]`
Caused by:
found duplicate binary name e, but all binary targets must have a unique name
",
error = ERROR)));
"));
});

test!(duplicate_example_names {
Expand All @@ -345,13 +335,12 @@ test!(duplicate_example_names {
.file("examples/ex2.rs", r#"fn main () -> () {}"#);

assert_that(foo.cargo_process("build").arg("--example").arg("ex"),
execs().with_status(101).with_stderr(&format!("\
{error} failed to parse manifest at `[..]`
execs().with_status(101).with_stderr("\
[ERROR] failed to parse manifest at `[..]`
Caused by:
found duplicate example name ex, but all binary targets must have a unique name
",
error = ERROR)));
"));
});

test!(duplicate_bench_names {
Expand All @@ -374,13 +363,12 @@ test!(duplicate_bench_names {
.file("benches/ex2.rs", r#"fn main () {}"#);

assert_that(foo.cargo_process("bench"),
execs().with_status(101).with_stderr(&format!("\
{error} failed to parse manifest at `[..]`
execs().with_status(101).with_stderr("\
[ERROR] failed to parse manifest at `[..]`
Caused by:
found duplicate bench name ex, but all binary targets must have a unique name
",
error = ERROR)));
"));
});

test!(duplicate_deps {
Expand Down Expand Up @@ -418,13 +406,12 @@ test!(duplicate_deps {
.file("src/main.rs", r#"fn main () {}"#);

assert_that(foo.cargo_process("build"),
execs().with_status(101).with_stderr(&format!("\
{error} failed to parse manifest at `[..]`
execs().with_status(101).with_stderr("\
[ERROR] failed to parse manifest at `[..]`
Caused by:
found duplicate dependency name bar, but all dependencies must have a unique name
",
error = ERROR)));
"));
});

test!(unused_keys {
Expand Down
11 changes: 5 additions & 6 deletions tests/test_bad_manifest_path.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use support::{project, execs, main_file, basic_bin_manifest, ERROR};
use support::{project, execs, main_file, basic_bin_manifest};
use hamcrest::{assert_that};

fn setup() {}
Expand All @@ -12,9 +12,8 @@ fn assert_not_a_cargo_toml(command: &str, manifest_path_argument: &str) {
.arg("--manifest-path").arg(manifest_path_argument)
.cwd(p.root().parent().unwrap()),
execs().with_status(101)
.with_stderr(&format!("{error} the manifest-path must be a path \
to a Cargo.toml file",
error = ERROR)));
.with_stderr(&format!("[ERROR] the manifest-path must be a path \
to a Cargo.toml file")));
}

#[allow(deprecated)] // connect => join in 1.3
Expand All @@ -28,8 +27,8 @@ fn assert_cargo_toml_doesnt_exist(command: &str, manifest_path_argument: &str) {
.cwd(p.root().parent().unwrap()),
execs().with_status(101)
.with_stderr(
format!("{error} manifest path `{}` does not exist",
expected_path, error = ERROR)
format!("[ERROR] manifest path `{}` does not exist",
expected_path)
));
}

Expand Down
12 changes: 5 additions & 7 deletions tests/test_cargo.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::str;

use cargo_process;
use support::paths;
use support::{execs, project, mkdir_recursive, ProjectBuilder, ERROR};
use support::{execs, project, mkdir_recursive, ProjectBuilder};
use hamcrest::{assert_that};

fn setup() {
Expand Down Expand Up @@ -102,12 +102,11 @@ test!(find_closest_biuld_to_build {

assert_that(pr,
execs().with_status(101)
.with_stderr(&format!("{error} no such subcommand
.with_stderr(&format!("[ERROR] no such subcommand
<tab>Did you mean `build`?
",
error = ERROR)));
")));
});

// if a subcommand is more than 3 edit distance away, we don't make a suggestion
Expand All @@ -118,9 +117,8 @@ test!(find_closest_dont_correct_nonsense {

assert_that(pr,
execs().with_status(101)
.with_stderr(&format!("{error} no such subcommand
",
error = ERROR)));
.with_stderr(&format!("[ERROR] no such subcommand
")));
});

test!(override_cargo_home {
Expand Down
Loading

0 comments on commit e3bc030

Please sign in to comment.