From e2e3e49f50636b3f1cb56734ff4cbf070bf4cd70 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 31 Jul 2018 15:30:08 -0700 Subject: [PATCH 1/4] test: Add a failing test case for #242 --- tests/all/build.rs | 18 ++++++++++++++++++ tests/all/main.rs | 2 ++ tests/fixtures/not-a-crate/README.md | 1 + 3 files changed, 21 insertions(+) create mode 100644 tests/all/build.rs create mode 100644 tests/fixtures/not-a-crate/README.md diff --git a/tests/all/build.rs b/tests/all/build.rs new file mode 100644 index 0000000..ea66d3b --- /dev/null +++ b/tests/all/build.rs @@ -0,0 +1,18 @@ +use structopt::StructOpt; +use utils; +use wasm_pack::{command, logger, Cli}; + +#[test] +fn build_in_non_crate_directory_doesnt_panic() { + let fixture = utils::fixture::fixture("tests/fixtures/not-a-crate"); + let cli = Cli::from_iter_safe(vec![ + "wasm-pack", + "build", + &fixture.path.display().to_string(), + ]).unwrap(); + let logger = logger::new(&cli.cmd, cli.verbosity).unwrap(); + assert!( + command::run_wasm_pack(cli.cmd, &logger).is_err(), + "running wasm-pack in a non-crate directory should fail, but it should not panic" + ); +} diff --git a/tests/all/main.rs b/tests/all/main.rs index 998588e..57f8885 100644 --- a/tests/all/main.rs +++ b/tests/all/main.rs @@ -3,9 +3,11 @@ extern crate failure; #[macro_use] extern crate serde_derive; extern crate serde_json; +extern crate structopt; extern crate tempfile; extern crate wasm_pack; +mod build; mod manifest; mod readme; mod utils; diff --git a/tests/fixtures/not-a-crate/README.md b/tests/fixtures/not-a-crate/README.md new file mode 100644 index 0000000..ea36f59 --- /dev/null +++ b/tests/fixtures/not-a-crate/README.md @@ -0,0 +1 @@ +This is not a Rust crate! From ed35e497c7348d53e3eeda4ade4e20efbf2684e7 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 31 Jul 2018 15:35:11 -0700 Subject: [PATCH 2/4] Propagate missing `Cargo.toml` errors instead of unwrapping them This allows them to be reported to the user, rather than panic and dump a human-error failure. Fixes #242 --- src/command/build.rs | 17 ++++++++--------- src/command/mod.rs | 2 +- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/command/build.rs b/src/command/build.rs index c9ebf59..0d649e8 100644 --- a/src/command/build.rs +++ b/src/command/build.rs @@ -64,12 +64,15 @@ pub struct BuildOptions { // build_config: Option, } -impl From for Build { - fn from(build_opts: BuildOptions) -> Self { +type BuildStep = fn(&mut Build, &Step, &Logger) -> Result<(), Error>; + +impl Build { + /// Construct a build command from the given options. + pub fn try_from_opts(build_opts: BuildOptions) -> Result { let crate_path = set_crate_path(build_opts.path); - let crate_name = manifest::get_crate_name(&crate_path).unwrap(); + let crate_name = manifest::get_crate_name(&crate_path)?; // let build_config = manifest::xxx(&crate_path).xxx(); - Build { + Ok(Build { crate_path, scope: build_opts.scope, disable_dts: build_opts.disable_dts, @@ -77,13 +80,9 @@ impl From for Build { debug: build_opts.debug, // build_config, crate_name, - } + }) } -} - -type BuildStep = fn(&mut Build, &Step, &Logger) -> Result<(), Error>; -impl Build { /// Execute this `Build` command. pub fn run(&mut self, log: &Logger, mode: &BuildMode) -> Result<(), Error> { let process_steps = Build::get_process_steps(mode); diff --git a/src/command/mod.rs b/src/command/mod.rs index 9b20672..fe001fa 100644 --- a/src/command/mod.rs +++ b/src/command/mod.rs @@ -84,7 +84,7 @@ pub fn run_wasm_pack(command: Command, log: &Logger) -> result::Result<(), Error "normal" => BuildMode::Normal, _ => BuildMode::Normal, }; - Build::from(build_opts).run(&log, &build_mode) + Build::try_from_opts(build_opts).and_then(|mut b| b.run(&log, &build_mode)) } Command::Pack { path } => { info!(&log, "Running pack command..."); From 7fff2b6a15bbf4d35e44d736127d8ef23203d258 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 31 Jul 2018 15:47:24 -0700 Subject: [PATCH 3/4] Better error message when running wasm-pack in a non-crate directory --- src/manifest.rs | 6 ++++++ tests/all/build.rs | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/manifest.rs b/src/manifest.rs index 2cc3919..81d464f 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -68,6 +68,12 @@ struct Repository { fn read_cargo_toml(path: &Path) -> Result { let manifest_path = path.join("Cargo.toml"); + if !manifest_path.is_file() { + return Error::crate_config(&format!( + "Crate directory is missing a `Cargo.toml` file; is `{}` the wrong directory?", + path.display() + )).map(|_| unreachable!()); + } let mut cargo_file = File::open(manifest_path)?; let mut cargo_contents = String::new(); cargo_file.read_to_string(&mut cargo_contents)?; diff --git a/tests/all/build.rs b/tests/all/build.rs index ea66d3b..ea821b5 100644 --- a/tests/all/build.rs +++ b/tests/all/build.rs @@ -11,8 +11,11 @@ fn build_in_non_crate_directory_doesnt_panic() { &fixture.path.display().to_string(), ]).unwrap(); let logger = logger::new(&cli.cmd, cli.verbosity).unwrap(); + let result = command::run_wasm_pack(cli.cmd, &logger); assert!( - command::run_wasm_pack(cli.cmd, &logger).is_err(), + result.is_err(), "running wasm-pack in a non-crate directory should fail, but it should not panic" ); + let err_msg = result.unwrap_err().to_string(); + assert!(err_msg.contains("missing a `Cargo.toml`")); } From a32b03d7a1667378cedc416ca25738f4eb5f3096 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Tue, 31 Jul 2018 16:21:27 -0700 Subject: [PATCH 4/4] rustfmt: Re run `cargo fmt` for new nightly --- src/logger.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/logger.rs b/src/logger.rs index d100ccf..0f272a9 100644 --- a/src/logger.rs +++ b/src/logger.rs @@ -11,7 +11,10 @@ use std::path::PathBuf; /// Create the logger for wasm-pack that will output any info warning or errors we encounter pub fn new(cmd: &Command, verbosity: u8) -> Result { let log_path = log_file_path(&cmd); - let file = OpenOptions::new().create(true).append(true).open(log_path)?; + let file = OpenOptions::new() + .create(true) + .append(true) + .open(log_path)?; let decorator = PlainDecorator::new(file); let drain = FullFormat::new(decorator).build().fuse();