Auto merge of #2223 - alexcrichton:better-dev-experience, r=brson

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

@@ -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())
     }

tests/support/mod.rs

@@ -135,13 +135,9 @@ impl ProjectBuilder {
     }
 
     pub fn process<T: AsRef<OsStr>>(&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 {

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<PathBuf> {
 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));
 });

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()}
 }

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;
 }
 

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 ([..])

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));
 });
 

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 {

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(""));
 });
 

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: AsRef<OsStr>>(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"))
+}