updating tests

Author: perryqh

Cargo.lock

@@ -34,6 +34,6 @@ tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
 [dev-dependencies]
 assert_cmd = "2.0.16"
 rusty-hook = "^0.11.2"
-predicates = "3.1.2"
+predicates = "3.1.3"
 pretty_assertions = "1.4.1" # Shows a more readable diff when comparing objects
 indoc = "2.0.5"

Cargo.toml

@@ -66,12 +66,12 @@ impl<'a> ProjectBuilder<'a> {
         builder.filter_entry(move |entry: &DirEntry| {
             let path = entry.path();
             let file_name = entry.file_name().to_str().unwrap_or("");
-            if let Some(tracked_files) = &tracked_files {
-                if let Some(ft) = entry.file_type() {
-                    if ft.is_file() && !tracked_files.contains_key(path) {
-                        return false;
-                    }
-                }
+            if let Some(tracked_files) = &tracked_files
+                && let Some(ft) = entry.file_type()
+                && ft.is_file()
+                && !tracked_files.contains_key(path)
+            {
+                return false;
             }
             if let Some(ft) = entry.file_type()
                 && ft.is_dir()

src/project_builder.rs

@@ -42,7 +42,7 @@ mod tests {
 
         std::fs::write(tmp_dir.path().join("test.txt"), "test").unwrap();
         let tracked = find_tracked_files(tmp_dir.path()).unwrap();
-        assert!(tracked.len() == 0);
+        assert!(tracked.is_empty());
 
         std::process::Command::new("git")
             .arg("add")

src/tracked_files.rs

@@ -1,10 +1,47 @@
+use assert_cmd::prelude::*;
+use codeowners::runner::{self, RunConfig};
+use predicates::prelude::*;
 use std::fs;
 use std::path::Path;
-use std::process::Command;
-
-use codeowners::runner::{self, RunConfig};
+use std::{error::Error, process::Command};
 use tempfile::TempDir;
 
+#[allow(dead_code)]
+pub enum OutputStream {
+    Stdout,
+    Stderr,
+}
+
+#[allow(dead_code)]
+pub fn run_codeowners<I, P>(
+    relative_fixture_path: &str,
+    command: &[&str],
+    success: bool,
+    stream: OutputStream,
+    output_predicate: I,
+) -> Result<(), Box<dyn Error>>
+where
+    I: assert_cmd::assert::IntoOutputPredicate<P>,
+    P: Predicate<[u8]>,
+{
+    let fixture_root = Path::new("tests/fixtures").join(relative_fixture_path);
+    let temp_dir = setup_fixture_repo(&fixture_root);
+    let project_root = temp_dir.path();
+    git_add_all_files(project_root);
+    let mut cmd = Command::cargo_bin("codeowners")?;
+    let assert = cmd.arg("--project-root").arg(project_root).arg("--no-cache").args(command).assert();
+    let assert = if success { assert.success() } else { assert.failure() };
+    match stream {
+        OutputStream::Stdout => {
+            assert.stdout(output_predicate);
+        }
+        OutputStream::Stderr => {
+            assert.stderr(output_predicate);
+        }
+    }
+    Ok(())
+}
+
 #[allow(dead_code)]
 pub fn teardown() {
     glob::glob("tests/fixtures/*/tmp/cache/codeowners")

tests/common/mod.rs

@@ -1,76 +1,73 @@
-use assert_cmd::prelude::*;
 use indoc::indoc;
 use predicates::prelude::*;
-use std::{error::Error, process::Command};
+use std::error::Error;
+
+mod common;
+use common::OutputStream;
+use common::run_codeowners;
 
 #[test]
 fn test_validate() -> Result<(), Box<dyn Error>> {
-    Command::cargo_bin("codeowners")?
-        .arg("--project-root")
-        .arg("tests/fixtures/invalid_project")
-        .arg("--no-cache")
-        .arg("validate")
-        .assert()
-        .failure()
-        .stdout(predicate::eq(indoc! {"
+    run_codeowners(
+        "invalid_project",
+        &["validate"],
+        false,
+        OutputStream::Stdout,
+        predicate::eq(indoc! {"
 
-            CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file
+    CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file
 
-            Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways
+    Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways
 
-            gems/payroll_calculator/calculator.rb
-             owner: Payments
-              - Owner annotation at the top of the file
-             owner: Payroll
-              - Owner specified in Team YML's `owned_gems`
+    gems/payroll_calculator/calculator.rb
+     owner: Payments
+      - Owner annotation at the top of the file
+     owner: Payroll
+      - Owner specified in Team YML's `owned_gems`
 
-            ruby/app/services/multi_owned.rb
-             owner: Payments
-              - Owner annotation at the top of the file
-             owner: Payroll
-              - Owner specified in `ruby/app/services/.codeowner`
+    ruby/app/services/multi_owned.rb
+     owner: Payments
+      - Owner annotation at the top of the file
+     owner: Payroll
+      - Owner specified in `ruby/app/services/.codeowner`
 
-            Found invalid team annotations
-            - ruby/app/models/blockchain.rb is referencing an invalid team - 'Web3'
+    Found invalid team annotations
+    - ruby/app/models/blockchain.rb is referencing an invalid team - 'Web3'
 
-            Some files are missing ownership
-            - ruby/app/unowned.rb
+    Some files are missing ownership
+    - ruby/app/unowned.rb
 
-        "}));
+    "}),
+    )?;
     Ok(())
 }
 
 #[test]
 fn test_for_file() -> Result<(), Box<dyn Error>> {
-    Command::cargo_bin("codeowners")?
-        .arg("--project-root")
-        .arg("tests/fixtures/invalid_project")
-        .arg("--no-cache")
-        .arg("for-file")
-        .arg("ruby/app/models/blockchain.rb")
-        .assert()
-        .success()
-        .stdout(predicate::eq(indoc! {"
+    run_codeowners(
+        "invalid_project",
+        &["for-file", "ruby/app/models/blockchain.rb"],
+        true,
+        OutputStream::Stdout,
+        predicate::eq(indoc! {"
             Team: Unowned
             Github Team: Unowned
             Team YML: 
             Description:
             - Unowned
-            "}));
+            "}),
+    )?;
     Ok(())
 }
 
 #[test]
 fn test_for_file_multiple_owners() -> Result<(), Box<dyn Error>> {
-    Command::cargo_bin("codeowners")?
-        .arg("--project-root")
-        .arg("tests/fixtures/invalid_project")
-        .arg("--no-cache")
-        .arg("for-file")
-        .arg("ruby/app/services/multi_owned.rb")
-        .assert()
-        .failure()
-        .stdout(predicate::eq(indoc! {"
+    run_codeowners(
+        "invalid_project",
+        &["for-file", "ruby/app/services/multi_owned.rb"],
+        false,
+        OutputStream::Stdout,
+        predicate::eq(indoc! {"
             Error: file is owned by multiple teams!
             
             Team: Payments
@@ -84,6 +81,7 @@ fn test_for_file_multiple_owners() -> Result<(), Box<dyn Error>> {
             Team YML: config/teams/payroll.yml
             Description:
             - Owner specified in `ruby/app/services/.codeowner`
-        "}));
+        "}),
+    )?;
     Ok(())
 }

tests/invalid_project_test.rs

@@ -1,30 +1,36 @@
-use assert_cmd::prelude::*;
-use std::{error::Error, path::Path, process::Command};
+use std::{error::Error, path::Path};
+
+use common::OutputStream;
+use common::run_codeowners;
+use predicates::prelude::*;
+
+mod common;
 
 #[test]
 fn test_validate() -> Result<(), Box<dyn Error>> {
-    Command::cargo_bin("codeowners")?
-        .arg("--project-root")
-        .arg("tests/fixtures/multiple-directory-owners")
-        .arg("--no-cache")
-        .arg("validate")
-        .assert()
-        .success();
+    run_codeowners(
+        "multiple-directory-owners",
+        &["validate"],
+        true,
+        OutputStream::Stdout,
+        predicate::eq(""),
+    )?;
 
     Ok(())
 }
 
 #[test]
 fn test_generate() -> Result<(), Box<dyn Error>> {
-    Command::cargo_bin("codeowners")?
-        .arg("--project-root")
-        .arg("tests/fixtures/multiple-directory-owners")
-        .arg("--codeowners-file-path")
-        .arg("../../../tmp/CODEOWNERS")
-        .arg("--no-cache")
-        .arg("generate")
-        .assert()
-        .success();
+    let codeowners_abs = std::env::current_dir()?.join("tmp/CODEOWNERS");
+    let codeowners_str = codeowners_abs.to_str().unwrap();
+
+    run_codeowners(
+        "multiple-directory-owners",
+        &["--codeowners-file-path", codeowners_str, "generate"],
+        true,
+        OutputStream::Stdout,
+        predicate::eq(""),
+    )?;
 
     let expected_codeowners: String = std::fs::read_to_string(Path::new("tests/fixtures/multiple-directory-owners/.github/CODEOWNERS"))?;
     let actual_codeowners: String = std::fs::read_to_string(Path::new("tmp/CODEOWNERS"))?;