Skip to content

Commit f4e0752

Browse files
martinemdemartinemde
committed
accept file paths to speed up validate
1 parent c35015f commit f4e0752

File tree

4 files changed

+201
-10
lines changed

4 files changed

+201
-10
lines changed

‎src/cli.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,17 @@ enum Command {
3838
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.",
3939
visible_alias = "v"
4040
)]
41-
Validate,
41+
Validate {
42+
#[arg(help = "Optional list of files to validate ownership for (fast mode for git hooks)")]
43+
files: Vec<String>,
44+
},
4245

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

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

114119
let runner_result = match args.command {
115-
Command::Validate => runner::validate(&run_config, vec![]),
120+
Command::Validate { files } => runner::validate(&run_config, files),
116121
Command::Generate { skip_stage } => runner::generate(&run_config, !skip_stage),
117-
Command::GenerateAndValidate { skip_stage } => runner::generate_and_validate(&run_config, vec![], !skip_stage),
122+
Command::GenerateAndValidate { files, skip_stage } => runner::generate_and_validate(&run_config, files, !skip_stage),
118123
Command::ForFile {
119124
name,
120125
from_codeowners,

‎src/runner.rs‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,15 @@ impl Runner {
9191
})
9292
}
9393

94-
pub fn validate(&self) -> RunResult {
94+
pub fn validate(&self, file_paths: Vec<String>) -> RunResult {
95+
if file_paths.is_empty() {
96+
self.validate_all()
97+
} else {
98+
self.validate_files(file_paths)
99+
}
100+
}
101+
102+
fn validate_all(&self) -> RunResult {
95103
match self.ownership.validate() {
96104
Ok(_) => RunResult::default(),
97105
Err(err) => RunResult {
@@ -101,6 +109,40 @@ impl Runner {
101109
}
102110
}
103111

112+
fn validate_files(&self, file_paths: Vec<String>) -> RunResult {
113+
let mut unowned_files = Vec::new();
114+
let mut io_errors = Vec::new();
115+
116+
for file_path in file_paths {
117+
match team_for_file_from_codeowners(&self.run_config, &file_path) {
118+
Ok(Some(_)) => {}
119+
Ok(None) => unowned_files.push(file_path),
120+
Err(err) => io_errors.push(format!("{}: {}", file_path, err)),
121+
}
122+
}
123+
124+
if !unowned_files.is_empty() {
125+
let validation_errors = std::iter::once("Unowned files detected:".to_string())
126+
.chain(unowned_files.into_iter().map(|file| format!(" {}", file)))
127+
.collect();
128+
129+
return RunResult {
130+
validation_errors,
131+
io_errors,
132+
..Default::default()
133+
};
134+
}
135+
136+
if !io_errors.is_empty() {
137+
return RunResult {
138+
io_errors,
139+
..Default::default()
140+
};
141+
}
142+
143+
RunResult::default()
144+
}
145+
104146
pub fn generate(&self, git_stage: bool) -> RunResult {
105147
let content = self.ownership.generate_file();
106148
if let Some(parent) = &self.run_config.codeowners_file_path.parent() {
@@ -120,12 +162,12 @@ impl Runner {
120162
}
121163
}
122164

123-
pub fn generate_and_validate(&self, git_stage: bool) -> RunResult {
165+
pub fn generate_and_validate(&self, file_paths: Vec<String>, git_stage: bool) -> RunResult {
124166
let run_result = self.generate(git_stage);
125167
if run_result.has_errors() {
126168
return run_result;
127169
}
128-
self.validate()
170+
self.validate(file_paths)
129171
}
130172

131173
fn git_stage(&self) {

‎src/runner/api.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,16 @@ pub fn for_team(run_config: &RunConfig, team_name: &str) -> RunResult {
1616
run(run_config, |runner| runner.for_team(team_name))
1717
}
1818

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

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

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

3131
pub fn delete_cache(run_config: &RunConfig) -> RunResult {

‎tests/validate_files_test.rs‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
use assert_cmd::prelude::*;
2+
use predicates::prelude::*;
3+
use std::{error::Error, process::Command};
4+
5+
mod common;
6+
7+
use common::*;
8+
9+
#[test]
10+
fn test_validate_with_owned_files() -> Result<(), Box<dyn Error>> {
11+
run_codeowners(
12+
"valid_project",
13+
&["validate", "ruby/app/models/payroll.rb", "ruby/app/models/bank_account.rb"],
14+
true,
15+
OutputStream::Stdout,
16+
predicate::eq(""),
17+
)?;
18+
19+
Ok(())
20+
}
21+
22+
#[test]
23+
fn test_validate_with_unowned_file() -> Result<(), Box<dyn Error>> {
24+
run_codeowners(
25+
"valid_project",
26+
&["validate", "ruby/app/unowned.rb"],
27+
false,
28+
OutputStream::Stdout,
29+
predicate::str::contains("ruby/app/unowned.rb").and(predicate::str::contains("Unowned")),
30+
)?;
31+
32+
Ok(())
33+
}
34+
35+
#[test]
36+
fn test_validate_with_mixed_files() -> Result<(), Box<dyn Error>> {
37+
run_codeowners(
38+
"valid_project",
39+
&["validate", "ruby/app/models/payroll.rb", "ruby/app/unowned.rb"],
40+
false,
41+
OutputStream::Stdout,
42+
predicate::str::contains("ruby/app/unowned.rb").and(predicate::str::contains("Unowned")),
43+
)?;
44+
45+
Ok(())
46+
}
47+
48+
#[test]
49+
fn test_validate_with_no_files() -> Result<(), Box<dyn Error>> {
50+
// Existing behavior - validates entire project
51+
run_codeowners("valid_project", &["validate"], true, OutputStream::Stdout, predicate::eq(""))?;
52+
53+
Ok(())
54+
}
55+
56+
#[test]
57+
fn test_generate_and_validate_with_owned_files() -> Result<(), Box<dyn Error>> {
58+
let fixture_root = std::path::Path::new("tests/fixtures/valid_project");
59+
let temp_dir = setup_fixture_repo(fixture_root);
60+
let project_root = temp_dir.path();
61+
git_add_all_files(project_root);
62+
63+
let codeowners_path = project_root.join("tmp/CODEOWNERS");
64+
65+
Command::cargo_bin("codeowners")?
66+
.arg("--project-root")
67+
.arg(project_root)
68+
.arg("--codeowners-file-path")
69+
.arg(&codeowners_path)
70+
.arg("--no-cache")
71+
.arg("generate-and-validate")
72+
.arg("ruby/app/models/payroll.rb")
73+
.arg("ruby/app/models/bank_account.rb")
74+
.assert()
75+
.success();
76+
77+
Ok(())
78+
}
79+
80+
#[test]
81+
fn test_generate_and_validate_with_unowned_file() -> Result<(), Box<dyn Error>> {
82+
let fixture_root = std::path::Path::new("tests/fixtures/valid_project");
83+
let temp_dir = setup_fixture_repo(fixture_root);
84+
let project_root = temp_dir.path();
85+
git_add_all_files(project_root);
86+
87+
let codeowners_path = project_root.join("tmp/CODEOWNERS");
88+
89+
Command::cargo_bin("codeowners")?
90+
.arg("--project-root")
91+
.arg(project_root)
92+
.arg("--codeowners-file-path")
93+
.arg(&codeowners_path)
94+
.arg("--no-cache")
95+
.arg("generate-and-validate")
96+
.arg("ruby/app/unowned.rb")
97+
.assert()
98+
.failure()
99+
.stdout(predicate::str::contains("ruby/app/unowned.rb"))
100+
.stdout(predicate::str::contains("Unowned"));
101+
102+
Ok(())
103+
}
104+
105+
#[test]
106+
fn test_validate_with_absolute_path() -> Result<(), Box<dyn Error>> {
107+
let fixture_root = std::path::Path::new("tests/fixtures/valid_project");
108+
let temp_dir = setup_fixture_repo(fixture_root);
109+
let project_root = temp_dir.path();
110+
git_add_all_files(project_root);
111+
112+
let file_absolute_path = project_root.join("ruby/app/models/payroll.rb").canonicalize()?;
113+
114+
Command::cargo_bin("codeowners")?
115+
.arg("--project-root")
116+
.arg(project_root)
117+
.arg("--no-cache")
118+
.arg("validate")
119+
.arg(file_absolute_path.to_str().unwrap())
120+
.assert()
121+
.success();
122+
123+
Ok(())
124+
}
125+
126+
#[test]
127+
fn test_validate_only_checks_codeowners_file() -> Result<(), Box<dyn Error>> {
128+
// This test demonstrates that `validate` with files only checks the CODEOWNERS file
129+
// It does NOT check file annotations or other ownership sources
130+
//
131+
// If a file has an annotation but is missing from CODEOWNERS, `validate` will report it as unowned
132+
// This is why `generate-and-validate` should be used for accuracy
133+
134+
// ruby/app/models/bank_account.rb has @team Payments annotation and is in CODEOWNERS
135+
run_codeowners(
136+
"valid_project",
137+
&["validate", "ruby/app/models/bank_account.rb"],
138+
true,
139+
OutputStream::Stdout,
140+
predicate::eq(""),
141+
)?;
142+
143+
Ok(())
144+
}

0 commit comments

Comments
 (0)