diff --git a/src/ownership/validator.rs b/src/ownership/validator.rs index 664de3a..39509d9 100644 --- a/src/ownership/validator.rs +++ b/src/ownership/validator.rs @@ -65,6 +65,7 @@ impl Validator { errors.append(&mut self.invalid_team_annotation(&team_names)); errors.append(&mut self.invalid_package_ownership(&team_names)); + errors.append(&mut self.invalid_directory_ownership(&team_names)); errors } @@ -107,6 +108,20 @@ impl Validator { .collect() } + /// `DirectoryMapper::entries` skips unresolvable owners, so the directory silently + /// inherits its ancestor's owner and nothing else reports the bad name. + fn invalid_directory_ownership(&self, team_names: &HashSet<&TeamName>) -> Vec { + self.project + .directory_codeowner_files + .iter() + .filter(|directory_codeowner_file| !team_names.contains(&directory_codeowner_file.owner)) + .map(|directory_codeowner_file| Error::InvalidTeam { + name: directory_codeowner_file.owner.clone(), + path: self.project.relative_path(&directory_codeowner_file.path).to_owned(), + }) + .collect() + } + fn validate_file_ownership(&self) -> Vec { let mut validation_errors = Vec::new(); diff --git a/tests/fixtures/invalid-directory-codeowner/.github/CODEOWNERS b/tests/fixtures/invalid-directory-codeowner/.github/CODEOWNERS new file mode 100644 index 0000000..fed28cf --- /dev/null +++ b/tests/fixtures/invalid-directory-codeowner/.github/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 + + +# Owner in .codeowner +/app/services/**/** @footeam + +# Team YML ownership +/config/teams/foo.yml @footeam diff --git a/tests/fixtures/invalid-directory-codeowner/app/services/.codeowner b/tests/fixtures/invalid-directory-codeowner/app/services/.codeowner new file mode 100644 index 0000000..bc56c4d --- /dev/null +++ b/tests/fixtures/invalid-directory-codeowner/app/services/.codeowner @@ -0,0 +1 @@ +Foo diff --git a/tests/fixtures/invalid-directory-codeowner/app/services/nested/.codeowner b/tests/fixtures/invalid-directory-codeowner/app/services/nested/.codeowner new file mode 100644 index 0000000..c2075a5 --- /dev/null +++ b/tests/fixtures/invalid-directory-codeowner/app/services/nested/.codeowner @@ -0,0 +1 @@ +Web3 diff --git a/tests/fixtures/invalid-directory-codeowner/app/services/nested/nested_file.rb b/tests/fixtures/invalid-directory-codeowner/app/services/nested/nested_file.rb new file mode 100644 index 0000000..d19bcc1 --- /dev/null +++ b/tests/fixtures/invalid-directory-codeowner/app/services/nested/nested_file.rb @@ -0,0 +1,2 @@ +class NestedFile +end diff --git a/tests/fixtures/invalid-directory-codeowner/config/code_ownership.yml b/tests/fixtures/invalid-directory-codeowner/config/code_ownership.yml new file mode 100644 index 0000000..c76f028 --- /dev/null +++ b/tests/fixtures/invalid-directory-codeowner/config/code_ownership.yml @@ -0,0 +1,10 @@ +--- +owned_globs: + - "{app,components,config,frontend,lib,packs,spec}/**/*.{rb,rake,js,jsx,ts,tsx,json,yml}" +unowned_globs: + - config/code_ownership.yml +javascript_package_paths: + - javascript/packages/** +vendored_gems_path: gems +team_file_glob: + - config/teams/**/*.yml \ No newline at end of file diff --git a/tests/fixtures/invalid-directory-codeowner/config/teams/foo.yml b/tests/fixtures/invalid-directory-codeowner/config/teams/foo.yml new file mode 100644 index 0000000..7c3977c --- /dev/null +++ b/tests/fixtures/invalid-directory-codeowner/config/teams/foo.yml @@ -0,0 +1,5 @@ +name: Foo +github: + team: "@footeam" + members: + - fooer diff --git a/tests/invalid_directory_codeowner_test.rs b/tests/invalid_directory_codeowner_test.rs new file mode 100644 index 0000000..260db32 --- /dev/null +++ b/tests/invalid_directory_codeowner_test.rs @@ -0,0 +1,25 @@ +use indoc::indoc; +use predicates::prelude::*; +use std::error::Error; + +mod common; +use common::OutputStream; +use common::run_codeowners; + +/// A nested `.codeowner` naming an unregistered team, under one naming a real team: +/// ownership falls through to the ancestor, so nothing else reports the bad name. +#[test] +fn test_validate_reports_directory_codeowner_with_invalid_team() -> Result<(), Box> { + run_codeowners( + "invalid-directory-codeowner", + &["validate"], + false, + OutputStream::Stdout, + predicate::str::contains(indoc! {" + Found invalid team annotations + - app/services/nested/.codeowner is referencing an invalid team - 'Web3' + "}), + )?; + + Ok(()) +}