diff --git a/Cargo.lock b/Cargo.lock index 45a55d2..010d6e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -179,7 +179,7 @@ checksum = "1462739cb27611015575c0c11df5df7601141071f07518d56fcc1be504cbec97" [[package]] name = "codeowners" -version = "0.2.8" +version = "0.2.9" dependencies = [ "assert_cmd", "clap", diff --git a/Cargo.toml b/Cargo.toml index 2132df5..00aa843 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "codeowners" -version = "0.2.8" +version = "0.2.9" edition = "2024" [profile.release] diff --git a/src/common_test.rs b/src/common_test.rs index 2e9db19..2dea43f 100644 --- a/src/common_test.rs +++ b/src/common_test.rs @@ -246,13 +246,21 @@ pub mod tests { relative_path: "packs/jscomponents/comp.ts".to_owned(), content: "// @team Foo\n".to_owned(), }, + TestProjectFile { + relative_path: "packs/jscomponents/comp-colon.ts".to_owned(), + content: "// @team: FooColon\n".to_owned(), + }, TestProjectFile { relative_path: "packs/[admin]/comp.ts".to_owned(), content: "// @team Bar\n".to_owned(), }, TestProjectFile { relative_path: "packs/bar/comp.rb".to_owned(), - content: "// @team Bar\n".to_owned(), + content: "# @team Bar\n".to_owned(), + }, + TestProjectFile { + relative_path: "packs/bar/comp-colon.rb".to_owned(), + content: "# @team: BarColon\n".to_owned(), }, ], ); diff --git a/src/ownership/mapper/team_file_mapper.rs b/src/ownership/mapper/team_file_mapper.rs index 3bd8e77..f82d688 100644 --- a/src/ownership/mapper/team_file_mapper.rs +++ b/src/ownership/mapper/team_file_mapper.rs @@ -50,12 +50,12 @@ impl Mapper for TeamFileMapper { for owned_file in &self.project.files { if let Some(ref owner) = owned_file.owner { - let team = team_by_name.get(owner); - - if let Some(team) = team { - let relative_path = self.project.relative_path(&owned_file.path); - path_to_team.insert(relative_path.to_owned(), team.name.clone()); - } + let relative_path = self.project.relative_path(&owned_file.path); + let team_name = team_by_name + .get(owner) + .map(|team| team.name.clone()) + .unwrap_or_else(|| owner.clone()); + path_to_team.insert(relative_path.to_owned(), team_name); } } @@ -114,6 +114,8 @@ mod tests { (PathBuf::from("packs/[admin]/comp.ts"), "Bar".to_owned()), (PathBuf::from("packs/bar/comp.rb"), "Bar".to_owned()), (PathBuf::from("packs/jscomponents/comp.ts"), "Foo".to_owned()), + (PathBuf::from("packs/jscomponents/comp-colon.ts"), "FooColon".to_owned()), + (PathBuf::from("packs/bar/comp-colon.rb"), "BarColon".to_owned()), ]), Source::TeamFile, )]; diff --git a/src/ownership/tests.rs b/src/ownership/tests.rs deleted file mode 100644 index 67d17dd..0000000 --- a/src/ownership/tests.rs +++ /dev/null @@ -1,219 +0,0 @@ -use std::{error::Error, path::Path}; - -use crate::{ - common_test::tests::{build_ownership_with_all_mappers, build_ownership_with_directory_codeowners}, - project::{Package, PackageType, Project, ProjectFile, Team, VendoredGem}, -}; - -use super::Ownership; -use pretty_assertions::assert_eq; - -fn build_payroll_team() -> Team { - Team { - path: Path::new("config/teams/payroll.yml").to_owned(), - name: "Payroll".to_owned(), - github_team: "@Payroll-Eng".to_owned(), - owned_globs: vec![], - avoid_ownership: false, - owned_gems: vec![], - } -} - -fn build_payroll_team_with_owned_gem() -> Team { - Team { - path: Path::new("config/teams/payroll.yml").to_owned(), - name: "Payroll".to_owned(), - github_team: "@Payroll-Eng".to_owned(), - owned_globs: vec![], - avoid_ownership: false, - owned_gems: vec!["payroll_calculator".to_owned()], - } -} - -fn build_annotated_file() -> ProjectFile { - ProjectFile { - owner: Some("Payroll".to_owned()), - path: Path::new("packs/payroll/services/runner.rb").to_owned(), - } -} - -fn build_unannotated_file() -> ProjectFile { - ProjectFile { - owner: None, - path: Path::new("packs/payroll/services/runner_helper.rb").to_owned(), - } -} - -fn build_project_with_annotated_file() -> Project { - Project { - base_path: Path::new("").to_owned(), - files: vec![build_annotated_file(), build_unannotated_file()], - packages: vec![], - teams: vec![build_payroll_team()], - vendored_gems: vec![], - codeowners_file: "".to_owned(), - directory_codeowner_files: vec![], - } -} - -fn build_payroll_team_with_owned_glob() -> Team { - Team { - path: Path::new("config/teams/payroll.yml").to_owned(), - name: "Payroll".to_owned(), - github_team: "@Payroll-Eng".to_owned(), - owned_globs: vec!["packs/payroll/**".to_owned()], - avoid_ownership: false, - owned_gems: vec![], - } -} - -fn build_project_with_team_specific_owned_globs() -> Project { - Project { - base_path: Path::new("").to_owned(), - files: vec![build_unannotated_file()], - packages: vec![], - teams: vec![build_payroll_team_with_owned_glob()], - vendored_gems: vec![], - codeowners_file: "".to_owned(), - directory_codeowner_files: vec![], - } -} - -fn build_project_with_packages() -> Project { - Project { - base_path: Path::new("").to_owned(), - files: vec![build_unannotated_file()], - packages: vec![ - Package { - path: Path::new("packs/payroll_package/package.yml").to_owned(), - package_type: PackageType::Ruby, - owner: "Payroll".to_owned(), - }, - Package { - path: Path::new("frontend/payroll_flow/package.json").to_owned(), - package_type: PackageType::Javascript, - owner: "Payroll".to_owned(), - }, - ], - teams: vec![build_payroll_team()], - vendored_gems: vec![], - codeowners_file: "".to_owned(), - directory_codeowner_files: vec![], - } -} - -fn build_project_with_team_owned_gems() -> Project { - Project { - base_path: Path::new("").to_owned(), - files: vec![build_unannotated_file()], - packages: vec![], - teams: vec![build_payroll_team_with_owned_gem()], - vendored_gems: vec![VendoredGem { - path: Path::new("components/payroll_calculator").to_owned(), - name: "payroll_calculator".to_owned(), - }], - codeowners_file: "".to_owned(), - directory_codeowner_files: vec![], - } -} - -#[test] -fn test_annotations_at_the_top_of_file() { - let ownership = Ownership::build(build_project_with_annotated_file()); - - assert_eq!( - ownership.generate_file(), - with_disclaimer(vec![ - "# Annotations at the top of file", - "/packs/payroll/services/runner.rb @Payroll-Eng", - "", - "# Team YML ownership", - "/config/teams/payroll.yml @Payroll-Eng", - "", - ]) - .join("\n") - ) -} - -#[test] -fn test_team_specific_owned_globs() { - let ownership = Ownership::build(build_project_with_team_specific_owned_globs()); - - assert_eq!( - ownership.generate_file(), - with_disclaimer(vec![ - "# Team-specific owned globs", - "/packs/payroll/** @Payroll-Eng", - "", - "# Team YML ownership", - "/config/teams/payroll.yml @Payroll-Eng", - "", - ]) - .join("\n") - ) -} - -#[test] -fn test_owner_metadata_in_package() { - let ownership = Ownership::build(build_project_with_packages()); - - assert_eq!( - ownership.generate_file(), - with_disclaimer(vec![ - "# Owner metadata key in package.yml", - "/packs/payroll_package/**/** @Payroll-Eng", - "", - "# Owner metadata key in package.json", - "/frontend/payroll_flow/**/** @Payroll-Eng", - "", - "# Team YML ownership", - "/config/teams/payroll.yml @Payroll-Eng", - "", - ]) - .join("\n") - ) -} - -#[test] -fn test_team_owned_gems() { - let ownership = Ownership::build(build_project_with_team_owned_gems()); - - assert_eq!( - ownership.generate_file(), - with_disclaimer(vec![ - "# Team YML ownership", - "/config/teams/payroll.yml @Payroll-Eng", - "", - "# Team owned gems", - "/components/payroll_calculator/**/** @Payroll-Eng", - "" - ]) - .join("\n") - ) -} - -#[test] -fn test_file_with_multiple_directory_owners() -> Result<(), Box> { - let ownership = build_ownership_with_directory_codeowners()?; - let result = ownership.validate(); - assert_eq!(&result.ok(), &Some(())); - Ok(()) -} - -#[test] -fn test_all_mappers() -> Result<(), Box> { - let ownership = build_ownership_with_all_mappers()?; - let result = ownership.validate(); - assert_eq!(&result.ok(), &Some(())); - Ok(()) -} - -fn with_disclaimer(lines: Vec<&str>) -> Vec { - let mut buffer: Vec = Vec::new(); - let mut disclaimer = crate::ownership::file_generator::FileGenerator::disclaimer(); - - buffer.append(&mut disclaimer); - buffer.append(&mut lines.iter().map(|l| l.to_string()).collect()); - - buffer -} diff --git a/src/project_file_builder.rs b/src/project_file_builder.rs index 9a8e79d..05269bb 100644 --- a/src/project_file_builder.rs +++ b/src/project_file_builder.rs @@ -13,7 +13,7 @@ pub struct ProjectFileBuilder<'a> { } lazy_static! { - static ref TEAM_REGEX: Regex = Regex::new(r#"^(?:#|//) @team (.*)$"#).expect("error compiling regular expression"); + static ref TEAM_REGEX: Regex = Regex::new(r#"^(?:#|//) @team:? (.*)$"#).expect("error compiling regular expression"); } impl<'a> ProjectFileBuilder<'a> { @@ -73,3 +73,35 @@ pub(crate) fn build_project_file_without_cache(path: &PathBuf) -> ProjectFile { ProjectFile { path: path.clone(), owner } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_team_regex() { + let owner = TEAM_REGEX + .captures("// @team Foo") + .and_then(|cap| cap.get(1)) + .map(|m| m.as_str().to_string()); + assert_eq!(owner, Some("Foo".to_string())); + + let owner = TEAM_REGEX + .captures("// @team Foo") + .and_then(|cap| cap.get(1)) + .map(|m| m.as_str().to_string()); + assert_eq!(owner, Some("Foo".to_string())); + + let owner = TEAM_REGEX + .captures("// @team: Foo") + .and_then(|cap| cap.get(1)) + .map(|m| m.as_str().to_string()); + assert_eq!(owner, Some("Foo".to_string())); + + let owner = TEAM_REGEX + .captures("# @team: Foo") + .and_then(|cap| cap.get(1)) + .map(|m| m.as_str().to_string()); + assert_eq!(owner, Some("Foo".to_string())); + } +} diff --git a/tests/fixtures/valid_project/.github/CODEOWNERS b/tests/fixtures/valid_project/.github/CODEOWNERS index 6df553a..f39bb1f 100644 --- a/tests/fixtures/valid_project/.github/CODEOWNERS +++ b/tests/fixtures/valid_project/.github/CODEOWNERS @@ -9,6 +9,7 @@ # Annotations at the top of file /javascript/packages/PayrollFlow/index.tsx @PayrollTeam +/javascript/packages/list/page-admin.tsx @PaymentsTeam /ruby/app/models/bank_account.rb @PaymentsTeam /ruby/app/models/payroll.rb @PayrollTeam diff --git a/tests/fixtures/valid_project/javascript/packages/list/page-admin.tsx b/tests/fixtures/valid_project/javascript/packages/list/page-admin.tsx new file mode 100644 index 0000000..3d3deef --- /dev/null +++ b/tests/fixtures/valid_project/javascript/packages/list/page-admin.tsx @@ -0,0 +1 @@ +// @team: Payments \ No newline at end of file diff --git a/tests/fixtures/valid_project/ruby/app/models/payroll.rb b/tests/fixtures/valid_project/ruby/app/models/payroll.rb index ac9c4ed..0e6bba8 100644 --- a/tests/fixtures/valid_project/ruby/app/models/payroll.rb +++ b/tests/fixtures/valid_project/ruby/app/models/payroll.rb @@ -1,3 +1,3 @@ -# @team Payroll +# @team: Payroll class Payroll; end diff --git a/tests/invalid_project_test.rs b/tests/invalid_project_test.rs index 824fa63..4a23f83 100644 --- a/tests/invalid_project_test.rs +++ b/tests/invalid_project_test.rs @@ -34,7 +34,6 @@ fn test_validate() -> Result<(), Box> { - ruby/app/models/blockchain.rb is referencing an invalid team - 'Web3' Some files are missing ownership - - ruby/app/models/blockchain.rb - ruby/app/unowned.rb "}));