test coverage for error messages

Cargo.toml

@@ -24,3 +24,4 @@ tracing-subscriber = { version = "0.3.16", features = ["env-filter"] }
 [dev-dependencies]
 assert_cmd = "2.0.10"
 rusty-hook = "^0.11.2"
+predicates = "3.0.2"

src/main.rs

@@ -1,4 +1,4 @@
-use ext::IntoContext;
+use error_stack_ext::IntoContext;
 use ownership::Ownership;
 
 use crate::project::Project;
@@ -13,7 +13,7 @@ use std::{
 };
 
 mod config;
-mod ext;
+mod error_stack_ext;
 mod ownership;
 mod project;
 
@@ -85,7 +85,7 @@ impl Context for Error {}
 
 fn main() -> Result<(), Error> {
  install_logger();
-print_validation_errors_to_stdout(cli())?;
+maybe_print_errors(cli())?;
 
  Ok(())
 }
@@ -99,14 +99,13 @@ fn cli() -> Result<(), Error> {
 
  let config_file = File::open(&config_path)
  .into_context(Error::Io)
- .attach_printable(format!("{}", config_path.to_string_lossy()))?;
+ .attach_printable(format!("Can't open config file: {}", config_path.to_string_lossy()))?;
 
  let config = serde_yaml::from_reader(config_file).into_context(Error::Io)?;
 
  let ownership = Ownership::build(Project::build(&project_root, &codeowners_file_path, &config).change_context(Error::Io)?);
- let command = args.command;
 
- match command {
+ match args.command {
  Command::Validate => ownership.validate().into_context(Error::ValidationFailed)?,
  Command::Generate => {
  std::fs::write(codeowners_file_path, ownership.generate_file()).into_context(Error::Io)?;
@@ -120,7 +119,7 @@ fn cli() -> Result<(), Error> {
  Ok(())
 }
 
-fn print_validation_errors_to_stdout(result: Result<(), Error>) -> Result<(), Error> {
+fn maybe_print_errors(result: Result<(), Error>) -> Result<(), Error> {
  if let Err(error) = result {
  if let Some(validation_errors) = error.downcast_ref::<ownership::ValidatorErrors>() {
  println!("{}", validation_errors);

src/ownership/validator.rs

@@ -157,6 +157,8 @@ impl Error {
 impl Display for Errors {
  fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
  let grouped_errors = self.0.iter().into_group_map_by(|error| error.error_category_message());
+ let grouped_errors = Vec::from_iter(grouped_errors.iter());
+
  for (error_category_message, errors) in grouped_errors {
  write!(f, "\n{}", error_category_message)?;
 

src/project.rs

@@ -13,7 +13,7 @@ use rayon::prelude::{IntoParallelIterator, ParallelIterator};
 use regex::Regex;
 use tracing::{debug, instrument};
 
-use crate::{config::Config, ext::IntoContext};
+use crate::{config::Config, error_stack_ext::IntoContext};
 use glob_match::glob_match;
 
 pub struct Project {
@@ -131,7 +131,7 @@ impl Context for Error {}
 impl Project {
  #[instrument(level = "debug", skip_all)]
  pub fn build(base_path: &Path, codeowners_file_path: &Path, config: &Config) -> Result<Self, Error> {
- debug!("scanning project ({})", base_path.to_string_lossy());
+ debug!(base_path = base_path.to_str(), "scanning project");
 
  let mut owned_file_paths: Vec<PathBuf> = Vec::new();
  let mut packages: Vec<Package> = Vec::new();

tests/fixtures/invalid_project/.github/CODEOWNERS

@@ -0,0 +1 @@
+

tests/fixtures/invalid_project/config/code_ownership.yml

@@ -0,0 +1,10 @@
+owned_globs:
+ - "**/*.{rb,tsx}"
+ruby_package_paths:
+ - ruby/packages/**/*
+javascript_package_paths:
+ - javascript/packages/**
+team_file_glob:
+ - config/teams/**/*.yml
+vendored_gems_path: gems
+unowned_globs:

tests/fixtures/invalid_project/config/teams/payments.yml

@@ -0,0 +1,5 @@
+name: Payments
+github:
+ team: '@PaymentTeam'
+owned_globs:
+ - ruby/app/payments/**/*

tests/fixtures/invalid_project/config/teams/payroll.yml

@@ -0,0 +1,9 @@
+name: Payroll
+github:
+ team: '@PayrollTeam'
+ruby:
+ owned_gems:
+ - payroll_calculator
+javascript:
+ owned_packages:
+ - 'PayrollFlow'

tests/fixtures/invalid_project/gems/payroll_calculator/calculator.rb

@@ -0,0 +1,6 @@
+# @team Payments
+class PayrollCalculator
+ def calculate
+ 10_000
+ end
+end

tests/fixtures/invalid_project/ruby/app/models/bank_account.rb

@@ -0,0 +1,3 @@
+# @team Payments
+
+class BankAccount; end

tests/fixtures/invalid_project/ruby/app/models/payroll.rb

@@ -0,0 +1,3 @@
+# @team Payroll
+
+class Payroll; end

tests/fixtures/invalid_project/ruby/app/payments/nacha.rb

@@ -0,0 +1 @@
+class Nacha; end

tests/fixtures/invalid_project/ruby/packages/payroll_flow/package.yml

@@ -0,0 +1,2 @@
+metadata:
+ owner: Payroll

tests/fixtures/valid_project/.github/CODEOWNERS

@@ -13,7 +13,7 @@
 /ruby/app/models/payroll.rb @PayrollTeam
 
 # Team-specific owned globs
-/ruby/app/payments/**/* @PayrollTeam
+/ruby/app/payments/**/* @PaymentsTeam
 
 # Owner metadata key in package.yml
 /ruby/packages/payroll_flow/**/** @PayrollTeam
@@ -22,8 +22,8 @@
 /javascript/packages/PayrollFlow/**/** @PayrollTeam
 
 # Team YML ownership
-/config/teams/payments.yml @PayrollTeam
-/config/teams/payroll.yml @PaymentsTeam
+/config/teams/payments.yml @PaymentsTeam
+/config/teams/payroll.yml @PayrollTeam
 
 # Team owned gems
-/gems/payroll_calculator @PaymentsTeam
+/gems/payroll_calculator @PayrollTeam

tests/fixtures/valid_project/config/teams/payments.yml

@@ -1,5 +1,5 @@
-name: Payroll
+name: Payments
 github:
- team: '@PayrollTeam'
+ team: '@PaymentsTeam'
 owned_globs:
  - ruby/app/payments/**/*

tests/fixtures/valid_project/config/teams/payroll.yml

@@ -1,6 +1,6 @@
-name: Payments
+name: Payroll
 github:
- team: '@PaymentsTeam'
+ team: '@PayrollTeam'
 ruby:
  owned_gems:
  - payroll_calculator

tests/invalid_project_test.rs

@@ -0,0 +1,18 @@
+use assert_cmd::prelude::*;
+use predicates::prelude::*;
+use std::{error::Error, process::Command};
+
+#[test]
+fn test_validate() -> Result<(), Box<dyn Error>> {
+ Command::cargo_bin("codeowners")?
+ .arg("--project-root")
+ .arg("tests/fixtures/invalid_project")
+ .arg("validate")
+ .assert()
+ .failure()
+ .stdout(predicate::str::contains("CODEOWNERS out of date. Run `codeownership generate` to update the CODEOWNERS file"))
+ .stdout(predicate::str::contains("Some files are missing ownership:\n- ruby/app/unowned.rb"))
+ .stdout(predicate::str::contains("Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways.\n- gems/payroll_calculator/calculator.rb (owner: Payments, source: team_file_mapper)\n- gems/payroll_calculator/calculator.rb (owner: Payroll, source: team_gem_mapper)"));
+
+ Ok(())
+}