From 4886abbd3c181b3ebb1a1bafa2d3db7f5a27fdff Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Wed, 3 Dec 2025 08:20:21 -0600 Subject: [PATCH 1/7] Add executable_name field to Config with default value --- src/config.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/config.rs b/src/config.rs index 893cd06..c81897d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -25,6 +25,9 @@ pub struct Config { #[serde(default = "default_ignore_dirs")] pub ignore_dirs: Vec, + + #[serde(default = "default_executable_name")] + pub executable_name: String, } #[allow(dead_code)] @@ -61,6 +64,10 @@ fn vendored_gems_path() -> String { "vendored/".to_string() } +fn default_executable_name() -> String { + "codeowners".to_string() +} + fn default_ignore_dirs() -> Vec { vec![ ".cursor".to_owned(), From 1c0fa31b3501e14b09794037483b0bb49997e770 Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Wed, 3 Dec 2025 10:14:45 -0600 Subject: [PATCH 2/7] Include executable name in validator errors and messages --- src/ownership.rs | 1 + src/ownership/validator.rs | 25 +++++++++++++++---------- src/project.rs | 1 + src/project_builder.rs | 1 + 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/ownership.rs b/src/ownership.rs index ff8cbc6..6b36193 100644 --- a/src/ownership.rs +++ b/src/ownership.rs @@ -123,6 +123,7 @@ impl Ownership { project: self.project.clone(), mappers: self.mappers(), file_generator: FileGenerator { mappers: self.mappers() }, + executable_name: self.project.executable_name.clone(), }; validator.validate() diff --git a/src/ownership/validator.rs b/src/ownership/validator.rs index dd89f66..1502916 100644 --- a/src/ownership/validator.rs +++ b/src/ownership/validator.rs @@ -21,6 +21,7 @@ pub struct Validator { pub project: Arc, pub mappers: Vec>, pub file_generator: FileGenerator, + pub executable_name: String, } #[derive(Debug)] @@ -28,7 +29,7 @@ enum Error { InvalidTeam { name: String, path: PathBuf }, FileWithoutOwner { path: PathBuf }, FileWithMultipleOwners { path: PathBuf, owners: Vec }, - CodeownershipFileIsStale, + CodeownershipFileIsStale { executable_name: String }, } #[derive(Debug)] @@ -130,12 +131,16 @@ impl Validator { match self.project.get_codeowners_file() { Ok(current_file) => { if generated_file != current_file { - vec![Error::CodeownershipFileIsStale] + vec![Error::CodeownershipFileIsStale { + executable_name: self.executable_name.to_string(), + }] } else { vec![] } } - Err(_) => vec![Error::CodeownershipFileIsStale], // Treat any read error as stale file + Err(_) => vec![Error::CodeownershipFileIsStale { + executable_name: self.executable_name.to_string(), + }], } } @@ -161,13 +166,13 @@ impl Validator { impl Error { pub fn category(&self) -> String { match self { - Error::FileWithoutOwner { path: _ } => "Some files are missing ownership".to_owned(), - Error::FileWithMultipleOwners { path: _, owners: _ } => "Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways".to_owned(), - Error::CodeownershipFileIsStale => { - "CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file".to_owned() + Error::FileWithoutOwner { path: _ } => "Some files are missing ownership".to_owned(), + Error::FileWithMultipleOwners { path: _, owners: _ } => "Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways".to_owned(), + Error::CodeownershipFileIsStale { executable_name } => { + format!("CODEOWNERS out of date. Run `{} generate` to update the CODEOWNERS file", executable_name.to_string()) + } + Error::InvalidTeam { name: _, path: _ } => "Found invalid team annotations".to_owned(), } - Error::InvalidTeam { name: _, path: _ } => "Found invalid team annotations".to_owned(), - } } pub fn messages(&self) -> Vec { @@ -187,7 +192,7 @@ impl Error { vec![messages.join("\n")] } - Error::CodeownershipFileIsStale => vec![], + Error::CodeownershipFileIsStale { executable_name: _ } => vec![], Error::InvalidTeam { name, path } => vec![format!("- {} is referencing an invalid team - '{}'", path.to_string_lossy(), name)], } } diff --git a/src/project.rs b/src/project.rs index c09b247..3670ed1 100644 --- a/src/project.rs +++ b/src/project.rs @@ -17,6 +17,7 @@ pub struct Project { pub codeowners_file_path: PathBuf, pub directory_codeowner_files: Vec, pub teams_by_name: HashMap, + pub executable_name: String, } #[derive(Clone, Debug)] diff --git a/src/project_builder.rs b/src/project_builder.rs index 1945b0a..80d1564 100644 --- a/src/project_builder.rs +++ b/src/project_builder.rs @@ -294,6 +294,7 @@ impl<'a> ProjectBuilder<'a> { codeowners_file_path: self.codeowners_file_path.to_path_buf(), directory_codeowner_files: directory_codeowners, teams_by_name, + executable_name: self.config.executable_name.clone(), }) } } From acd49c763fc5074fd88d13466f3ff13f0d454099 Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Wed, 3 Dec 2025 10:26:32 -0600 Subject: [PATCH 3/7] Add support for custom executable_name in config --- src/config.rs | 34 +++++++++++++++++++ src/ownership/file_owner_resolver.rs | 1 + src/ownership/validator.rs | 2 +- src/project.rs | 1 + tests/custom_executable_name_test.rs | 33 ++++++++++++++++++ .../custom_executable_name/.github/CODEOWNERS | 3 ++ .../config/code_ownership.yml | 6 ++++ .../config/teams/payments.yml | 6 ++++ .../ruby/app/payments/foo.rb | 4 +++ 9 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 tests/custom_executable_name_test.rs create mode 100644 tests/fixtures/custom_executable_name/.github/CODEOWNERS create mode 100644 tests/fixtures/custom_executable_name/config/code_ownership.yml create mode 100644 tests/fixtures/custom_executable_name/config/teams/payments.yml create mode 100644 tests/fixtures/custom_executable_name/ruby/app/payments/foo.rb diff --git a/src/config.rs b/src/config.rs index c81897d..6415a40 100644 --- a/src/config.rs +++ b/src/config.rs @@ -128,6 +128,40 @@ mod tests { vec!["frontend/**/node_modules/**/*", "frontend/**/__generated__/**/*"] ); assert_eq!(config.vendored_gems_path, "vendored/"); + assert_eq!(config.executable_name, "codeowners"); + Ok(()) + } + + #[test] + fn test_parse_config_with_custom_executable_name() -> Result<(), Box> { + let temp_dir = tempdir()?; + let config_path = temp_dir.path().join("config.yml"); + let config_str = indoc! {" + --- + owned_globs: + - \"**/*.rb\" + executable_name: my-custom-codeowners + "}; + fs::write(&config_path, config_str)?; + let config_file = File::open(&config_path)?; + let config: Config = serde_yaml::from_reader(config_file)?; + assert_eq!(config.executable_name, "my-custom-codeowners"); + Ok(()) + } + + #[test] + fn test_executable_name_defaults_when_not_specified() -> Result<(), Box> { + let temp_dir = tempdir()?; + let config_path = temp_dir.path().join("config.yml"); + let config_str = indoc! {" + --- + owned_globs: + - \"**/*.rb\" + "}; + fs::write(&config_path, config_str)?; + let config_file = File::open(&config_path)?; + let config: Config = serde_yaml::from_reader(config_file)?; + assert_eq!(config.executable_name, "codeowners"); Ok(()) } } diff --git a/src/ownership/file_owner_resolver.rs b/src/ownership/file_owner_resolver.rs index cac884a..737ee8e 100644 --- a/src/ownership/file_owner_resolver.rs +++ b/src/ownership/file_owner_resolver.rs @@ -293,6 +293,7 @@ mod tests { vendored_gems_path: vendored_path.to_string(), cache_directory: "tmp/cache/codeowners".to_string(), ignore_dirs: vec![], + executable_name: "codeowners".to_string(), } } diff --git a/src/ownership/validator.rs b/src/ownership/validator.rs index 1502916..627c85c 100644 --- a/src/ownership/validator.rs +++ b/src/ownership/validator.rs @@ -169,7 +169,7 @@ impl Error { Error::FileWithoutOwner { path: _ } => "Some files are missing ownership".to_owned(), Error::FileWithMultipleOwners { path: _, owners: _ } => "Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways".to_owned(), Error::CodeownershipFileIsStale { executable_name } => { - format!("CODEOWNERS out of date. Run `{} generate` to update the CODEOWNERS file", executable_name.to_string()) + format!("CODEOWNERS out of date. Run `{} generate` to update the CODEOWNERS file", executable_name) } Error::InvalidTeam { name: _, path: _ } => "Found invalid team annotations".to_owned(), } diff --git a/src/project.rs b/src/project.rs index 3670ed1..937a726 100644 --- a/src/project.rs +++ b/src/project.rs @@ -220,6 +220,7 @@ mod tests { codeowners_file_path: PathBuf::from(".github/CODEOWNERS"), directory_codeowner_files: vec![], teams_by_name: HashMap::new(), + executable_name: "codeowners".to_string(), }; let map = project.vendored_gem_by_name(); diff --git a/tests/custom_executable_name_test.rs b/tests/custom_executable_name_test.rs new file mode 100644 index 0000000..75b8c66 --- /dev/null +++ b/tests/custom_executable_name_test.rs @@ -0,0 +1,33 @@ +use predicates::prelude::*; +use std::error::Error; + +mod common; +use common::OutputStream; +use common::run_codeowners; + +#[test] +fn test_validate_uses_custom_executable_name() -> Result<(), Box> { + run_codeowners( + "custom_executable_name", + &["validate"], + false, + OutputStream::Stdout, + predicate::str::contains("CODEOWNERS out of date. Run `my-custom-tool generate` to update the CODEOWNERS file"), + )?; + Ok(()) +} + +#[test] +fn test_validate_default_executable_name() -> Result<(), Box> { + // This tests the invalid_project which doesn't specify executable_name + // and should use the default "codeowners" + run_codeowners( + "invalid_project", + &["validate"], + false, + OutputStream::Stdout, + predicate::str::contains("CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file"), + )?; + Ok(()) +} + diff --git a/tests/fixtures/custom_executable_name/.github/CODEOWNERS b/tests/fixtures/custom_executable_name/.github/CODEOWNERS new file mode 100644 index 0000000..1f80d85 --- /dev/null +++ b/tests/fixtures/custom_executable_name/.github/CODEOWNERS @@ -0,0 +1,3 @@ +# This is a stale CODEOWNERS file - it's intentionally wrong to trigger the error +ruby/app/payments/bar.rb @PaymentTeam + diff --git a/tests/fixtures/custom_executable_name/config/code_ownership.yml b/tests/fixtures/custom_executable_name/config/code_ownership.yml new file mode 100644 index 0000000..312fe56 --- /dev/null +++ b/tests/fixtures/custom_executable_name/config/code_ownership.yml @@ -0,0 +1,6 @@ +owned_globs: + - "**/*.rb" +team_file_glob: + - config/teams/**/*.yml +executable_name: my-custom-tool + diff --git a/tests/fixtures/custom_executable_name/config/teams/payments.yml b/tests/fixtures/custom_executable_name/config/teams/payments.yml new file mode 100644 index 0000000..21c9dfa --- /dev/null +++ b/tests/fixtures/custom_executable_name/config/teams/payments.yml @@ -0,0 +1,6 @@ +name: Payments +github: + team: "@PaymentTeam" +owned_globs: + - ruby/app/payments/** + diff --git a/tests/fixtures/custom_executable_name/ruby/app/payments/foo.rb b/tests/fixtures/custom_executable_name/ruby/app/payments/foo.rb new file mode 100644 index 0000000..2c5fbed --- /dev/null +++ b/tests/fixtures/custom_executable_name/ruby/app/payments/foo.rb @@ -0,0 +1,4 @@ +# A file owned by Payments +class Foo +end + From 3fac3d709ed73faf8a9edb5ecefdfc09afb2e39f Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Wed, 3 Dec 2025 10:28:14 -0600 Subject: [PATCH 4/7] Bump codeowners version to 0.3.1 --- Cargo.lock | 2 +- Cargo.toml | 2 +- tests/custom_executable_name_test.rs | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0d25561..d23cf31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -179,7 +179,7 @@ checksum = "b94f61472cee1439c0b966b47e3aca9ae07e45d070759512cd390ea2bebc6675" [[package]] name = "codeowners" -version = "0.3.0" +version = "0.3.1" dependencies = [ "assert_cmd", "clap", diff --git a/Cargo.toml b/Cargo.toml index e32dbb8..d7033f0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "codeowners" -version = "0.3.0" +version = "0.3.1" edition = "2024" [profile.release] diff --git a/tests/custom_executable_name_test.rs b/tests/custom_executable_name_test.rs index 75b8c66..cd6cb9b 100644 --- a/tests/custom_executable_name_test.rs +++ b/tests/custom_executable_name_test.rs @@ -30,4 +30,3 @@ fn test_validate_default_executable_name() -> Result<(), Box> { )?; Ok(()) } - From fa33ec1b5d7704f9047bc149e13dad65038f21f4 Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Thu, 4 Dec 2025 08:41:37 -0600 Subject: [PATCH 5/7] Add executable_path to RunConfig and use it in config loading --- README.md | 9 ++ src/cli.rs | 1 + src/crosscheck.rs | 4 +- src/runner.rs | 15 ++- src/runner/api.rs | 10 +- src/runner/types.rs | 11 ++ tests/common/mod.rs | 1 + tests/custom_executable_name_test.rs | 32 ----- tests/executable_name_config_test.rs | 67 +++++++++ .../custom_executable_name/.github/CODEOWNERS | 11 +- .../custom_executable_name/app/foo.rb | 3 + .../config/code_ownership.yml | 8 +- .../config/teams/foo.yml | 5 + .../.github/CODEOWNERS | 11 ++ .../default_executable_name/app/bar.rb | 3 + .../config/code_ownership.yml | 5 + .../config/teams/bar.yml | 5 + tests/run_config_executable_override_test.rs | 127 ++++++++++++++++++ tests/runner_api.rs | 4 + 19 files changed, 281 insertions(+), 51 deletions(-) delete mode 100644 tests/custom_executable_name_test.rs create mode 100644 tests/executable_name_config_test.rs create mode 100644 tests/fixtures/custom_executable_name/app/foo.rb create mode 100644 tests/fixtures/custom_executable_name/config/teams/foo.yml create mode 100644 tests/fixtures/default_executable_name/.github/CODEOWNERS create mode 100644 tests/fixtures/default_executable_name/app/bar.rb create mode 100644 tests/fixtures/default_executable_name/config/code_ownership.yml create mode 100644 tests/fixtures/default_executable_name/config/teams/bar.yml create mode 100644 tests/run_config_executable_override_test.rs diff --git a/README.md b/README.md index 8e0b880..8b1c910 100644 --- a/README.md +++ b/README.md @@ -213,6 +213,15 @@ codeowners gv --no-cache - `vendored_gems_path` (default: `'vendored/'`) - `cache_directory` (default: `'tmp/cache/codeowners'`) - `ignore_dirs` (default includes: `.git`, `node_modules`, `tmp`, etc.) +- `executable_name` (default: `'codeowners'`): Customize the command name shown in validation error messages. Useful when using `codeowners-rs` via wrappers like the [code_ownership](https://github.com/rubyatscale/code_ownership) Ruby gem. + +Example configuration with custom executable name: + +```yaml +owned_globs: + - '{app,components,config,frontend,lib,packs,spec}/**/*.{rb,rake,js,jsx,ts,tsx}' +executable_name: 'bin/codeownership' # For Ruby gem wrapper +``` See examples in `tests/fixtures/**/config/` for reference setups. diff --git a/src/cli.rs b/src/cli.rs index cb56ac6..a56b38b 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -114,6 +114,7 @@ pub fn cli() -> Result { codeowners_file_path, project_root, no_cache: args.no_cache, + executable_path: None, }; let runner_result = match args.command { diff --git a/src/crosscheck.rs b/src/crosscheck.rs index b8e9405..f27af94 100644 --- a/src/crosscheck.rs +++ b/src/crosscheck.rs @@ -6,7 +6,7 @@ use crate::{ ownership::file_owner_resolver::find_file_owners, project::Project, project_builder::ProjectBuilder, - runner::{RunConfig, RunResult, config_from_path, team_for_file_from_codeowners}, + runner::{RunConfig, RunResult, config_from_run_config, team_for_file_from_codeowners}, }; pub fn crosscheck_owners(run_config: &RunConfig, cache: &Cache) -> RunResult { @@ -43,7 +43,7 @@ fn do_crosscheck_owners(run_config: &RunConfig, cache: &Cache) -> Result Result { - config_from_path(&run_config.config_path).map_err(|e| e.to_string()) + config_from_run_config(run_config).map_err(|e| e.to_string()) } fn build_project(config: &Config, run_config: &RunConfig, cache: &Cache) -> Result { diff --git a/src/runner.rs b/src/runner.rs index 4d64750..6fe099f 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -1,4 +1,4 @@ -use std::{path::Path, process::Command}; +use std::process::Command; use error_stack::{Result, ResultExt}; use serde::Serialize; @@ -44,15 +44,20 @@ where runnable(runner) } -pub(crate) fn config_from_path(path: &Path) -> Result { - match crate::config::Config::load_from_path(path) { - Ok(c) => Ok(c), +pub(crate) fn config_from_run_config(run_config: &RunConfig) -> Result { + match crate::config::Config::load_from_path(&run_config.config_path) { + Ok(mut c) => { + if let Some(executable_name) = &run_config.executable_name() { + c.executable_name = executable_name.clone(); + } + Ok(c) + } Err(msg) => Err(error_stack::Report::new(Error::Io(msg))), } } impl Runner { pub fn new(run_config: &RunConfig) -> Result { - let config = config_from_path(&run_config.config_path)?; + let config = config_from_run_config(run_config)?; let cache: Cache = if run_config.no_cache { NoopCache::default().into() diff --git a/src/runner/api.rs b/src/runner/api.rs index 9a88512..33e3cc6 100644 --- a/src/runner/api.rs +++ b/src/runner/api.rs @@ -1,9 +1,9 @@ use std::collections::HashMap; -use crate::ownership::FileOwner; use crate::project::Team; +use crate::{ownership::FileOwner, runner::config_from_run_config}; -use super::{Error, ForFileResult, RunConfig, RunResult, config_from_path, run}; +use super::{Error, ForFileResult, RunConfig, RunResult, run}; pub fn for_file(run_config: &RunConfig, file_path: &str, from_codeowners: bool, json: bool) -> RunResult { if from_codeowners { @@ -38,7 +38,7 @@ pub fn crosscheck_owners(run_config: &RunConfig) -> RunResult { // Returns all owners for a file without creating a Runner (performance optimized) pub fn owners_for_file(run_config: &RunConfig, file_path: &str) -> error_stack::Result, Error> { - let config = config_from_path(&run_config.config_path)?; + let config = config_from_run_config(run_config)?; use crate::ownership::file_owner_resolver::find_file_owners; let owners = find_file_owners(&run_config.project_root, &config, std::path::Path::new(file_path)).map_err(Error::Io)?; Ok(owners) @@ -60,7 +60,7 @@ pub fn teams_for_files_from_codeowners( run_config: &RunConfig, file_paths: &[String], ) -> error_stack::Result>, Error> { - let config = config_from_path(&run_config.config_path)?; + let config = config_from_run_config(run_config)?; let res = crate::ownership::codeowners_query::teams_for_files_from_codeowners( &run_config.project_root, &run_config.codeowners_file_path, @@ -80,7 +80,7 @@ pub fn team_for_file_from_codeowners(run_config: &RunConfig, file_path: &str) -> // Fast path that avoids creating a full Runner for single file queries fn for_file_optimized(run_config: &RunConfig, file_path: &str, json: bool) -> RunResult { - let config = match config_from_path(&run_config.config_path) { + let config = match config_from_run_config(run_config) { Ok(c) => c, Err(err) => { return RunResult::from_io_error(Error::Io(err.to_string()), json); diff --git a/src/runner/types.rs b/src/runner/types.rs index c42fa02..124bd53 100644 --- a/src/runner/types.rs +++ b/src/runner/types.rs @@ -4,6 +4,8 @@ use std::path::PathBuf; use error_stack::Context; use serde::{Deserialize, Serialize}; +use crate::path_utils::relative_to_buf; + #[derive(Debug, Default, Serialize, Deserialize)] pub struct RunResult { pub validation_errors: Vec, @@ -17,6 +19,15 @@ pub struct RunConfig { pub codeowners_file_path: PathBuf, pub config_path: PathBuf, pub no_cache: bool, + pub executable_path: Option, +} + +impl RunConfig { + pub fn executable_name(&self) -> Option { + self.executable_path + .as_ref() + .map(|path| relative_to_buf(&self.project_root, path).to_string_lossy().to_string()) + } } #[derive(Debug, Serialize)] diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 226e528..d0815c3 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -157,6 +157,7 @@ pub fn build_run_config(project_root: &Path, codeowners_rel_path: &str) -> RunCo codeowners_file_path, config_path, no_cache: true, + executable_path: None, } } diff --git a/tests/custom_executable_name_test.rs b/tests/custom_executable_name_test.rs deleted file mode 100644 index cd6cb9b..0000000 --- a/tests/custom_executable_name_test.rs +++ /dev/null @@ -1,32 +0,0 @@ -use predicates::prelude::*; -use std::error::Error; - -mod common; -use common::OutputStream; -use common::run_codeowners; - -#[test] -fn test_validate_uses_custom_executable_name() -> Result<(), Box> { - run_codeowners( - "custom_executable_name", - &["validate"], - false, - OutputStream::Stdout, - predicate::str::contains("CODEOWNERS out of date. Run `my-custom-tool generate` to update the CODEOWNERS file"), - )?; - Ok(()) -} - -#[test] -fn test_validate_default_executable_name() -> Result<(), Box> { - // This tests the invalid_project which doesn't specify executable_name - // and should use the default "codeowners" - run_codeowners( - "invalid_project", - &["validate"], - false, - OutputStream::Stdout, - predicate::str::contains("CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file"), - )?; - Ok(()) -} diff --git a/tests/executable_name_config_test.rs b/tests/executable_name_config_test.rs new file mode 100644 index 0000000..0b0eb6e --- /dev/null +++ b/tests/executable_name_config_test.rs @@ -0,0 +1,67 @@ +use indoc::indoc; +use predicates::prelude::*; +use std::error::Error; + +mod common; +use common::OutputStream; +use common::run_codeowners; + +#[test] +fn test_validate_with_custom_executable_name() -> Result<(), Box> { + // When executable_name is configured, error should show that command + run_codeowners( + "custom_executable_name", + &["validate"], + false, + OutputStream::Stdout, + predicate::str::contains("Run `bin/codeownership generate`"), + )?; + Ok(()) +} + +#[test] +fn test_validate_with_default_executable_name() -> Result<(), Box> { + // When executable_name is not configured, error should show default "codeowners" + run_codeowners( + "default_executable_name", + &["validate"], + false, + OutputStream::Stdout, + predicate::str::contains("Run `codeowners generate`"), + )?; + Ok(()) +} + +#[test] +fn test_custom_executable_name_full_error_message() -> Result<(), Box> { + // Verify the complete error message format with custom executable + run_codeowners( + "custom_executable_name", + &["validate"], + false, + OutputStream::Stdout, + predicate::eq(indoc! {" + + CODEOWNERS out of date. Run `bin/codeownership generate` to update the CODEOWNERS file + + "}), + )?; + Ok(()) +} + +#[test] +fn test_default_executable_name_full_error_message() -> Result<(), Box> { + // Verify the complete error message format with default executable + run_codeowners( + "default_executable_name", + &["validate"], + false, + OutputStream::Stdout, + predicate::eq(indoc! {" + + CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file + + "}), + )?; + Ok(()) +} diff --git a/tests/fixtures/custom_executable_name/.github/CODEOWNERS b/tests/fixtures/custom_executable_name/.github/CODEOWNERS index 1f80d85..0b24c66 100644 --- a/tests/fixtures/custom_executable_name/.github/CODEOWNERS +++ b/tests/fixtures/custom_executable_name/.github/CODEOWNERS @@ -1,3 +1,10 @@ -# This is a stale CODEOWNERS file - it's intentionally wrong to trigger the error -ruby/app/payments/bar.rb @PaymentTeam +# STOP! - DO NOT EDIT THIS FILE MANUALLY +# This file was automatically generated by "codeowners". +# +# CODEOWNERS is used for GitHub to suggest code/file owners to various GitHub +# teams. This is useful when developers create Pull Requests since the +# code/file owner is notified. Reference GitHub docs for more details: +# https://help.github.com/en/articles/about-code-owners +# Outdated content to trigger validation error +/app/old.rb @FooTeam diff --git a/tests/fixtures/custom_executable_name/app/foo.rb b/tests/fixtures/custom_executable_name/app/foo.rb new file mode 100644 index 0000000..4804b19 --- /dev/null +++ b/tests/fixtures/custom_executable_name/app/foo.rb @@ -0,0 +1,3 @@ +# @team Foo +puts 'foo' + diff --git a/tests/fixtures/custom_executable_name/config/code_ownership.yml b/tests/fixtures/custom_executable_name/config/code_ownership.yml index 312fe56..fa25a7d 100644 --- a/tests/fixtures/custom_executable_name/config/code_ownership.yml +++ b/tests/fixtures/custom_executable_name/config/code_ownership.yml @@ -1,6 +1,4 @@ +--- owned_globs: - - "**/*.rb" -team_file_glob: - - config/teams/**/*.yml -executable_name: my-custom-tool - + - "{app,config}/**/*.rb" +executable_name: "bin/codeownership" diff --git a/tests/fixtures/custom_executable_name/config/teams/foo.yml b/tests/fixtures/custom_executable_name/config/teams/foo.yml new file mode 100644 index 0000000..f515b0c --- /dev/null +++ b/tests/fixtures/custom_executable_name/config/teams/foo.yml @@ -0,0 +1,5 @@ +--- +name: Foo +github: + team: '@FooTeam' + diff --git a/tests/fixtures/default_executable_name/.github/CODEOWNERS b/tests/fixtures/default_executable_name/.github/CODEOWNERS new file mode 100644 index 0000000..47f3d2b --- /dev/null +++ b/tests/fixtures/default_executable_name/.github/CODEOWNERS @@ -0,0 +1,11 @@ +# STOP! - DO NOT EDIT THIS FILE MANUALLY +# This file was automatically generated by "codeowners". +# +# CODEOWNERS is used for GitHub to suggest code/file owners to various GitHub +# teams. This is useful when developers create Pull Requests since the +# code/file owner is notified. Reference GitHub docs for more details: +# https://help.github.com/en/articles/about-code-owners + +# Outdated content to trigger validation error +/app/old.rb @BarTeam + diff --git a/tests/fixtures/default_executable_name/app/bar.rb b/tests/fixtures/default_executable_name/app/bar.rb new file mode 100644 index 0000000..7be278a --- /dev/null +++ b/tests/fixtures/default_executable_name/app/bar.rb @@ -0,0 +1,3 @@ +# @team Bar +puts 'bar' + diff --git a/tests/fixtures/default_executable_name/config/code_ownership.yml b/tests/fixtures/default_executable_name/config/code_ownership.yml new file mode 100644 index 0000000..3c38444 --- /dev/null +++ b/tests/fixtures/default_executable_name/config/code_ownership.yml @@ -0,0 +1,5 @@ +--- +owned_globs: + - "{app,config}/**/*.rb" +# No executable_name specified - should use default + diff --git a/tests/fixtures/default_executable_name/config/teams/bar.yml b/tests/fixtures/default_executable_name/config/teams/bar.yml new file mode 100644 index 0000000..3e804f0 --- /dev/null +++ b/tests/fixtures/default_executable_name/config/teams/bar.yml @@ -0,0 +1,5 @@ +--- +name: Bar +github: + team: '@BarTeam' + diff --git a/tests/run_config_executable_override_test.rs b/tests/run_config_executable_override_test.rs new file mode 100644 index 0000000..78aae38 --- /dev/null +++ b/tests/run_config_executable_override_test.rs @@ -0,0 +1,127 @@ +use std::error::Error; +use std::path::{Path, PathBuf}; + +mod common; +use common::{build_run_config, git_add_all_files, setup_fixture_repo}; + +#[test] +fn test_run_config_executable_path_overrides_config_file() -> Result<(), Box> { + use codeowners::runner::validate; + + let fixture_root = Path::new("tests/fixtures/custom_executable_name"); + let temp_dir = setup_fixture_repo(fixture_root); + let project_path = temp_dir.path(); + git_add_all_files(project_path); + + // This fixture has executable_name: "bin/codeownership" in config + // But we'll override it with RunConfig.executable_path + + let mut run_config = build_run_config(project_path, ".github/CODEOWNERS"); + // Use a relative path that gets displayed as-is in error messages + run_config.executable_path = Some(PathBuf::from("my-wrapper-tool")); + + let result = validate(&run_config, vec![]); + + // Should use "my-wrapper-tool" from executable_path, NOT "bin/codeownership" from config + assert!(!result.validation_errors.is_empty(), "Expected validation errors but got none"); + let error_msg = result.validation_errors.join("\n"); + assert!( + error_msg.contains("Run `my-wrapper-tool generate`"), + "Expected error to contain 'my-wrapper-tool generate' but got: {}", + error_msg + ); + assert!( + !error_msg.contains("bin/codeownership"), + "Error should not contain config file's executable_name when overridden" + ); + + Ok(()) +} + +#[test] +fn test_run_config_without_executable_path_uses_config_file() -> Result<(), Box> { + use codeowners::runner::validate; + + let fixture_root = Path::new("tests/fixtures/custom_executable_name"); + let temp_dir = setup_fixture_repo(fixture_root); + let project_path = temp_dir.path(); + git_add_all_files(project_path); + + // This fixture has executable_name: "bin/codeownership" in config + + let mut run_config = build_run_config(project_path, ".github/CODEOWNERS"); + run_config.executable_path = None; // Explicitly no override + + let result = validate(&run_config, vec![]); + + // Should use "bin/codeownership" from config file + assert!(!result.validation_errors.is_empty(), "Expected validation errors but got none"); + let error_msg = result.validation_errors.join("\n"); + assert!( + error_msg.contains("Run `bin/codeownership generate`"), + "Expected error to contain 'bin/codeownership generate' but got: {}", + error_msg + ); + + Ok(()) +} + +#[test] +fn test_run_config_executable_path_overrides_default() -> Result<(), Box> { + use codeowners::runner::validate; + + let fixture_root = Path::new("tests/fixtures/default_executable_name"); + let temp_dir = setup_fixture_repo(fixture_root); + let project_path = temp_dir.path(); + git_add_all_files(project_path); + + // This fixture has NO executable_name in config (uses default "codeowners") + + let mut run_config = build_run_config(project_path, ".github/CODEOWNERS"); + run_config.executable_path = Some(PathBuf::from("custom-command")); + + let result = validate(&run_config, vec![]); + + // Should use "custom-command" from executable_path, NOT default "codeowners" + assert!(!result.validation_errors.is_empty(), "Expected validation errors but got none"); + let error_msg = result.validation_errors.join("\n"); + assert!( + error_msg.contains("Run `custom-command generate`"), + "Expected error to contain 'custom-command generate' but got: {}", + error_msg + ); + assert!( + !error_msg.contains("codeowners generate"), + "Error should not contain default when overridden" + ); + + Ok(()) +} + +#[test] +fn test_executable_name_extraction_from_path() { + use codeowners::runner::RunConfig; + + let mut run_config = RunConfig { + project_root: PathBuf::from("."), + codeowners_file_path: PathBuf::from(".github/CODEOWNERS"), + config_path: PathBuf::from("config/code_ownership.yml"), + no_cache: true, + executable_path: None, + }; + + // Test with None + assert_eq!(run_config.executable_name(), None); + + // Test with simple path (no directory component) + run_config.executable_path = Some(PathBuf::from("codeowners")); + assert_eq!(run_config.executable_name(), Some("codeowners".to_string())); + + // Test with relative path - returns full relative path string + run_config.executable_path = Some(PathBuf::from("bin/codeownership")); + assert_eq!(run_config.executable_name(), Some("bin/codeownership".to_string())); + + // Test with another relative path + run_config.executable_path = Some(PathBuf::from("tools/my-tool")); + assert_eq!(run_config.executable_name(), Some("tools/my-tool".to_string())); +} diff --git a/tests/runner_api.rs b/tests/runner_api.rs index 1d1f9f6..f8bd81d 100644 --- a/tests/runner_api.rs +++ b/tests/runner_api.rs @@ -38,6 +38,7 @@ team_file_glob: codeowners_file_path: temp_dir.path().join(".github/CODEOWNERS").to_path_buf(), config_path: temp_dir.path().join("config/code_ownership.yml").to_path_buf(), no_cache: true, + executable_path: None, }; let file_owner = runner::file_owner_for_file(&run_config, "app/consumers/deep/nesting/nestdir/deep_file.rb") @@ -63,6 +64,7 @@ fn test_teams_for_files_from_codeowners() { codeowners_file_path: project_root.join(".github/CODEOWNERS").to_path_buf(), config_path: project_root.join("config/code_ownership.yml").to_path_buf(), no_cache: true, + executable_path: None, }; let teams = runner::teams_for_files_from_codeowners(&run_config, &file_paths.iter().map(|s| s.to_string()).collect::>()).unwrap(); @@ -129,6 +131,7 @@ javascript_package_paths: codeowners_file_path: td.path().join(".github/CODEOWNERS"), config_path: td.path().join("config/code_ownership.yml"), no_cache: true, + executable_path: None, }; // Ensure CODEOWNERS file matches generator output to avoid out-of-date errors @@ -170,6 +173,7 @@ javascript_package_paths: codeowners_file_path: td.path().join(".github/CODEOWNERS"), config_path: td.path().join("config/code_ownership.yml"), no_cache: true, + executable_path: None, }; let gv = runner::generate_and_validate(&rc, vec![], true); From 55d8dbd02968717084ad61d3d8cbe869633e3ef8 Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Thu, 4 Dec 2025 13:47:20 -0600 Subject: [PATCH 6/7] Simplify executable_path handling by using String directly --- src/runner.rs | 2 +- src/runner/types.rs | 12 +------ tests/run_config_executable_override_test.rs | 34 ++------------------ 3 files changed, 5 insertions(+), 43 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index 6fe099f..fb2b752 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -47,7 +47,7 @@ where pub(crate) fn config_from_run_config(run_config: &RunConfig) -> Result { match crate::config::Config::load_from_path(&run_config.config_path) { Ok(mut c) => { - if let Some(executable_name) = &run_config.executable_name() { + if let Some(executable_name) = &run_config.executable_path { c.executable_name = executable_name.clone(); } Ok(c) diff --git a/src/runner/types.rs b/src/runner/types.rs index 124bd53..8fb2609 100644 --- a/src/runner/types.rs +++ b/src/runner/types.rs @@ -4,8 +4,6 @@ use std::path::PathBuf; use error_stack::Context; use serde::{Deserialize, Serialize}; -use crate::path_utils::relative_to_buf; - #[derive(Debug, Default, Serialize, Deserialize)] pub struct RunResult { pub validation_errors: Vec, @@ -19,15 +17,7 @@ pub struct RunConfig { pub codeowners_file_path: PathBuf, pub config_path: PathBuf, pub no_cache: bool, - pub executable_path: Option, -} - -impl RunConfig { - pub fn executable_name(&self) -> Option { - self.executable_path - .as_ref() - .map(|path| relative_to_buf(&self.project_root, path).to_string_lossy().to_string()) - } + pub executable_path: Option, } #[derive(Debug, Serialize)] diff --git a/tests/run_config_executable_override_test.rs b/tests/run_config_executable_override_test.rs index 78aae38..0d1fce5 100644 --- a/tests/run_config_executable_override_test.rs +++ b/tests/run_config_executable_override_test.rs @@ -1,5 +1,5 @@ use std::error::Error; -use std::path::{Path, PathBuf}; +use std::path::Path; mod common; use common::{build_run_config, git_add_all_files, setup_fixture_repo}; @@ -18,7 +18,7 @@ fn test_run_config_executable_path_overrides_config_file() -> Result<(), Box Result<(), Box Result<(), Box Date: Thu, 4 Dec 2025 13:49:16 -0600 Subject: [PATCH 7/7] Rename executable_path to executable_name throughout code and tests --- src/cli.rs | 2 +- src/runner.rs | 2 +- src/runner/types.rs | 2 +- tests/common/mod.rs | 2 +- tests/run_config_executable_override_test.rs | 6 +++--- tests/runner_api.rs | 8 ++++---- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index a56b38b..c9cef15 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -114,7 +114,7 @@ pub fn cli() -> Result { codeowners_file_path, project_root, no_cache: args.no_cache, - executable_path: None, + executable_name: None, }; let runner_result = match args.command { diff --git a/src/runner.rs b/src/runner.rs index fb2b752..50756f3 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -47,7 +47,7 @@ where pub(crate) fn config_from_run_config(run_config: &RunConfig) -> Result { match crate::config::Config::load_from_path(&run_config.config_path) { Ok(mut c) => { - if let Some(executable_name) = &run_config.executable_path { + if let Some(executable_name) = &run_config.executable_name { c.executable_name = executable_name.clone(); } Ok(c) diff --git a/src/runner/types.rs b/src/runner/types.rs index 8fb2609..9ecffe8 100644 --- a/src/runner/types.rs +++ b/src/runner/types.rs @@ -17,7 +17,7 @@ pub struct RunConfig { pub codeowners_file_path: PathBuf, pub config_path: PathBuf, pub no_cache: bool, - pub executable_path: Option, + pub executable_name: Option, } #[derive(Debug, Serialize)] diff --git a/tests/common/mod.rs b/tests/common/mod.rs index d0815c3..c419b49 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -157,7 +157,7 @@ pub fn build_run_config(project_root: &Path, codeowners_rel_path: &str) -> RunCo codeowners_file_path, config_path, no_cache: true, - executable_path: None, + executable_name: None, } } diff --git a/tests/run_config_executable_override_test.rs b/tests/run_config_executable_override_test.rs index 0d1fce5..35e1c4b 100644 --- a/tests/run_config_executable_override_test.rs +++ b/tests/run_config_executable_override_test.rs @@ -18,7 +18,7 @@ fn test_run_config_executable_path_overrides_config_file() -> Result<(), Box Result<(), Box< // This fixture has executable_name: "bin/codeownership" in config let mut run_config = build_run_config(project_path, ".github/CODEOWNERS"); - run_config.executable_path = None; // Explicitly no override + run_config.executable_name = None; // Explicitly no override let result = validate(&run_config, vec![]); @@ -78,7 +78,7 @@ fn test_run_config_executable_path_overrides_default() -> Result<(), Box>()).unwrap(); @@ -131,7 +131,7 @@ javascript_package_paths: codeowners_file_path: td.path().join(".github/CODEOWNERS"), config_path: td.path().join("config/code_ownership.yml"), no_cache: true, - executable_path: None, + executable_name: None, }; // Ensure CODEOWNERS file matches generator output to avoid out-of-date errors @@ -173,7 +173,7 @@ javascript_package_paths: codeowners_file_path: td.path().join(".github/CODEOWNERS"), config_path: td.path().join("config/code_ownership.yml"), no_cache: true, - executable_path: None, + executable_name: None, }; let gv = runner::generate_and_validate(&rc, vec![], true);