From 26a5eeffa9b6751687421a851a78f2b3b772ec32 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 17 Dec 2015 09:53:14 -0800 Subject: [PATCH] Consolidate creating processes in tests Each test wants to be sure to reset HOME and remove CARGO_HOME from the environment, but this was done inconsistently throughout the test suite. This commit consolidates process creation so there's only one point for creating a process ready to execute the Cargo that's being tested. --- tests/support/git.rs | 4 ++++ tests/support/mod.rs | 10 +++----- tests/test_cargo.rs | 46 ++++++++++++++++++------------------ tests/test_cargo_install.rs | 18 +++++++------- tests/test_cargo_new.rs | 6 ++--- tests/test_cargo_package.rs | 9 ++++--- tests/test_cargo_registry.rs | 8 ++----- tests/test_cargo_search.rs | 10 ++++---- tests/test_shell.rs | 6 ++--- tests/tests.rs | 15 ++++++++++++ 10 files changed, 69 insertions(+), 63 deletions(-) diff --git a/tests/support/git.rs b/tests/support/git.rs index a84e5287dee..54b95ae79b4 100644 --- a/tests/support/git.rs +++ b/tests/support/git.rs @@ -53,6 +53,10 @@ impl RepoBuilder { "Initial commit", &tree, &[]).unwrap(); } + pub fn root(&self) -> &Path { + self.repo.workdir().unwrap() + } + pub fn url(&self) -> Url { path2url(self.repo.workdir().unwrap().to_path_buf()) } diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 627f989e184..00796905155 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -135,13 +135,9 @@ impl ProjectBuilder { } pub fn process>(&self, program: T) -> ProcessBuilder { - let mut p = process(program); - p.cwd(&self.root()) - .env("HOME", &paths::home()) - .env_remove("CARGO_HOME") // make sure we don't pick up an outer one - .env_remove("CARGO_TARGET_DIR") // we assume 'target' - .env_remove("MSYSTEM"); // assume cmd.exe everywhere on windows - return p; + let mut p = ::process(program); + p.cwd(self.root()); + return p } pub fn cargo(&self, cmd: &str) -> ProcessBuilder { diff --git a/tests/test_cargo.rs b/tests/test_cargo.rs index 7ca12e0632f..3bcc2e56db7 100644 --- a/tests/test_cargo.rs +++ b/tests/test_cargo.rs @@ -4,10 +4,10 @@ use std::fs::{self, File}; use std::io::prelude::*; use std::path::{Path, PathBuf}; use std::str; -use cargo::util::process; +use cargo_process; use support::paths; -use support::{execs, project, cargo_dir, mkdir_recursive, ProjectBuilder}; +use support::{execs, project, mkdir_recursive, ProjectBuilder}; use hamcrest::{assert_that}; fn setup() { @@ -43,9 +43,7 @@ fn path() -> Vec { test!(list_commands_looks_at_path { let proj = project("list-non-overlapping"); let proj = fake_executable(proj, &Path::new("path-test"), "cargo-1"); - let mut pr = process(&cargo_dir().join("cargo")); - pr.cwd(&proj.root()) - .env("HOME", &paths::home()); + let mut pr = cargo_process(); let mut path = path(); path.push(proj.root().join("path-test")); @@ -58,8 +56,8 @@ test!(list_commands_looks_at_path { }); test!(find_closest_biuld_to_build { - let mut pr = process(&cargo_dir().join("cargo")); - pr.arg("biuld").cwd(&paths::root()).env("HOME", &paths::home()); + let mut pr = cargo_process(); + pr.arg("biuld"); assert_that(pr, execs().with_status(101) @@ -72,8 +70,16 @@ Did you mean `build`? // if a subcommand is more than 3 edit distance away, we don't make a suggestion test!(find_closest_dont_correct_nonsense { - let mut pr = process(&cargo_dir().join("cargo")); - pr.arg("asdf").cwd(&paths::root()).env("HOME", &paths::home()); + let paths = path().into_iter().filter(|p| { + fs::read_dir(p).into_iter() + .flat_map(|i| i) + .filter_map(|e| e.ok()) + .all(|e| !e.file_name().to_str().unwrap_or("").starts_with("cargo-")) + }); + let mut pr = cargo_process(); + pr.arg("asdf") + .cwd(&paths::root()) + .env("PATH", env::join_paths(paths).unwrap()); assert_that(pr, execs().with_status(101) @@ -92,11 +98,9 @@ test!(override_cargo_home { git = false "#).unwrap(); - assert_that(process(&cargo_dir().join("cargo")) + assert_that(cargo_process() .arg("new").arg("foo") - .cwd(&paths::root()) .env("USER", "foo") - .env("HOME", &paths::home()) .env("CARGO_HOME", &my_home), execs().with_status(0)); @@ -107,22 +111,18 @@ test!(override_cargo_home { }); test!(cargo_help { - assert_that(process(&cargo_dir().join("cargo")), + assert_that(cargo_process(), execs().with_status(0)); - assert_that(process(&cargo_dir().join("cargo")).arg("help"), + assert_that(cargo_process().arg("help"), execs().with_status(0)); - assert_that(process(&cargo_dir().join("cargo")).arg("-h"), + assert_that(cargo_process().arg("-h"), execs().with_status(0)); - assert_that(process(&cargo_dir().join("cargo")) - .arg("help").arg("build"), + assert_that(cargo_process().arg("help").arg("build"), execs().with_status(0)); - assert_that(process(&cargo_dir().join("cargo")) - .arg("build").arg("-h"), + assert_that(cargo_process().arg("build").arg("-h"), execs().with_status(0)); - assert_that(process(&cargo_dir().join("cargo")) - .arg("help").arg("-h"), + assert_that(cargo_process().arg("help").arg("-h"), execs().with_status(0)); - assert_that(process(&cargo_dir().join("cargo")) - .arg("help").arg("help"), + assert_that(cargo_process().arg("help").arg("help"), execs().with_status(0)); }); diff --git a/tests/test_cargo_install.rs b/tests/test_cargo_install.rs index 5897a880ae8..97057a42f50 100644 --- a/tests/test_cargo_install.rs +++ b/tests/test_cargo_install.rs @@ -3,10 +3,10 @@ use std::fs::{self, File}; use std::io::prelude::*; use std::path::{Path, PathBuf}; -use cargo::util::{process, ProcessBuilder}; +use cargo::util::ProcessBuilder; use hamcrest::{assert_that, existing_file, is_not, Matcher, MatchResult}; -use support::{project, execs, cargo_dir}; +use support::{project, execs}; use support::{UPDATING, DOWNLOADING, COMPILING, INSTALLING, REMOVING}; use support::paths; use support::registry::Package; @@ -17,6 +17,12 @@ use self::InstalledExe as has_installed_exe; fn setup() { } +fn cargo_process(s: &str) -> ProcessBuilder { + let mut p = ::cargo_process(); + p.arg(s); + return p +} + fn pkg(name: &str, vers: &str) { Package::new(name, vers) .file("src/lib.rs", "") @@ -27,14 +33,6 @@ fn pkg(name: &str, vers: &str) { .publish() } -fn cargo_process(s: &str) -> ProcessBuilder { - let mut p = process(&cargo_dir().join("cargo")); - p.arg(s).cwd(&paths::root()) - .env("HOME", &paths::home()) - .env_remove("CARGO_HOME"); - return p; -} - fn exe(name: &str) -> String { if cfg!(windows) {format!("{}.exe", name)} else {name.to_string()} } diff --git a/tests/test_cargo_new.rs b/tests/test_cargo_new.rs index 7e8f8c02398..8c61d05ae27 100644 --- a/tests/test_cargo_new.rs +++ b/tests/test_cargo_new.rs @@ -3,7 +3,7 @@ use std::io::prelude::*; use std::env; use tempdir::TempDir; -use support::{execs, paths, cargo_dir}; +use support::{execs, paths}; use support::paths::CargoPathExt; use hamcrest::{assert_that, existing_file, existing_dir, is_not}; @@ -19,8 +19,8 @@ fn my_process(s: &str) -> ProcessBuilder { } fn cargo_process(s: &str) -> ProcessBuilder { - let mut p = process(&cargo_dir().join("cargo")); - p.arg(s).cwd(&paths::root()).env("HOME", &paths::home()); + let mut p = ::cargo_process(); + p.arg(s); return p; } diff --git a/tests/test_cargo_package.rs b/tests/test_cargo_package.rs index 7220dab4ba8..1a26ba359b9 100644 --- a/tests/test_cargo_package.rs +++ b/tests/test_cargo_package.rs @@ -3,12 +3,11 @@ use std::io::Cursor; use std::io::prelude::*; use std::path::Path; -use cargo::util::process; use flate2::read::GzDecoder; use git2; use tar::Archive; -use support::{project, execs, cargo_dir, paths, git, path2url}; +use support::{project, execs, paths, git, path2url}; use support::{PACKAGING, VERIFYING, COMPILING, ARCHIVING, UPDATING, DOWNLOADING}; use support::registry::{self, Package}; use hamcrest::{assert_that, existing_file}; @@ -215,8 +214,8 @@ test!(package_verbose { "#) .file("a/src/lib.rs", ""); p.build(); - let mut cargo = process(&cargo_dir().join("cargo")); - cargo.cwd(&root).env("HOME", &paths::home()); + let mut cargo = ::cargo_process(); + cargo.cwd(p.root()); assert_that(cargo.clone().arg("build"), execs().with_status(0)); assert_that(cargo.arg("package").arg("-v").arg("--no-verify"), execs().with_status(0).with_stdout(&format!("\ @@ -330,7 +329,7 @@ test!(package_new_git_repo { p.build(); git2::Repository::init(&p.root()).unwrap(); - assert_that(p.process(cargo_dir().join("cargo")).arg("package") + assert_that(::cargo_process().arg("package").cwd(p.root()) .arg("--no-verify").arg("-v"), execs().with_status(0).with_stdout(&format!("\ {packaging} foo v0.0.1 ([..]) diff --git a/tests/test_cargo_registry.rs b/tests/test_cargo_registry.rs index 064a417a487..e9a4528757f 100644 --- a/tests/test_cargo_registry.rs +++ b/tests/test_cargo_registry.rs @@ -1,8 +1,7 @@ use std::fs::{self, File}; use std::io::prelude::*; -use cargo::util::process; -use support::{project, execs, cargo_dir}; +use support::{project, execs}; use support::{UPDATING, DOWNLOADING, COMPILING, PACKAGING, VERIFYING, ADDING, REMOVING}; use support::paths::{self, CargoPathExt}; use support::registry::{self, Package}; @@ -563,10 +562,7 @@ test!(dev_dependency_not_used { test!(login_with_no_cargo_dir { let home = paths::home().join("new-home"); fs::create_dir(&home).unwrap(); - assert_that(process(&cargo_dir().join("cargo")) - .arg("login").arg("foo").arg("-v") - .cwd(&paths::root()) - .env("HOME", &home), + assert_that(::cargo_process().arg("login").arg("foo").arg("-v"), execs().with_status(0)); }); diff --git a/tests/test_cargo_search.rs b/tests/test_cargo_search.rs index 2e269919bc4..db754ffe279 100644 --- a/tests/test_cargo_search.rs +++ b/tests/test_cargo_search.rs @@ -4,9 +4,9 @@ use std::path::PathBuf; use url::Url; -use cargo::util::{process, ProcessBuilder}; +use cargo::util::ProcessBuilder; use support::UPDATING; -use support::{execs, cargo_dir}; +use support::execs; use support::paths; use support::git::repo; @@ -35,9 +35,9 @@ fn setup() { } fn cargo_process(s: &str) -> ProcessBuilder { - let mut b = process(&cargo_dir().join("cargo")); - b.arg(s).cwd(&paths::root()).env("HOME", &paths::home()); - b + let mut b = ::cargo_process(); + b.arg(s); + return b } test!(simple { diff --git a/tests/test_shell.rs b/tests/test_shell.rs index 366215be4ee..8d74c3aa2c3 100644 --- a/tests/test_shell.rs +++ b/tests/test_shell.rs @@ -6,9 +6,8 @@ use hamcrest::{assert_that}; use cargo::core::shell::{Shell, ShellConfig}; use cargo::core::shell::ColorConfig::{Auto,Always, Never}; -use cargo::util::process; -use support::{Tap, cargo_dir, execs, shell_writes}; +use support::{Tap, execs, shell_writes}; fn setup() { } @@ -81,8 +80,7 @@ test!(color_explicitly_enabled { test!(no_term { // Verify that shell creation is successful when $TERM does not exist. - assert_that(process(&cargo_dir().join("cargo")) - .env_remove("TERM"), + assert_that(::cargo_process().env_remove("TERM"), execs().with_stderr("")); }); diff --git a/tests/tests.rs b/tests/tests.rs index 19aaf0afff0..761fee61bb0 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -17,6 +17,7 @@ extern crate url; extern crate log; use cargo::util::Rustc; +use std::ffi::OsStr; mod support; macro_rules! test { @@ -81,3 +82,17 @@ fn is_nightly() -> bool { fn can_panic() -> bool { RUSTC.with(|r| !(r.host.contains("msvc") && !r.host.contains("x86_64"))) } + +fn process>(t: T) -> cargo::util::ProcessBuilder { + let mut p = cargo::util::process(t.as_ref()); + p.cwd(&support::paths::root()) + .env("HOME", &support::paths::home()) + .env_remove("CARGO_HOME") + .env_remove("CARGO_TARGET_DIR") // we assume 'target' + .env_remove("MSYSTEM"); // assume cmd.exe everywhere on windows + return p +} + +fn cargo_process() -> cargo::util::ProcessBuilder { + process(&support::cargo_dir().join("cargo")) +}