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

Dull the errors #2421

Merged
merged 1 commit into from
Mar 12, 2016
Merged
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
2 changes: 1 addition & 1 deletion src/bin/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
None => Ok(None),
Some(err) => {
Err(match err.exit.as_ref().and_then(|e| e.code()) {
Some(i) => CliError::new("", i),
Some(i) => CliError::new("bench failed", i),
None => CliError::from_error(Human(err), 101)
})
}
Expand Down
2 changes: 1 addition & 1 deletion src/bin/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
None => Ok(None),
Some(err) => {
Err(match err.exit.as_ref().and_then(|e| e.code()) {
Some(i) => CliError::new("", i),
Some(i) => CliError::new("test failed", i),
None => CliError::from_error(Human(err), 101)
})
}
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl MultiShell {
}

pub fn error<T: ToString>(&mut self, message: T) -> CargoResult<()> {
self.err().say(message, RED)
self.err().say_status("error", message.to_string(), RED)
}

pub fn warn<T: ToString>(&mut self, message: T) -> CargoResult<()> {
Expand Down
14 changes: 10 additions & 4 deletions src/cargo/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,15 @@ pub fn shell(verbosity: Verbosity, color_config: ColorConfig) -> MultiShell {
// For fatal errors, print to stderr;
// and for others, e.g. docopt version info, print to stdout.
fn output(err: String, shell: &mut MultiShell, fatal: bool) {
let std_shell = if fatal {shell.err()} else {shell.out()};
let color = if fatal {RED} else {BLACK};
let _ = std_shell.say(err, color);
let (std_shell, color, message) = if fatal {
(shell.err(), RED, Some("error"))
} else {
(shell.out(), BLACK, None)
};
let _ = match message{
Some(text) => std_shell.say_status(text, err.to_string(), color),
None => std_shell.say(err, color)
};
}

pub fn handle_error(err: CliError, shell: &mut MultiShell) {
Expand All @@ -194,7 +200,7 @@ pub fn handle_error(err: CliError, shell: &mut MultiShell) {

let hide = unknown && shell.get_verbose() != Verbose;
if hide {
let _ = shell.err().say("An unknown error occurred", RED);
let _ = shell.err().say_status("error", "An unknown error occurred", RED);
} else {
output(error.to_string(), shell, fatal);
}
Expand Down
1 change: 1 addition & 0 deletions tests/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,7 @@ pub fn path2url(p: PathBuf) -> Url {

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";
Expand Down
93 changes: 53 additions & 40 deletions tests/test_bad_config.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use support::{project, execs};
use support::{project, execs, ERROR};
use support::registry::Package;
use hamcrest::assert_that;

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

test!(bad2 {
Expand All @@ -38,8 +40,8 @@ test!(bad2 {
proxy = 3.0
"#);
assert_that(foo.cargo_process("publish").arg("-v"),
execs().with_status(101).with_stderr("\
Couldn't load Cargo configuration
execs().with_status(101).with_stderr(&format!("\
{error} Couldn't load Cargo configuration

Caused by:
failed to load TOML configuration from `[..]config`
Expand All @@ -52,7 +54,7 @@ Caused by:

Caused by:
found TOML configuration value of unknown type `float`
"));
", error = ERROR)));
});

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

test!(bad4 {
Expand All @@ -82,13 +85,14 @@ test!(bad4 {
name = false
"#);
assert_that(foo.cargo_process("new").arg("-v").arg("foo"),
execs().with_status(101).with_stderr("\
Failed to create project `foo` at `[..]`
execs().with_status(101).with_stderr(&format!("\
{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 @@ -102,8 +106,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("\
Couldn't load Cargo configuration
execs().with_status(101).with_stderr(&format!("\
{error} Couldn't load Cargo configuration

Caused by:
failed to merge key `foo` between files:
Expand All @@ -112,7 +116,8 @@ Caused by:

Caused by:
expected integer, but found string
"));
",
error = ERROR)));
});

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

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

assert_that(foo.cargo_process("build").arg("-v"),
execs().with_status(101).with_stderr("\
Couldn't load Cargo configuration
execs().with_status(101).with_stderr(&format!("\
{error} Couldn't load Cargo configuration

Caused by:
could not parse TOML configuration in `[..]config`
Expand All @@ -193,7 +199,8 @@ Caused by:
could not parse input as TOML
[..]config:1:2 expected `=`, but found eof

"));
",
error = ERROR)));
});

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

assert_that(foo.cargo_process("build").arg("-v"),
execs().with_status(101).with_stderr("\
failed to parse lock file at: [..]Cargo.lock
execs().with_status(101).with_stderr(&format!("\
{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 @@ -230,15 +238,16 @@ test!(bad_git_dependency {
.file("src/lib.rs", "");

assert_that(foo.cargo_process("build").arg("-v"),
execs().with_status(101).with_stderr("\
Unable to update file:///
execs().with_status(101).with_stderr(&format!("\
{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 @@ -276,14 +285,15 @@ test!(malformed_override {
.file("src/lib.rs", "");

assert_that(foo.cargo_process("build"),
execs().with_status(101).with_stderr("\
failed to parse manifest at `[..]`
execs().with_status(101).with_stderr(&format!("\
{error} failed to parse manifest at `[..]`

Caused by:
could not parse input as TOML
Cargo.toml:[..]

"));
",
error = ERROR)));
});

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

assert_that(foo.cargo_process("build"),
execs().with_status(101).with_stderr("\
failed to parse manifest at `[..]`
execs().with_status(101).with_stderr(&format!("\
{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 @@ -334,12 +345,13 @@ 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("\
failed to parse manifest at `[..]`
execs().with_status(101).with_stderr(&format!("\
{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 @@ -362,12 +374,13 @@ test!(duplicate_bench_names {
.file("benches/ex2.rs", r#"fn main () {}"#);

assert_that(foo.cargo_process("bench"),
execs().with_status(101).with_stderr("\
failed to parse manifest at `[..]`
execs().with_status(101).with_stderr(&format!("\
{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!(unused_keys {
Expand Down
9 changes: 6 additions & 3 deletions tests/test_bad_manifest_path.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use support::{project, execs, main_file, basic_bin_manifest};
use support::{project, execs, main_file, basic_bin_manifest, ERROR};
use hamcrest::{assert_that};

fn setup() {}
Expand All @@ -12,7 +12,9 @@ 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("the manifest-path must be a path to a Cargo.toml file"));
.with_stderr(&format!("{error} the manifest-path must be a path \
to a Cargo.toml file",
error = ERROR)));
}

#[allow(deprecated)] // connect => join in 1.3
Expand All @@ -26,7 +28,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!("manifest path `{}` does not exist", expected_path)
format!("{error} manifest path `{}` does not exist",
expected_path, error = ERROR)
));
}

Expand Down
12 changes: 7 additions & 5 deletions tests/test_cargo.rs
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};
use support::{execs, project, mkdir_recursive, ProjectBuilder, ERROR};
use hamcrest::{assert_that};

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

assert_that(pr,
execs().with_status(101)
.with_stderr("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 @@ -83,8 +84,9 @@ test!(find_closest_dont_correct_nonsense {

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

test!(override_cargo_home {
Expand Down
Loading