diff --git a/README.md b/README.md index 8b1c910..ebbafcf 100644 --- a/README.md +++ b/README.md @@ -253,9 +253,10 @@ use codeowners::runner::{RunConfig, for_file, teams_for_files_from_codeowners}; fn main() { let run_config = RunConfig { project_root: std::path::PathBuf::from("."), - codeowners_file_path: std::path::PathBuf::from(".github/CODEOWNERS"), + codeowners_file_path: Some(std::path::PathBuf::from(".github/CODEOWNERS")), // optional, if None provided, will be resolved from config/env config_path: std::path::PathBuf::from("config/code_ownership.yml"), no_cache: true, // set false to enable on-disk caching + executable_name: None, }; // Find owner for a single file using the optimized path (not just CODEOWNERS) diff --git a/src/cli.rs b/src/cli.rs index c9cef15..0721a51 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -65,9 +65,9 @@ struct Args { #[command(subcommand)] command: Command, - /// Path for the CODEOWNERS file. - #[arg(long, default_value = "./.github/CODEOWNERS")] - codeowners_file_path: PathBuf, + /// Path for the CODEOWNERS file (overrides codeowners_path from the config) [default: .github/CODEOWNERS] + #[arg(long)] + codeowners_file_path: Option, /// Path for the configuration file #[arg(long, default_value = "./config/code_ownership.yml")] config_path: PathBuf, @@ -93,8 +93,11 @@ impl Args { Ok(self.absolute_path(&self.config_path)?.clean()) } - fn absolute_codeowners_path(&self) -> Result { - Ok(self.absolute_path(&self.codeowners_file_path)?.clean()) + fn absolute_codeowners_path(&self) -> Result, RunnerError> { + match &self.codeowners_file_path { + Some(path) => Ok(Some(self.absolute_path(path)?.clean())), + None => Ok(None), + } } fn absolute_path(&self, path: &Path) -> Result { diff --git a/src/config.rs b/src/config.rs index e8a7820..5282aa3 100644 --- a/src/config.rs +++ b/src/config.rs @@ -28,6 +28,9 @@ pub struct Config { #[serde(default = "default_executable_name")] pub executable_name: String, + + #[serde(default = "default_codeowners_path")] + pub codeowners_path: String, } #[allow(dead_code)] @@ -84,6 +87,10 @@ fn default_ignore_dirs() -> Vec { ] } +fn default_codeowners_path() -> String { + ".github".to_string() +} + impl Config { pub fn load_from_path(path: &Path) -> std::result::Result { let file = File::open(path).map_err(|e| format!("Can't open config file: {} ({})", path.to_string_lossy(), e))?; @@ -163,4 +170,37 @@ mod tests { assert_eq!(config.executable_name, "codeowners"); Ok(()) } + + #[test] + fn test_codeowners_path_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.codeowners_path, ".github"); + Ok(()) + } + + #[test] + fn test_codeowners_path_can_be_customized() -> Result<(), Box> { + let temp_dir = tempdir()?; + let config_path = temp_dir.path().join("config.yml"); + let config_str = indoc! {" + --- + owned_globs: + - \"**/*.rb\" + codeowners_path: docs + "}; + 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.codeowners_path, "docs"); + Ok(()) + } } diff --git a/src/crosscheck.rs b/src/crosscheck.rs index f27af94..015071b 100644 --- a/src/crosscheck.rs +++ b/src/crosscheck.rs @@ -47,12 +47,9 @@ fn load_config(run_config: &RunConfig) -> Result { } fn build_project(config: &Config, run_config: &RunConfig, cache: &Cache) -> Result { - let mut project_builder = ProjectBuilder::new( - config, - run_config.project_root.clone(), - run_config.codeowners_file_path.clone(), - cache, - ); + use crate::runner::resolve_codeowners_file_path; + let codeowners_file_path = resolve_codeowners_file_path(run_config, config); + let mut project_builder = ProjectBuilder::new(config, run_config.project_root.clone(), codeowners_file_path, cache); project_builder.build().map_err(|e| e.to_string()) } diff --git a/src/ownership/file_owner_resolver.rs b/src/ownership/file_owner_resolver.rs index 737ee8e..8b15e38 100644 --- a/src/ownership/file_owner_resolver.rs +++ b/src/ownership/file_owner_resolver.rs @@ -294,6 +294,7 @@ mod tests { cache_directory: "tmp/cache/codeowners".to_string(), ignore_dirs: vec![], executable_name: "codeowners".to_string(), + codeowners_path: ".github".to_string(), } } diff --git a/src/ownership/mapper/package_mapper.rs b/src/ownership/mapper/package_mapper.rs index 351dfd6..2472e3f 100644 --- a/src/ownership/mapper/package_mapper.rs +++ b/src/ownership/mapper/package_mapper.rs @@ -147,7 +147,7 @@ mod tests { use std::{error::Error, path::Path}; #[test] fn test_remove_nested_packages() { - let packages = vec![ + let packages = [ Package { path: Path::new("packs/a/package.yml").to_owned(), package_type: PackageType::Ruby, diff --git a/src/runner.rs b/src/runner.rs index 8ac7d76..90ee772 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -1,3 +1,4 @@ +use std::path::PathBuf; use std::process::Command; use error_stack::{Result, ResultExt}; @@ -20,6 +21,7 @@ pub struct Runner { ownership: Ownership, cache: Cache, config: Config, + codeowners_file_path: PathBuf, } pub fn version() -> String { @@ -55,9 +57,30 @@ pub(crate) fn config_from_run_config(run_config: &RunConfig) -> Result Err(error_stack::Report::new(Error::Io(msg))), } } + +/// Resolves the CODEOWNERS file path with the following priority: +/// 1. Explicit `codeowners_file_path` in `RunConfig` (if provided from e.g. CLI flag) +/// 2. `CODEOWNERS_PATH` environment variable (if set and not empty) +/// 3. Computed from `codeowners_path` directory path in config + "CODEOWNERS" filename +/// 4. Default fallback to `.github/CODEOWNERS` (using default codeowners_path from config) +pub(crate) fn resolve_codeowners_file_path(run_config: &RunConfig, config: &Config) -> PathBuf { + if let Some(ref path) = run_config.codeowners_file_path { + return path.clone(); + } + + if let Ok(env_path) = std::env::var("CODEOWNERS_PATH") + && !env_path.is_empty() + { + return run_config.project_root.join(env_path); + } + + run_config.project_root.join(&config.codeowners_path).join("CODEOWNERS") +} + impl Runner { pub fn new(run_config: &RunConfig) -> Result { let config = config_from_run_config(run_config)?; + let codeowners_file_path = resolve_codeowners_file_path(run_config, &config); let cache: Cache = if run_config.no_cache { NoopCache::default().into() @@ -71,12 +94,7 @@ impl Runner { .into() }; - let mut project_builder = ProjectBuilder::new( - &config, - run_config.project_root.clone(), - run_config.codeowners_file_path.clone(), - &cache, - ); + let mut project_builder = ProjectBuilder::new(&config, run_config.project_root.clone(), codeowners_file_path.clone(), &cache); let project = project_builder.build().change_context(Error::Io(format!( "Can't build project: {}", &run_config.config_path.to_string_lossy() @@ -93,6 +111,7 @@ impl Runner { ownership, cache, config, + codeowners_file_path, }) } @@ -150,10 +169,10 @@ impl Runner { 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() { + if let Some(parent) = &self.codeowners_file_path.parent() { let _ = std::fs::create_dir_all(parent); } - match std::fs::write(&self.run_config.codeowners_file_path, content) { + match std::fs::write(&self.codeowners_file_path, content) { Ok(_) => { if git_stage { self.git_stage(); @@ -178,7 +197,7 @@ impl Runner { fn git_stage(&self) { let _ = Command::new("git") .arg("add") - .arg(&self.run_config.codeowners_file_path) + .arg(&self.codeowners_file_path) .current_dir(&self.run_config.project_root) .output(); } diff --git a/src/runner/api.rs b/src/runner/api.rs index 33e3cc6..31edbc4 100644 --- a/src/runner/api.rs +++ b/src/runner/api.rs @@ -61,9 +61,10 @@ pub fn teams_for_files_from_codeowners( file_paths: &[String], ) -> error_stack::Result>, Error> { let config = config_from_run_config(run_config)?; + let codeowners_file_path = super::resolve_codeowners_file_path(run_config, &config); let res = crate::ownership::codeowners_query::teams_for_files_from_codeowners( &run_config.project_root, - &run_config.codeowners_file_path, + &codeowners_file_path, &config.team_file_glob, file_paths, ) diff --git a/src/runner/types.rs b/src/runner/types.rs index 9ecffe8..f5e00fd 100644 --- a/src/runner/types.rs +++ b/src/runner/types.rs @@ -14,7 +14,7 @@ pub struct RunResult { #[derive(Debug, Clone)] pub struct RunConfig { pub project_root: PathBuf, - pub codeowners_file_path: PathBuf, + pub codeowners_file_path: Option, pub config_path: PathBuf, pub no_cache: bool, pub executable_name: Option, diff --git a/tests/codeowners_path_test.rs b/tests/codeowners_path_test.rs new file mode 100644 index 0000000..add02a0 --- /dev/null +++ b/tests/codeowners_path_test.rs @@ -0,0 +1,92 @@ +use predicates::prelude::predicate; +use std::{error::Error, fs, path::Path}; + +mod common; + +use common::OutputStream; +use common::git_add_all_files; +use common::run_codeowners; +use common::setup_fixture_repo; + +#[test] +fn test_generate_uses_codeowners_path_from_config() -> Result<(), Box> { + let fixture_root = Path::new("tests/fixtures/custom_codeowners_path"); + let temp_dir = setup_fixture_repo(fixture_root); + let project_root = temp_dir.path(); + git_add_all_files(project_root); + + let mut cmd = assert_cmd::Command::cargo_bin("codeowners")?; + cmd.arg("--project-root") + .arg(project_root) + .arg("--no-cache") + .arg("generate") + .assert() + .success(); + + let expected_codeowners: String = std::fs::read_to_string(Path::new("tests/fixtures/custom_codeowners_path/expected/CODEOWNERS"))?; + let actual_codeowners: String = std::fs::read_to_string(project_root.join("docs/CODEOWNERS"))?; + + assert_eq!(expected_codeowners, actual_codeowners); + + Ok(()) +} + +#[test] +fn test_cli_overrides_codeowners_path_from_config() -> Result<(), Box> { + fs::create_dir_all("tmp")?; + let codeowners_abs = std::env::current_dir()?.join("tmp/CODEOWNERS"); + let codeowners_str = codeowners_abs.to_str().unwrap(); + + run_codeowners( + "custom_codeowners_path", + &["--codeowners-file-path", codeowners_str, "generate"], + true, + OutputStream::Stdout, + predicate::eq(""), + )?; + + let expected_codeowners: String = std::fs::read_to_string(Path::new("tests/fixtures/custom_codeowners_path/expected/CODEOWNERS"))?; + let actual_codeowners: String = std::fs::read_to_string(Path::new("tmp/CODEOWNERS"))?; + + assert_eq!(expected_codeowners, actual_codeowners); + + Ok(()) +} + +#[test] +fn test_validate_uses_codeowners_path_from_config() -> Result<(), Box> { + run_codeowners( + "custom_codeowners_path", + &["validate"], + true, + OutputStream::Stdout, + predicate::eq(""), + )?; + + Ok(()) +} + +#[test] +fn test_validate_uses_cli_override() -> Result<(), Box> { + fs::create_dir_all("tmp")?; + let codeowners_abs = std::env::current_dir()?.join("tmp/CODEOWNERS"); + let codeowners_str = codeowners_abs.to_str().unwrap(); + + run_codeowners( + "custom_codeowners_path", + &["--codeowners-file-path", codeowners_str, "generate"], + true, + OutputStream::Stdout, + predicate::eq(""), + )?; + + run_codeowners( + "custom_codeowners_path", + &["--codeowners-file-path", codeowners_str, "validate"], + true, + OutputStream::Stdout, + predicate::eq(""), + )?; + + Ok(()) +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs index c419b49..0500841 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -154,7 +154,7 @@ pub fn build_run_config(project_root: &Path, codeowners_rel_path: &str) -> RunCo let config_path = project_root.join("config/code_ownership.yml"); RunConfig { project_root, - codeowners_file_path, + codeowners_file_path: Some(codeowners_file_path), config_path, no_cache: true, executable_name: None, diff --git a/tests/fixtures/custom_codeowners_path/config/code_ownership.yml b/tests/fixtures/custom_codeowners_path/config/code_ownership.yml new file mode 100644 index 0000000..ce76757 --- /dev/null +++ b/tests/fixtures/custom_codeowners_path/config/code_ownership.yml @@ -0,0 +1,11 @@ +owned_globs: + - "{config,ruby}/**/*.{rb,yml}" +ruby_package_paths: [] +javascript_package_paths: [] +team_file_glob: + - config/teams/**/*.yml +unbuilt_gems_path: gems +unowned_globs: + - config/code_ownership.yml +codeowners_path: docs + diff --git a/tests/fixtures/custom_codeowners_path/config/teams/test_team.yml b/tests/fixtures/custom_codeowners_path/config/teams/test_team.yml new file mode 100644 index 0000000..451a0ea --- /dev/null +++ b/tests/fixtures/custom_codeowners_path/config/teams/test_team.yml @@ -0,0 +1,6 @@ +name: TestTeam +github: + team: "@TestTeam" + members: + - testmember + diff --git a/tests/fixtures/custom_codeowners_path/docs/CODEOWNERS b/tests/fixtures/custom_codeowners_path/docs/CODEOWNERS new file mode 100644 index 0000000..0a66c8b --- /dev/null +++ b/tests/fixtures/custom_codeowners_path/docs/CODEOWNERS @@ -0,0 +1,14 @@ +# STOP! - DO NOT EDIT THIS FILE MANUALLY +# This file was automatically generated by "bin/codeownership validate". +# +# 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 + + +# Annotations at the top of file +/ruby/app/models/test.rb @TestTeam + +# Team YML ownership +/config/teams/test_team.yml @TestTeam diff --git a/tests/fixtures/custom_codeowners_path/expected/CODEOWNERS b/tests/fixtures/custom_codeowners_path/expected/CODEOWNERS new file mode 100644 index 0000000..0a66c8b --- /dev/null +++ b/tests/fixtures/custom_codeowners_path/expected/CODEOWNERS @@ -0,0 +1,14 @@ +# STOP! - DO NOT EDIT THIS FILE MANUALLY +# This file was automatically generated by "bin/codeownership validate". +# +# 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 + + +# Annotations at the top of file +/ruby/app/models/test.rb @TestTeam + +# Team YML ownership +/config/teams/test_team.yml @TestTeam diff --git a/tests/fixtures/custom_codeowners_path/ruby/app/models/test.rb b/tests/fixtures/custom_codeowners_path/ruby/app/models/test.rb new file mode 100644 index 0000000..00a0e56 --- /dev/null +++ b/tests/fixtures/custom_codeowners_path/ruby/app/models/test.rb @@ -0,0 +1,3 @@ +# @team TestTeam +class Test +end diff --git a/tests/git_stage_test.rs b/tests/git_stage_test.rs index ae8f80d..a5a9078 100644 --- a/tests/git_stage_test.rs +++ b/tests/git_stage_test.rs @@ -38,7 +38,10 @@ where let result = func(&run_config, stage); assert_no_run_errors(&result); - assert!(run_config.codeowners_file_path.exists(), "CODEOWNERS file was not created"); + assert!( + run_config.codeowners_file_path.as_ref().unwrap().exists(), + "CODEOWNERS file was not created", + ); let staged = is_file_staged(&run_config.project_root, CODEOWNERS_REL); assert_eq!(staged, expected_staged, "unexpected staged state for CODEOWNERS"); } diff --git a/tests/runner_api.rs b/tests/runner_api.rs index cfbe20a..53a1cc3 100644 --- a/tests/runner_api.rs +++ b/tests/runner_api.rs @@ -35,7 +35,7 @@ team_file_glob: let run_config = RunConfig { project_root: temp_dir.path().to_path_buf(), - codeowners_file_path: temp_dir.path().join(".github/CODEOWNERS").to_path_buf(), + codeowners_file_path: Some(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_name: None, @@ -61,7 +61,7 @@ fn test_teams_for_files_from_codeowners() { ]; let run_config = RunConfig { project_root: project_root.to_path_buf(), - codeowners_file_path: project_root.join(".github/CODEOWNERS").to_path_buf(), + codeowners_file_path: Some(project_root.join(".github/CODEOWNERS").to_path_buf()), config_path: project_root.join("config/code_ownership.yml").to_path_buf(), no_cache: true, executable_name: None, @@ -128,7 +128,7 @@ javascript_package_paths: let rc = RunConfig { project_root: td.path().to_path_buf(), - codeowners_file_path: td.path().join(".github/CODEOWNERS"), + codeowners_file_path: Some(td.path().join(".github/CODEOWNERS")), config_path: td.path().join("config/code_ownership.yml"), no_cache: true, executable_name: None, @@ -170,7 +170,7 @@ javascript_package_paths: let rc = RunConfig { project_root: td.path().to_path_buf(), - codeowners_file_path: td.path().join(".github/CODEOWNERS"), + codeowners_file_path: Some(td.path().join(".github/CODEOWNERS")), config_path: td.path().join("config/code_ownership.yml"), no_cache: true, executable_name: None,