diff --git a/src/project_builder.rs b/src/project_builder.rs index e717b65..689fd10 100644 --- a/src/project_builder.rs +++ b/src/project_builder.rs @@ -73,16 +73,30 @@ impl<'a> ProjectBuilder<'a> { // Prune traversal early: skip heavy and irrelevant directories let ignore_dirs = self.config.ignore_dirs.clone(); + let team_file_glob = self.config.team_file_glob.clone(); let base_path = self.base_path.clone(); let tracked_files = tracked_files::find_tracked_files(&self.base_path); builder.filter_entry(move |entry: &DirEntry| { let path = entry.path(); let file_name = entry.file_name().to_str().unwrap_or(""); + // Team config files are exempt from the tracked-files check below: a + // freshly-created team.yml defines real ownership rules the moment it + // exists, not scratch work someone might never commit, so requiring + // `git add` before a new team is considered would silently produce + // "unowned file" errors for anything that should now belong to it + // (rubyatscale/code_ownership#149). Ordinary source files are left + // subject to the tracked-files check, preserving the original, + // intentional behavior of not forcing ownership onto untracked + // scratch files a developer has no plan to commit (#46/#74/#76). + let is_team_file = path + .strip_prefix(&base_path) + .is_ok_and(|relative_path| matches_globs(relative_path, &team_file_glob)); if let Some(tracked_files) = &tracked_files && let Some(ft) = entry.file_type() && ft.is_file() && !tracked_files.contains_key(path) + && !is_team_file { return false; } diff --git a/tests/untracked_new_file_test.rs b/tests/untracked_new_file_test.rs new file mode 100644 index 0000000..861a01b --- /dev/null +++ b/tests/untracked_new_file_test.rs @@ -0,0 +1,63 @@ +use assert_cmd::prelude::*; +use std::{error::Error, fs, path::Path, process::Command}; + +mod common; +use common::{git_add_all_files, setup_fixture_repo}; + +const FIXTURE: &str = "tests/fixtures/valid_project"; + +/// rubyatscale/code_ownership#149: adding a new team config must not require +/// `git add`-ing it first. The project walk excludes untracked files by +/// design (see tests/untracked_source_file_test.rs and codeowners-rs#46/#74/ +/// #76 - it lets a developer keep scratch files around without being forced +/// to assign them an owner), but that exclusion used to apply to team config +/// files too - so a brand-new, not-yet-staged team file was invisible to the +/// walk, and any *already-tracked* file that should newly become owned by +/// that team was reported as unowned instead (the team file disappears from +/// the walk, but the file it's supposed to own is still there and still +/// expected to have an owner - simply making both files new/untracked +/// together hides the bug, since then neither is visible and no mismatch is +/// ever observed). +/// +/// This mirrors the issue's exact repro shape: README.md already exists and +/// is already committed (as it would be from `rails new` or any prior +/// commit); only the *new team* and the *config edit adding README.md to +/// owned_globs* are freshly created and not yet staged. +#[test] +fn test_validate_succeeds_after_adding_new_untracked_team_for_an_existing_tracked_file() -> Result<(), Box> { + let temp_dir = setup_fixture_repo(Path::new(FIXTURE)); + let project_root = temp_dir.path(); + // README.md is part of the initial commit, so it's already tracked + // before the new team is ever introduced. + fs::write(project_root.join("README.md"), "# hello\n")?; + git_add_all_files(project_root); + Command::new("git") + .args(["commit", "-m", "initial"]) + .current_dir(project_root) + .output()?; + + // A brand-new team config, added later and never staged. + fs::write( + project_root.join("config/teams/docs.yml"), + "name: Docs\ngithub:\n team: '@DocsTeam'\nowned_globs:\n - README.md\n", + )?; + + let config_path = project_root.join("config/code_ownership.yml"); + let config = fs::read_to_string(&config_path)?; + let updated_config = config.replacen("owned_globs:\n", "owned_globs:\n - \"README.md\"\n", 1); + assert_ne!( + config, updated_config, + "expected to find and extend the owned_globs list in the fixture config" + ); + fs::write(&config_path, updated_config)?; + + Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg(project_root) + .arg("--no-cache") + .arg("generate-and-validate") + .assert() + .success(); + + Ok(()) +} diff --git a/tests/untracked_source_file_test.rs b/tests/untracked_source_file_test.rs new file mode 100644 index 0000000..6d19465 --- /dev/null +++ b/tests/untracked_source_file_test.rs @@ -0,0 +1,40 @@ +use assert_cmd::prelude::*; +use std::{error::Error, fs, path::Path, process::Command}; + +mod common; +use common::{git_add_all_files, setup_fixture_repo}; + +const FIXTURE: &str = "tests/fixtures/valid_project"; + +/// The fix for rubyatscale/code_ownership#149 (see untracked_new_file_test.rs) +/// only exempts team config files from the tracked-files check - it must NOT +/// broaden inclusion to untracked source files in general. That exclusion is +/// intentional (codeowners-rs#46/#74/#76: "don't fail when untracked git +/// files aren't in codeowners"), letting a developer keep a scratch file +/// around locally without `validate` forcing them to assign it an owner. +#[test] +fn test_untracked_source_file_is_still_ignored() -> Result<(), Box> { + let temp_dir = setup_fixture_repo(Path::new(FIXTURE)); + let project_root = temp_dir.path(); + git_add_all_files(project_root); + Command::new("git") + .args(["commit", "-m", "initial"]) + .current_dir(project_root) + .output()?; + + // A new source file matching owned_globs, but never staged. If it were + // included, this would fail with "missing ownership" since nothing + // claims it; if it's correctly ignored (matching pre-#149-fix behavior + // for non-team files), validation succeeds unchanged. + fs::write(project_root.join("ruby/app/models/scratch.rb"), "class Scratch; end\n")?; + + Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg(project_root) + .arg("--no-cache") + .arg("generate-and-validate") + .assert() + .success(); + + Ok(()) +}