Skip to content

Commit 5899d1e

Browse files
committed
accept file paths to speed up validate
1 parent c35015f commit 5899d1e

File tree

3 files changed

+212
-7
lines changed

3 files changed

+212
-7
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/api.rs

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,66 @@ 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+
fn validate_files_fast(run_config: &RunConfig, file_paths: Vec<String>) -> RunResult {
20+
let mut unowned_files = Vec::new();
21+
let mut io_errors = Vec::new();
22+
23+
for file_path in file_paths {
24+
match team_for_file_from_codeowners(run_config, &file_path) {
25+
Ok(Some(_)) => {}
26+
Ok(None) => unowned_files.push(file_path),
27+
Err(err) => io_errors.push(format!("{}: {}", file_path, err)),
28+
}
29+
}
30+
31+
if !unowned_files.is_empty() {
32+
let validation_errors = std::iter::once("Unowned files detected:".to_string())
33+
.chain(unowned_files.into_iter().map(|file| format!(" {}", file)))
34+
.collect();
35+
36+
return RunResult {
37+
validation_errors,
38+
io_errors,
39+
..Default::default()
40+
};
41+
}
42+
43+
if !io_errors.is_empty() {
44+
return RunResult {
45+
io_errors,
46+
..Default::default()
47+
};
48+
}
49+
50+
RunResult::default()
51+
}
52+
53+
pub fn validate(run_config: &RunConfig, file_paths: Vec<String>) -> RunResult {
54+
if file_paths.is_empty() {
55+
// Existing behavior - validate entire project
56+
run(run_config, |runner| runner.validate())
57+
} else {
58+
// Fast file-first approach for git hooks
59+
validate_files_fast(run_config, file_paths)
60+
}
2161
}
2262

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

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))
67+
pub fn generate_and_validate(run_config: &RunConfig, file_paths: Vec<String>, git_stage: bool) -> RunResult {
68+
if file_paths.is_empty() {
69+
// Existing behavior - generate and validate entire project
70+
run(run_config, |runner| runner.generate_and_validate(git_stage))
71+
} else {
72+
// Fast file-first approach for git hooks
73+
let run_result = run(run_config, |runner| runner.generate(git_stage));
74+
if run_result.has_errors() {
75+
return run_result;
76+
}
77+
validate_files_fast(run_config, file_paths)
78+
}
2979
}
3080

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

tests/validate_files_test.rs

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
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(
52+
"valid_project",
53+
&["validate"],
54+
true,
55+
OutputStream::Stdout,
56+
predicate::eq(""),
57+
)?;
58+
59+
Ok(())
60+
}
61+
62+
#[test]
63+
fn test_generate_and_validate_with_owned_files() -> Result<(), Box<dyn Error>> {
64+
let fixture_root = std::path::Path::new("tests/fixtures/valid_project");
65+
let temp_dir = setup_fixture_repo(fixture_root);
66+
let project_root = temp_dir.path();
67+
git_add_all_files(project_root);
68+
69+
let codeowners_path = project_root.join("tmp/CODEOWNERS");
70+
71+
Command::cargo_bin("codeowners")?
72+
.arg("--project-root")
73+
.arg(project_root)
74+
.arg("--codeowners-file-path")
75+
.arg(&codeowners_path)
76+
.arg("--no-cache")
77+
.arg("generate-and-validate")
78+
.arg("ruby/app/models/payroll.rb")
79+
.arg("ruby/app/models/bank_account.rb")
80+
.assert()
81+
.success();
82+
83+
Ok(())
84+
}
85+
86+
#[test]
87+
fn test_generate_and_validate_with_unowned_file() -> Result<(), Box<dyn Error>> {
88+
let fixture_root = std::path::Path::new("tests/fixtures/valid_project");
89+
let temp_dir = setup_fixture_repo(fixture_root);
90+
let project_root = temp_dir.path();
91+
git_add_all_files(project_root);
92+
93+
let codeowners_path = project_root.join("tmp/CODEOWNERS");
94+
95+
Command::cargo_bin("codeowners")?
96+
.arg("--project-root")
97+
.arg(project_root)
98+
.arg("--codeowners-file-path")
99+
.arg(&codeowners_path)
100+
.arg("--no-cache")
101+
.arg("generate-and-validate")
102+
.arg("ruby/app/unowned.rb")
103+
.assert()
104+
.failure()
105+
.stdout(predicate::str::contains("ruby/app/unowned.rb"))
106+
.stdout(predicate::str::contains("Unowned"));
107+
108+
Ok(())
109+
}
110+
111+
#[test]
112+
fn test_validate_with_absolute_path() -> Result<(), Box<dyn Error>> {
113+
let fixture_root = std::path::Path::new("tests/fixtures/valid_project");
114+
let temp_dir = setup_fixture_repo(fixture_root);
115+
let project_root = temp_dir.path();
116+
git_add_all_files(project_root);
117+
118+
let file_absolute_path = project_root.join("ruby/app/models/payroll.rb").canonicalize()?;
119+
120+
Command::cargo_bin("codeowners")?
121+
.arg("--project-root")
122+
.arg(project_root)
123+
.arg("--no-cache")
124+
.arg("validate")
125+
.arg(file_absolute_path.to_str().unwrap())
126+
.assert()
127+
.success();
128+
129+
Ok(())
130+
}
131+
132+
#[test]
133+
fn test_validate_only_checks_codeowners_file() -> Result<(), Box<dyn Error>> {
134+
// This test demonstrates that `validate` with files only checks the CODEOWNERS file
135+
// It does NOT check file annotations or other ownership sources
136+
//
137+
// If a file has an annotation but is missing from CODEOWNERS, `validate` will report it as unowned
138+
// This is why `generate-and-validate` should be used for accuracy
139+
140+
// ruby/app/models/bank_account.rb has @team Payments annotation and is in CODEOWNERS
141+
run_codeowners(
142+
"valid_project",
143+
&["validate", "ruby/app/models/bank_account.rb"],
144+
true,
145+
OutputStream::Stdout,
146+
predicate::eq(""),
147+
)?;
148+
149+
Ok(())
150+
}

0 commit comments

Comments
 (0)