From 8689278b354aa80f40f8877813d91146a731c0fd Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Mon, 11 Aug 2025 11:18:26 -0500 Subject: [PATCH 1/3] adding git stage support --- src/cli.rs | 14 +++-- src/runner.rs | 31 +++++++--- tests/git_stage_test.rs | 134 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 166 insertions(+), 13 deletions(-) create mode 100644 tests/git_stage_test.rs diff --git a/src/cli.rs b/src/cli.rs index 1f94124..714487c 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -26,7 +26,10 @@ enum Command { about = "Generate the CODEOWNERS file and save it to '--codeowners-file-path'.", visible_alias = "g" )] - Generate, + Generate { + #[arg(long, short, default_value = "false", help = "Skip staging the CODEOWNERS file")] + skip_stage: bool, + }, #[clap( about = "Validate the validity of the CODEOWNERS file. A validation failure will exit with a failure code and a detailed output of the validation errors.", @@ -35,7 +38,10 @@ enum Command { Validate, #[clap(about = "Chains both `generate` and `validate` commands.", visible_alias = "gv")] - GenerateAndValidate, + GenerateAndValidate { + #[arg(long, short, default_value = "false", help = "Skip staging the CODEOWNERS file")] + skip_stage: bool, + }, #[clap(about = "Delete the cache file.", visible_alias = "d")] DeleteCache, @@ -101,8 +107,8 @@ pub fn cli() -> Result { let runner_result = match args.command { Command::Validate => runner::validate(&run_config, vec![]), - Command::Generate => runner::generate(&run_config), - Command::GenerateAndValidate => runner::generate_and_validate(&run_config, vec![]), + Command::Generate { skip_stage } => runner::generate(&run_config, !skip_stage), + Command::GenerateAndValidate { skip_stage } => runner::generate_and_validate(&run_config, vec![], !skip_stage), Command::ForFile { name, fast } => runner::for_file(&run_config, &name, fast), Command::ForTeam { name } => runner::for_team(&run_config, &name), Command::DeleteCache => runner::delete_cache(&run_config), diff --git a/src/runner.rs b/src/runner.rs index 2e5fd11..ef53829 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -2,6 +2,7 @@ use core::fmt; use std::{ fs::File, path::{Path, PathBuf}, + process::Command, }; use error_stack::{Context, Result, ResultExt}; @@ -91,12 +92,12 @@ pub fn validate(run_config: &RunConfig, _file_paths: Vec) -> RunResult { run_with_runner(run_config, |runner| runner.validate()) } -pub fn generate(run_config: &RunConfig) -> RunResult { - run_with_runner(run_config, |runner| runner.generate()) +pub fn generate(run_config: &RunConfig, git_stage: bool) -> RunResult { + run_with_runner(run_config, |runner| runner.generate(git_stage)) } -pub fn generate_and_validate(run_config: &RunConfig, _file_paths: Vec) -> RunResult { - run_with_runner(run_config, |runner| runner.generate_and_validate()) +pub fn generate_and_validate(run_config: &RunConfig, _file_paths: Vec, git_stage: bool) -> RunResult { + run_with_runner(run_config, |runner| runner.generate_and_validate(git_stage)) } pub fn delete_cache(run_config: &RunConfig) -> RunResult { @@ -199,14 +200,18 @@ impl Runner { }, } } - - pub fn generate(&self) -> RunResult { + pub fn generate(&self, git_stage: bool) -> RunResult { let content = self.ownership.generate_file(); if let Some(parent) = &self.run_config.codeowners_file_path.parent() { let _ = std::fs::create_dir_all(parent); } match std::fs::write(&self.run_config.codeowners_file_path, content) { - Ok(_) => RunResult::default(), + Ok(_) => { + if git_stage { + self.git_stage(); + } + RunResult::default() + } Err(err) => RunResult { io_errors: vec![err.to_string()], ..Default::default() @@ -214,14 +219,22 @@ impl Runner { } } - pub fn generate_and_validate(&self) -> RunResult { - let run_result = self.generate(); + pub fn generate_and_validate(&self, git_stage: bool) -> RunResult { + let run_result = self.generate(git_stage); if run_result.has_errors() { return run_result; } self.validate() } + fn git_stage(&self) { + let _ = Command::new("git") + .arg("add") + .arg(&self.run_config.codeowners_file_path) + .current_dir(&self.run_config.project_root) + .output(); + } + pub fn for_file(&self, file_path: &str) -> RunResult { let relative_file_path = Path::new(file_path) .strip_prefix(&self.run_config.project_root) diff --git a/tests/git_stage_test.rs b/tests/git_stage_test.rs new file mode 100644 index 0000000..e6f029d --- /dev/null +++ b/tests/git_stage_test.rs @@ -0,0 +1,134 @@ +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::Command; + +use codeowners::runner::{self, RunConfig}; + +fn copy_dir_recursive(from: &Path, to: &Path) { + fs::create_dir_all(to).expect("failed to create destination root"); + for entry in fs::read_dir(from).expect("failed to read source dir") { + let entry = entry.expect("failed to read dir entry"); + let file_type = entry.file_type().expect("failed to read file type"); + let src_path = entry.path(); + let dest_path = to.join(entry.file_name()); + if file_type.is_dir() { + copy_dir_recursive(&src_path, &dest_path); + } else if file_type.is_file() { + // Ensure parent exists then copy + if let Some(parent) = dest_path.parent() { + fs::create_dir_all(parent).expect("failed to create parent dir"); + } + fs::copy(&src_path, &dest_path).expect("failed to copy file"); + } + } +} + +fn init_git_repo(path: &Path) { + // Initialize a new git repository in the temp project + let status = Command::new("git") + .arg("init") + .current_dir(path) + .output() + .expect("failed to run git init"); + assert!( + status.status.success(), + "git init failed: {}", + String::from_utf8_lossy(&status.stderr) + ); + + // Configure a dummy identity to appease git if commits ever happen; not strictly needed for staging + let _ = Command::new("git") + .arg("config") + .arg("user.email") + .arg("test@example.com") + .current_dir(path) + .output(); + let _ = Command::new("git") + .arg("config") + .arg("user.name") + .arg("Test User") + .current_dir(path) + .output(); +} + +fn is_file_staged(repo_root: &Path, rel_path: &str) -> bool { + // Use git diff --name-only --cached to list staged files + let output = Command::new("git") + .arg("diff") + .arg("--name-only") + .arg("--cached") + .current_dir(repo_root) + .output() + .expect("failed to run git diff --cached"); + assert!( + output.status.success(), + "git diff failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + stdout.lines().any(|line| line.trim() == rel_path) +} + +fn build_run_config(project_root: &Path, codeowners_rel_path: &str) -> RunConfig { + let project_root = project_root.canonicalize().expect("failed to canonicalize project root"); + let codeowners_file_path = project_root.join(codeowners_rel_path); + let config_path = project_root.join("config/code_ownership.yml"); + RunConfig { + project_root, + codeowners_file_path, + config_path, + no_cache: true, + } +} + +#[test] +fn test_generate_stages_codeowners() { + let temp_dir = tempfile::tempdir().expect("failed to create tempdir"); + let temp_root = temp_dir.path().to_path_buf(); + + // Copy the valid project fixture into a temporary git repo + let fixture_root = PathBuf::from("tests/fixtures/valid_project"); + copy_dir_recursive(&fixture_root, &temp_root); + init_git_repo(&temp_root); + + // Run generate with staging enabled, targeting the standard CODEOWNERS path + let run_config = build_run_config(&temp_root, ".github/CODEOWNERS"); + let result = runner::generate(&run_config, true); + assert!(result.io_errors.is_empty(), "io_errors: {:?}", result.io_errors); + assert!( + result.validation_errors.is_empty(), + "validation_errors: {:?}", + result.validation_errors + ); + + // Assert CODEOWNERS file exists and is staged + let rel_path = ".github/CODEOWNERS"; + assert!(run_config.codeowners_file_path.exists(), "CODEOWNERS file was not created"); + assert!(is_file_staged(&run_config.project_root, rel_path), "CODEOWNERS file was not staged"); +} + +#[test] +fn test_generate_and_validate_stages_codeowners() { + let temp_dir = tempfile::tempdir().expect("failed to create tempdir"); + let temp_root = temp_dir.path().to_path_buf(); + + // Copy the valid project fixture into a temporary git repo + let fixture_root = PathBuf::from("tests/fixtures/valid_project"); + copy_dir_recursive(&fixture_root, &temp_root); + init_git_repo(&temp_root); + + // Run generate_and_validate with staging enabled + let run_config = build_run_config(&temp_root, ".github/CODEOWNERS"); + let result = runner::generate_and_validate(&run_config, vec![], true); + assert!(result.io_errors.is_empty(), "io_errors: {:?}", result.io_errors); + assert!( + result.validation_errors.is_empty(), + "validation_errors: {:?}", + result.validation_errors + ); + + // Assert CODEOWNERS file exists and is staged + let rel_path = ".github/CODEOWNERS"; + assert!(run_config.codeowners_file_path.exists(), "CODEOWNERS file was not created"); + assert!(is_file_staged(&run_config.project_root, rel_path), "CODEOWNERS file was not staged"); +} From 650502e1b809013f58bf919124b49bca51f278b9 Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Mon, 11 Aug 2025 11:42:21 -0500 Subject: [PATCH 2/3] testing for skip false --- tests/common/mod.rs | 94 ++++++++++++++++++++++++++ tests/git_stage_test.rs | 142 ++++++++-------------------------------- 2 files changed, 120 insertions(+), 116 deletions(-) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 836002a..9ef7cb2 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -1,4 +1,9 @@ use std::fs; +use std::path::Path; +use std::process::Command; + +use codeowners::runner::{self, RunConfig}; +use tempfile::TempDir; #[allow(dead_code)] pub fn teardown() { @@ -11,3 +16,92 @@ pub fn teardown() { } }); } + +pub fn copy_dir_recursive(from: &Path, to: &Path) { + fs::create_dir_all(to).expect("failed to create destination root"); + for entry in fs::read_dir(from).expect("failed to read source dir") { + let entry = entry.expect("failed to read dir entry"); + let file_type = entry.file_type().expect("failed to read file type"); + let src_path = entry.path(); + let dest_path = to.join(entry.file_name()); + if file_type.is_dir() { + copy_dir_recursive(&src_path, &dest_path); + } else if file_type.is_file() { + if let Some(parent) = dest_path.parent() { + fs::create_dir_all(parent).expect("failed to create parent dir"); + } + fs::copy(&src_path, &dest_path).expect("failed to copy file"); + } + } +} + +pub fn init_git_repo(path: &Path) { + let status = Command::new("git") + .arg("init") + .current_dir(path) + .output() + .expect("failed to run git init"); + assert!( + status.status.success(), + "git init failed: {}", + String::from_utf8_lossy(&status.stderr) + ); + + let _ = Command::new("git") + .arg("config") + .arg("user.email") + .arg("test@example.com") + .current_dir(path) + .output(); + let _ = Command::new("git") + .arg("config") + .arg("user.name") + .arg("Test User") + .current_dir(path) + .output(); +} + +pub fn is_file_staged(repo_root: &Path, rel_path: &str) -> bool { + let output = Command::new("git") + .arg("diff") + .arg("--name-only") + .arg("--cached") + .current_dir(repo_root) + .output() + .expect("failed to run git diff --cached"); + assert!( + output.status.success(), + "git diff failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + stdout.lines().any(|line| line.trim() == rel_path) +} + +pub fn build_run_config(project_root: &Path, codeowners_rel_path: &str) -> RunConfig { + let project_root = project_root.canonicalize().expect("failed to canonicalize project root"); + let codeowners_file_path = project_root.join(codeowners_rel_path); + let config_path = project_root.join("config/code_ownership.yml"); + RunConfig { + project_root, + codeowners_file_path, + config_path, + no_cache: true, + } +} + +pub fn setup_fixture_repo(fixture_root: &Path) -> TempDir { + let temp_dir = tempfile::tempdir().expect("failed to create tempdir"); + copy_dir_recursive(fixture_root, temp_dir.path()); + init_git_repo(temp_dir.path()); + temp_dir +} + +pub fn assert_no_run_errors(result: &runner::RunResult) { + assert!(result.io_errors.is_empty(), "io_errors: {:?}", result.io_errors); + assert!( + result.validation_errors.is_empty(), + "validation_errors: {:?}", + result.validation_errors + ); +} diff --git a/tests/git_stage_test.rs b/tests/git_stage_test.rs index e6f029d..ae8f80d 100644 --- a/tests/git_stage_test.rs +++ b/tests/git_stage_test.rs @@ -1,134 +1,44 @@ -use std::fs; -use std::path::{Path, PathBuf}; -use std::process::Command; +use std::path::Path; use codeowners::runner::{self, RunConfig}; -fn copy_dir_recursive(from: &Path, to: &Path) { - fs::create_dir_all(to).expect("failed to create destination root"); - for entry in fs::read_dir(from).expect("failed to read source dir") { - let entry = entry.expect("failed to read dir entry"); - let file_type = entry.file_type().expect("failed to read file type"); - let src_path = entry.path(); - let dest_path = to.join(entry.file_name()); - if file_type.is_dir() { - copy_dir_recursive(&src_path, &dest_path); - } else if file_type.is_file() { - // Ensure parent exists then copy - if let Some(parent) = dest_path.parent() { - fs::create_dir_all(parent).expect("failed to create parent dir"); - } - fs::copy(&src_path, &dest_path).expect("failed to copy file"); - } - } -} - -fn init_git_repo(path: &Path) { - // Initialize a new git repository in the temp project - let status = Command::new("git") - .arg("init") - .current_dir(path) - .output() - .expect("failed to run git init"); - assert!( - status.status.success(), - "git init failed: {}", - String::from_utf8_lossy(&status.stderr) - ); +mod common; +use common::{assert_no_run_errors, build_run_config, is_file_staged, setup_fixture_repo}; - // Configure a dummy identity to appease git if commits ever happen; not strictly needed for staging - let _ = Command::new("git") - .arg("config") - .arg("user.email") - .arg("test@example.com") - .current_dir(path) - .output(); - let _ = Command::new("git") - .arg("config") - .arg("user.name") - .arg("Test User") - .current_dir(path) - .output(); +#[test] +fn test_generate_stages_codeowners() { + run_and_check(runner::generate, true, true); } -fn is_file_staged(repo_root: &Path, rel_path: &str) -> bool { - // Use git diff --name-only --cached to list staged files - let output = Command::new("git") - .arg("diff") - .arg("--name-only") - .arg("--cached") - .current_dir(repo_root) - .output() - .expect("failed to run git diff --cached"); - assert!( - output.status.success(), - "git diff failed: {}", - String::from_utf8_lossy(&output.stderr) - ); - let stdout = String::from_utf8_lossy(&output.stdout); - stdout.lines().any(|line| line.trim() == rel_path) +#[test] +fn test_generate_and_validate_stages_codeowners() { + run_and_check(|rc, s| runner::generate_and_validate(rc, vec![], s), true, true); } -fn build_run_config(project_root: &Path, codeowners_rel_path: &str) -> RunConfig { - let project_root = project_root.canonicalize().expect("failed to canonicalize project root"); - let codeowners_file_path = project_root.join(codeowners_rel_path); - let config_path = project_root.join("config/code_ownership.yml"); - RunConfig { - project_root, - codeowners_file_path, - config_path, - no_cache: true, - } +#[test] +fn test_generate_does_not_stage_codeowners() { + run_and_check(runner::generate, false, false); } #[test] -fn test_generate_stages_codeowners() { - let temp_dir = tempfile::tempdir().expect("failed to create tempdir"); - let temp_root = temp_dir.path().to_path_buf(); - - // Copy the valid project fixture into a temporary git repo - let fixture_root = PathBuf::from("tests/fixtures/valid_project"); - copy_dir_recursive(&fixture_root, &temp_root); - init_git_repo(&temp_root); - - // Run generate with staging enabled, targeting the standard CODEOWNERS path - let run_config = build_run_config(&temp_root, ".github/CODEOWNERS"); - let result = runner::generate(&run_config, true); - assert!(result.io_errors.is_empty(), "io_errors: {:?}", result.io_errors); - assert!( - result.validation_errors.is_empty(), - "validation_errors: {:?}", - result.validation_errors - ); - - // Assert CODEOWNERS file exists and is staged - let rel_path = ".github/CODEOWNERS"; - assert!(run_config.codeowners_file_path.exists(), "CODEOWNERS file was not created"); - assert!(is_file_staged(&run_config.project_root, rel_path), "CODEOWNERS file was not staged"); +fn test_generate_and_validate_does_not_stage_codeowners() { + run_and_check(|rc, s| runner::generate_and_validate(rc, vec![], s), false, false); } -#[test] -fn test_generate_and_validate_stages_codeowners() { - let temp_dir = tempfile::tempdir().expect("failed to create tempdir"); - let temp_root = temp_dir.path().to_path_buf(); +const FIXTURE: &str = "tests/fixtures/valid_project"; +const CODEOWNERS_REL: &str = ".github/CODEOWNERS"; - // Copy the valid project fixture into a temporary git repo - let fixture_root = PathBuf::from("tests/fixtures/valid_project"); - copy_dir_recursive(&fixture_root, &temp_root); - init_git_repo(&temp_root); +fn run_and_check(func: F, stage: bool, expected_staged: bool) +where + F: FnOnce(&RunConfig, bool) -> runner::RunResult, +{ + let temp_dir = setup_fixture_repo(Path::new(FIXTURE)); + let run_config = build_run_config(temp_dir.path(), CODEOWNERS_REL); - // Run generate_and_validate with staging enabled - let run_config = build_run_config(&temp_root, ".github/CODEOWNERS"); - let result = runner::generate_and_validate(&run_config, vec![], true); - assert!(result.io_errors.is_empty(), "io_errors: {:?}", result.io_errors); - assert!( - result.validation_errors.is_empty(), - "validation_errors: {:?}", - result.validation_errors - ); + let result = func(&run_config, stage); + assert_no_run_errors(&result); - // Assert CODEOWNERS file exists and is staged - let rel_path = ".github/CODEOWNERS"; assert!(run_config.codeowners_file_path.exists(), "CODEOWNERS file was not created"); - assert!(is_file_staged(&run_config.project_root, rel_path), "CODEOWNERS file was not staged"); + let staged = is_file_staged(&run_config.project_root, CODEOWNERS_REL); + assert_eq!(staged, expected_staged, "unexpected staged state for CODEOWNERS"); } From 76c126079ecc237340f3e449cdfbf572751f290c Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Mon, 11 Aug 2025 11:47:41 -0500 Subject: [PATCH 3/3] functions are used in tests --- tests/common/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 9ef7cb2..780026b 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -17,6 +17,7 @@ pub fn teardown() { }); } +#[allow(dead_code)] pub fn copy_dir_recursive(from: &Path, to: &Path) { fs::create_dir_all(to).expect("failed to create destination root"); for entry in fs::read_dir(from).expect("failed to read source dir") { @@ -35,6 +36,7 @@ pub fn copy_dir_recursive(from: &Path, to: &Path) { } } +#[allow(dead_code)] pub fn init_git_repo(path: &Path) { let status = Command::new("git") .arg("init") @@ -61,6 +63,7 @@ pub fn init_git_repo(path: &Path) { .output(); } +#[allow(dead_code)] pub fn is_file_staged(repo_root: &Path, rel_path: &str) -> bool { let output = Command::new("git") .arg("diff") @@ -78,6 +81,7 @@ pub fn is_file_staged(repo_root: &Path, rel_path: &str) -> bool { stdout.lines().any(|line| line.trim() == rel_path) } +#[allow(dead_code)] pub fn build_run_config(project_root: &Path, codeowners_rel_path: &str) -> RunConfig { let project_root = project_root.canonicalize().expect("failed to canonicalize project root"); let codeowners_file_path = project_root.join(codeowners_rel_path); @@ -90,6 +94,7 @@ pub fn build_run_config(project_root: &Path, codeowners_rel_path: &str) -> RunCo } } +#[allow(dead_code)] pub fn setup_fixture_repo(fixture_root: &Path) -> TempDir { let temp_dir = tempfile::tempdir().expect("failed to create tempdir"); copy_dir_recursive(fixture_root, temp_dir.path()); @@ -97,6 +102,7 @@ pub fn setup_fixture_repo(fixture_root: &Path) -> TempDir { temp_dir } +#[allow(dead_code)] pub fn assert_no_run_errors(result: &runner::RunResult) { assert!(result.io_errors.is_empty(), "io_errors: {:?}", result.io_errors); assert!(