Auto merge of #9767 - nipunn1313:refactor, r=alexcrichton

Refactor fake_file() away from cargo_command tests

There are already similar preexisting test helpers which
also work on windows. Port over the executable-creation
helper into the file() helper and things appear to pass.

Was inspired to do this when trying to add tests to solve a separate issue.

Author: bors

crates/cargo-test-support/src/lib.rs

@@ -70,20 +70,36 @@ pub mod tools;
 struct FileBuilder {
     path: PathBuf,
     body: String,
+    executable: bool,
 }
 
 impl FileBuilder {
-    pub fn new(path: PathBuf, body: &str) -> FileBuilder {
+    pub fn new(path: PathBuf, body: &str, executable: bool) -> FileBuilder {
         FileBuilder {
             path,
             body: body.to_string(),
+            executable: executable,
         }
     }
 
-    fn mk(&self) {
+    fn mk(&mut self) {
+        if self.executable {
+            self.path.set_extension(env::consts::EXE_EXTENSION);
+        }
+
         self.dirname().mkdir_p();
         fs::write(&self.path, &self.body)
             .unwrap_or_else(|e| panic!("could not create file {}: {}", self.path.display(), e));
+
+        #[cfg(unix)]
+        if self.executable {
+            use std::os::unix::fs::PermissionsExt;
+
+            let mut perms = fs::metadata(&self.path).unwrap().permissions();
+            let mode = perms.mode();
+            perms.set_mode(mode | 0o111);
+            fs::set_permissions(&self.path, perms).unwrap();
+        }
     }
 
     fn dirname(&self) -> &Path {
@@ -122,11 +138,16 @@ impl SymlinkBuilder {
     }
 
     #[cfg(windows)]
-    fn mk(&self) {
+    fn mk(&mut self) {
         self.dirname().mkdir_p();
         if self.src_is_dir {
             t!(os::windows::fs::symlink_dir(&self.dst, &self.src));
         } else {
+            if let Some(ext) = self.dst.extension() {
+                if ext == env::consts::EXE_EXTENSION {
+                    self.src.set_extension(ext);
+                }
+            }
             t!(os::windows::fs::symlink_file(&self.dst, &self.src));
         }
     }
@@ -177,13 +198,22 @@ impl ProjectBuilder {
 
     /// Adds a file to the project.
     pub fn file<B: AsRef<Path>>(mut self, path: B, body: &str) -> Self {
-        self._file(path.as_ref(), body);
+        self._file(path.as_ref(), body, false);
         self
     }
 
-    fn _file(&mut self, path: &Path, body: &str) {
-        self.files
-            .push(FileBuilder::new(self.root.root().join(path), body));
+    /// Adds an executable file to the project.
+    pub fn executable<B: AsRef<Path>>(mut self, path: B, body: &str) -> Self {
+        self._file(path.as_ref(), body, true);
+        self
+    }
+
+    fn _file(&mut self, path: &Path, body: &str, executable: bool) {
+        self.files.push(FileBuilder::new(
+            self.root.root().join(path),
+            body,
+            executable,
+        ));
     }
 
     /// Adds a symlink to a file to the project.
@@ -219,13 +249,17 @@ impl ProjectBuilder {
 
         let manifest_path = self.root.root().join("Cargo.toml");
         if !self.no_manifest && self.files.iter().all(|fb| fb.path != manifest_path) {
-            self._file(Path::new("Cargo.toml"), &basic_manifest("foo", "0.0.1"))
+            self._file(
+                Path::new("Cargo.toml"),
+                &basic_manifest("foo", "0.0.1"),
+                false,
+            )
         }
 
         let past = time::SystemTime::now() - Duration::new(1, 0);
         let ftime = filetime::FileTime::from_system_time(past);
 
-        for file in self.files.iter() {
+        for file in self.files.iter_mut() {
             file.mk();
             if is_coarse_mtime() {
                 // Place the entire project 1 second in the past to ensure
@@ -237,7 +271,7 @@ impl ProjectBuilder {
             }
         }
 
-        for symlink in self.symlinks.iter() {
+        for symlink in self.symlinks.iter_mut() {
             symlink.mk();
         }
 
@@ -316,7 +350,7 @@ impl Project {
 
     /// Changes the contents of an existing file.
     pub fn change_file(&self, path: &str, body: &str) {
-        FileBuilder::new(self.root().join(path), body).mk()
+        FileBuilder::new(self.root().join(path), body, false).mk()
     }
 
     /// Creates a `ProcessBuilder` to run a program in the project

tests/testsuite/cargo_command.rs

@@ -1,63 +1,16 @@
 //! Tests for custom cargo commands and other global command features.
 
 use std::env;
-use std::fs::{self, File};
+use std::fs;
 use std::io::Read;
 use std::path::{Path, PathBuf};
 use std::process::Stdio;
 use std::str;
 
 use cargo_test_support::cargo_process;
-use cargo_test_support::paths::{self, CargoPathExt};
+use cargo_test_support::paths;
 use cargo_test_support::registry::Package;
-use cargo_test_support::{basic_bin_manifest, basic_manifest, cargo_exe, project, Project};
-
-#[cfg_attr(windows, allow(dead_code))]
-enum FakeKind<'a> {
-    Executable,
-    Symlink { target: &'a Path },
-}
-
-/// Adds an empty file with executable flags (and platform-dependent suffix).
-//
-// TODO: move this to `Project` if other cases using this emerge.
-fn fake_file(proj: Project, dir: &Path, name: &str, kind: &FakeKind<'_>) -> Project {
-    let path = proj
-        .root()
-        .join(dir)
-        .join(&format!("{}{}", name, env::consts::EXE_SUFFIX));
-    path.parent().unwrap().mkdir_p();
-    match *kind {
-        FakeKind::Executable => {
-            File::create(&path).unwrap();
-            make_executable(&path);
-        }
-        FakeKind::Symlink { target } => {
-            make_symlink(&path, target);
-        }
-    }
-    return proj;
-
-    #[cfg(unix)]
-    fn make_executable(p: &Path) {
-        use std::os::unix::prelude::*;
-
-        let mut perms = fs::metadata(p).unwrap().permissions();
-        let mode = perms.mode();
-        perms.set_mode(mode | 0o111);
-        fs::set_permissions(p, perms).unwrap();
-    }
-    #[cfg(windows)]
-    fn make_executable(_: &Path) {}
-    #[cfg(unix)]
-    fn make_symlink(p: &Path, t: &Path) {
-        ::std::os::unix::fs::symlink(t, p).expect("Failed to create symlink");
-    }
-    #[cfg(windows)]
-    fn make_symlink(_: &Path, _: &Path) {
-        panic!("Not supported")
-    }
-}
+use cargo_test_support::{basic_bin_manifest, basic_manifest, cargo_exe, project};
 
 fn path() -> Vec<PathBuf> {
     env::split_paths(&env::var_os("PATH").unwrap_or_default()).collect()
@@ -91,13 +44,9 @@ fn list_aliases_with_descriptions() {
 
 #[cargo_test]
 fn list_command_looks_at_path() {
-    let proj = project().build();
-    let proj = fake_file(
-        proj,
-        Path::new("path-test"),
-        "cargo-1",
-        &FakeKind::Executable,
-    );
+    let proj = project()
+        .executable(Path::new("path-test").join("cargo-1"), "")
+        .build();
 
     let mut path = path();
     path.push(proj.root().join("path-test"));
@@ -114,19 +63,11 @@ fn list_command_looks_at_path() {
     );
 }
 
-// Windows and symlinks don't currently mix well.
-#[cfg(unix)]
 #[cargo_test]
 fn list_command_resolves_symlinks() {
-    let proj = project().build();
-    let proj = fake_file(
-        proj,
-        Path::new("path-test"),
-        "cargo-2",
-        &FakeKind::Symlink {
-            target: &cargo_exe(),
-        },
-    );
+    let proj = project()
+        .symlink(cargo_exe(), Path::new("path-test").join("cargo-2"))
+        .build();
 
     let mut path = path();
     path.push(proj.root().join("path-test"));