Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,17 @@ enum Command {
about = "Validate the validity of the CODEOWNERS file. A validation failure will exit with a failure code and a detailed output of the validation errors.",
visible_alias = "v"
)]
Validate,
Validate {
#[arg(help = "Optional list of files to validate ownership for (fast mode for git hooks)")]
files: Vec<String>,
},

#[clap(about = "Chains both `generate` and `validate` commands.", visible_alias = "gv")]
GenerateAndValidate {
#[arg(long, short, default_value = "false", help = "Skip staging the CODEOWNERS file")]
skip_stage: bool,
#[arg(help = "Optional list of files to validate ownership for (fast mode for git hooks)")]
files: Vec<String>,
},

#[clap(about = "Delete the cache file.", visible_alias = "d")]
Expand Down Expand Up @@ -112,9 +117,9 @@ pub fn cli() -> Result<RunResult, RunnerError> {
};

let runner_result = match args.command {
Command::Validate => runner::validate(&run_config, vec![]),
Command::Validate { files } => runner::validate(&run_config, files),
Command::Generate { skip_stage } => runner::generate(&run_config, !skip_stage),
Command::GenerateAndValidate { skip_stage } => runner::generate_and_validate(&run_config, vec![], !skip_stage),
Command::GenerateAndValidate { files, skip_stage } => runner::generate_and_validate(&run_config, files, !skip_stage),
Command::ForFile {
name,
from_codeowners,
Expand Down
48 changes: 45 additions & 3 deletions src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,15 @@ impl Runner {
})
}

pub fn validate(&self) -> RunResult {
pub fn validate(&self, file_paths: Vec<String>) -> RunResult {
if file_paths.is_empty() {
self.validate_all()
} else {
self.validate_files(file_paths)
}
}

fn validate_all(&self) -> RunResult {
match self.ownership.validate() {
Ok(_) => RunResult::default(),
Err(err) => RunResult {
Expand All @@ -101,6 +109,40 @@ impl Runner {
}
}

fn validate_files(&self, file_paths: Vec<String>) -> RunResult {
let mut unowned_files = Vec::new();
let mut io_errors = Vec::new();

for file_path in file_paths {
match team_for_file_from_codeowners(&self.run_config, &file_path) {
Ok(Some(_)) => {}
Ok(None) => unowned_files.push(file_path),
Err(err) => io_errors.push(format!("{}: {}", file_path, err)),
}
}

if !unowned_files.is_empty() {
let validation_errors = std::iter::once("Unowned files detected:".to_string())
.chain(unowned_files.into_iter().map(|file| format!(" {}", file)))
.collect();

return RunResult {
validation_errors,
io_errors,
..Default::default()
};
}

if !io_errors.is_empty() {
return RunResult {
io_errors,
..Default::default()
};
}

RunResult::default()
}

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() {
Expand All @@ -120,12 +162,12 @@ impl Runner {
}
}

pub fn generate_and_validate(&self, git_stage: bool) -> RunResult {
pub fn generate_and_validate(&self, file_paths: Vec<String>, git_stage: bool) -> RunResult {
let run_result = self.generate(git_stage);
if run_result.has_errors() {
return run_result;
}
self.validate()
self.validate(file_paths)
}

fn git_stage(&self) {
Expand Down
8 changes: 4 additions & 4 deletions src/runner/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,16 @@ pub fn for_team(run_config: &RunConfig, team_name: &str) -> RunResult {
run(run_config, |runner| runner.for_team(team_name))
}

pub fn validate(run_config: &RunConfig, _file_paths: Vec<String>) -> RunResult {
run(run_config, |runner| runner.validate())
pub fn validate(run_config: &RunConfig, file_paths: Vec<String>) -> RunResult {
run(run_config, |runner| runner.validate(file_paths))
}

pub fn generate(run_config: &RunConfig, git_stage: bool) -> RunResult {
run(run_config, |runner| runner.generate(git_stage))
}

pub fn generate_and_validate(run_config: &RunConfig, _file_paths: Vec<String>, git_stage: bool) -> RunResult {
run(run_config, |runner| runner.generate_and_validate(git_stage))
pub fn generate_and_validate(run_config: &RunConfig, file_paths: Vec<String>, git_stage: bool) -> RunResult {
run(run_config, |runner| runner.generate_and_validate(file_paths, git_stage))
}

pub fn delete_cache(run_config: &RunConfig) -> RunResult {
Expand Down
144 changes: 144 additions & 0 deletions tests/validate_files_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
use assert_cmd::prelude::*;
use predicates::prelude::*;
use std::{error::Error, process::Command};

mod common;

use common::*;

#[test]
fn test_validate_with_owned_files() -> Result<(), Box<dyn Error>> {
run_codeowners(
"valid_project",
&["validate", "ruby/app/models/payroll.rb", "ruby/app/models/bank_account.rb"],
true,
OutputStream::Stdout,
predicate::eq(""),
)?;

Ok(())
}

#[test]
fn test_validate_with_unowned_file() -> Result<(), Box<dyn Error>> {
run_codeowners(
"valid_project",
&["validate", "ruby/app/unowned.rb"],
false,
OutputStream::Stdout,
predicate::str::contains("ruby/app/unowned.rb").and(predicate::str::contains("Unowned")),
)?;

Ok(())
}

#[test]
fn test_validate_with_mixed_files() -> Result<(), Box<dyn Error>> {
run_codeowners(
"valid_project",
&["validate", "ruby/app/models/payroll.rb", "ruby/app/unowned.rb"],
false,
OutputStream::Stdout,
predicate::str::contains("ruby/app/unowned.rb").and(predicate::str::contains("Unowned")),
)?;

Ok(())
}

#[test]
fn test_validate_with_no_files() -> Result<(), Box<dyn Error>> {
// Existing behavior - validates entire project
run_codeowners("valid_project", &["validate"], true, OutputStream::Stdout, predicate::eq(""))?;

Ok(())
}

#[test]
fn test_generate_and_validate_with_owned_files() -> Result<(), Box<dyn Error>> {
let fixture_root = std::path::Path::new("tests/fixtures/valid_project");
let temp_dir = setup_fixture_repo(fixture_root);
let project_root = temp_dir.path();
git_add_all_files(project_root);

let codeowners_path = project_root.join("tmp/CODEOWNERS");

Command::cargo_bin("codeowners")?
.arg("--project-root")
.arg(project_root)
.arg("--codeowners-file-path")
.arg(&codeowners_path)
.arg("--no-cache")
.arg("generate-and-validate")
.arg("ruby/app/models/payroll.rb")
.arg("ruby/app/models/bank_account.rb")
.assert()
.success();

Ok(())
}

#[test]
fn test_generate_and_validate_with_unowned_file() -> Result<(), Box<dyn Error>> {
let fixture_root = std::path::Path::new("tests/fixtures/valid_project");
let temp_dir = setup_fixture_repo(fixture_root);
let project_root = temp_dir.path();
git_add_all_files(project_root);

let codeowners_path = project_root.join("tmp/CODEOWNERS");

Command::cargo_bin("codeowners")?
.arg("--project-root")
.arg(project_root)
.arg("--codeowners-file-path")
.arg(&codeowners_path)
.arg("--no-cache")
.arg("generate-and-validate")
.arg("ruby/app/unowned.rb")
.assert()
.failure()
.stdout(predicate::str::contains("ruby/app/unowned.rb"))
.stdout(predicate::str::contains("Unowned"));

Ok(())
}

#[test]
fn test_validate_with_absolute_path() -> Result<(), Box<dyn Error>> {
let fixture_root = std::path::Path::new("tests/fixtures/valid_project");
let temp_dir = setup_fixture_repo(fixture_root);
let project_root = temp_dir.path();
git_add_all_files(project_root);

let file_absolute_path = project_root.join("ruby/app/models/payroll.rb").canonicalize()?;

Command::cargo_bin("codeowners")?
.arg("--project-root")
.arg(project_root)
.arg("--no-cache")
.arg("validate")
.arg(file_absolute_path.to_str().unwrap())
.assert()
.success();

Ok(())
}

#[test]
fn test_validate_only_checks_codeowners_file() -> Result<(), Box<dyn Error>> {
// This test demonstrates that `validate` with files only checks the CODEOWNERS file
// It does NOT check file annotations or other ownership sources
//
// If a file has an annotation but is missing from CODEOWNERS, `validate` will report it as unowned
// This is why `generate-and-validate` should be used for accuracy

// ruby/app/models/bank_account.rb has @team Payments annotation and is in CODEOWNERS
run_codeowners(
"valid_project",
&["validate", "ruby/app/models/bank_account.rb"],
true,
OutputStream::Stdout,
predicate::eq(""),
)?;

Ok(())
}