diff --git a/src/ownership.rs b/src/ownership.rs index bbd2099..c0422e4 100644 --- a/src/ownership.rs +++ b/src/ownership.rs @@ -122,7 +122,9 @@ impl Ownership { let validator = Validator { project: self.project.clone(), mappers: self.mappers(), - file_generator: FileGenerator { mappers: self.mappers() }, + file_generator: FileGenerator { + mappers: self.codeowners_file_mappers(), + }, executable_name: self.project.executable_name.clone(), }; @@ -166,7 +168,9 @@ impl Ownership { #[instrument(level = "debug", skip_all)] pub fn generate_file(&self) -> String { info!("generating codeowners file"); - let file_generator = FileGenerator { mappers: self.mappers() }; + let file_generator = FileGenerator { + mappers: self.codeowners_file_mappers(), + }; file_generator.generate_file() } @@ -182,6 +186,20 @@ impl Ownership { Box::new(TeamGemMapper::build(self.project.clone())), ] } + + // GitHub applies the last matching CODEOWNERS line, so annotations are written last: an annotated + // file can still match a broader team glob that excludes it through unowned_globs. + fn codeowners_file_mappers(&self) -> Vec> { + vec![ + Box::new(TeamGlobMapper::build(self.project.clone())), + Box::new(DirectoryMapper::build(self.project.clone())), + Box::new(RubyPackageMapper::build(self.project.clone())), + Box::new(JavascriptPackageMapper::build(self.project.clone())), + Box::new(TeamYmlMapper::build(self.project.clone())), + Box::new(TeamGemMapper::build(self.project.clone())), + Box::new(TeamFileMapper::build(self.project.clone())), + ] + } } #[cfg(test)] diff --git a/tests/annotation_in_unowned_team_glob_test.rs b/tests/annotation_in_unowned_team_glob_test.rs new file mode 100644 index 0000000..3ae70b5 --- /dev/null +++ b/tests/annotation_in_unowned_team_glob_test.rs @@ -0,0 +1,64 @@ +use assert_cmd::prelude::*; +use indoc::indoc; +use predicates::prelude::*; +use std::error::Error; +use std::path::Path; +use std::process::Command; +use tempfile::TempDir; + +mod common; +use common::OutputStream; +use common::git_add_all_files; +use common::run_codeowners; +use common::setup_fixture_repo; + +// Alpha owns ruby/app/**/* but excludes beta_owned.rb through unowned_globs, and Beta owns that file +// through an annotation. GitHub applies the last matching CODEOWNERS line, so the annotation line must +// come after Alpha's broader glob for GitHub to agree with for-file. +const FIXTURE: &str = "annotation_in_unowned_team_glob"; + +// Copies the fixture and regenerates its CODEOWNERS, so assertions exercise the generator rather than +// the committed file. +fn fixture_with_generated_codeowners() -> Result> { + let temp_dir = setup_fixture_repo(&Path::new("tests/fixtures").join(FIXTURE)); + git_add_all_files(temp_dir.path()); + codeowners(temp_dir.path(), &["generate"])?.assert().success(); + Ok(temp_dir) +} + +fn codeowners(project_root: &Path, args: &[&str]) -> Result> { + let mut cmd = Command::cargo_bin("codeowners")?; + cmd.arg("--project-root").arg(project_root).arg("--no-cache").args(args); + Ok(cmd) +} + +#[test] +fn test_validate_accepts_generated_order() -> Result<(), Box> { + run_codeowners(FIXTURE, &["validate"], true, OutputStream::Stdout, predicate::eq(""))?; + Ok(()) +} + +#[test] +fn test_generated_codeowners_agrees_with_for_file() -> Result<(), Box> { + let temp_dir = fixture_with_generated_codeowners()?; + codeowners(temp_dir.path(), &["crosscheck-owners"])? + .assert() + .success() + .stdout(predicate::eq(indoc! {" + Success! All files match between CODEOWNERS and for-file command. + "})); + Ok(()) +} + +#[test] +fn test_for_file_from_generated_codeowners_returns_annotated_owner() -> Result<(), Box> { + let temp_dir = fixture_with_generated_codeowners()?; + codeowners( + temp_dir.path(), + &["for-file", "--from-codeowners", "ruby/app/services/beta_owned.rb"], + )? + .assert() + .success() + .stdout(predicate::str::contains("Team: Beta")); + Ok(()) +} diff --git a/tests/executable_name_config_test.rs b/tests/executable_name_config_test.rs index 4dc135d..3ad64b5 100644 --- a/tests/executable_name_config_test.rs +++ b/tests/executable_name_config_test.rs @@ -45,15 +45,15 @@ fn test_custom_executable_name_full_error_message() -> Result<(), Box -# Outdated content to trigger validation error -/app/old.rb @FooTeam + - +# Annotations at the top of file - +/app/foo.rb @FooTeam - + +# Team-specific owned globs +/ruby/app/payments/** @PaymentTeam + +# Team YML ownership +/config/teams/foo.yml @FooTeam +/config/teams/payments.yml @PaymentTeam + + + +# Annotations at the top of file + +/app/foo.rb @FooTeam CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file @@ -74,11 +74,11 @@ fn test_default_executable_name_full_error_message() -> Result<(), Box Result<(), Box> { +# 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 - +/gems/payroll_calculator/calculator.rb @PaymentTeam - +/ruby/app/models/bank_account.rb @PaymentTeam - +/ruby/app/models/payroll.rb @PayrollTeam - +/ruby/app/services/multi_owned.rb @PaymentTeam - + +# Team-specific owned globs +/ruby/app/payments/**/* @PaymentTeam + @@ -44,6 +38,12 @@ fn test_validate() -> Result<(), Box> { + +# Team owned gems +/gems/payroll_calculator/**/** @PayrollTeam + + + +# Annotations at the top of file + +/gems/payroll_calculator/calculator.rb @PaymentTeam + +/ruby/app/models/bank_account.rb @PaymentTeam + +/ruby/app/models/payroll.rb @PayrollTeam + +/ruby/app/services/multi_owned.rb @PaymentTeam CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file diff --git a/tests/valid_project_test.rs b/tests/valid_project_test.rs index f207fac..33d40b0 100644 --- a/tests/valid_project_test.rs +++ b/tests/valid_project_test.rs @@ -299,12 +299,6 @@ fn test_for_team() -> Result<(), Box> { predicate::eq(indoc! {" # Code Ownership Report for `Payroll` Team - ## Annotations at the top of file - /javascript/packages/PayrollFlow/index.tsx - /ruby/app/models/payroll.rb - /ruby/app/views/foos/edit.erb - /ruby/app/views/foos/new.html.erb - ## Team-specific owned globs This team owns nothing in this category. @@ -324,6 +318,12 @@ fn test_for_team() -> Result<(), Box> { ## Team owned gems /gems/payroll_calculator/**/** + + ## Annotations at the top of file + /javascript/packages/PayrollFlow/index.tsx + /ruby/app/models/payroll.rb + /ruby/app/views/foos/edit.erb + /ruby/app/views/foos/new.html.erb "}), )?;