diff --git a/src/runner.rs b/src/runner.rs index 433dea2..6f6c688 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::Command; use error_stack::{Result, ResultExt}; @@ -151,7 +151,7 @@ impl Runner { path }; - // Apply the same filtering logic as project_builder.rs:172 + // Mirror the filtering applied by ProjectBuilder when walking the project matches_globs(relative_path, &self.config.owned_globs) && !matches_globs(relative_path, &self.config.unowned_globs) }) .collect(); @@ -432,8 +432,7 @@ impl RunResult { } } -/// Helper function to check if a path matches any of the provided glob patterns. -/// This is used to filter files by owned_globs and unowned_globs configuration. +/// Returns true if `path` matches any of the provided glob patterns. fn matches_globs(path: &Path, globs: &[String]) -> bool { match path.to_str() { Some(s) => globs.iter().any(|glob| glob_match(glob, s)), diff --git a/tests/validate_files_test.rs b/tests/validate_files_test.rs index ca3bce9..541b5bf 100644 --- a/tests/validate_files_test.rs +++ b/tests/validate_files_test.rs @@ -145,37 +145,11 @@ fn test_validate_only_checks_codeowners_file() -> Result<(), Box> { #[test] fn test_validate_files_respects_owned_globs_with_excluded_extensions() -> Result<(), Box> { - // ============================================================================ - // THIS TEST CURRENTLY FAILS ON MAIN - IT DEMONSTRATES THE BUG - // ============================================================================ + // Validates that files not matching owned_globs are silently skipped when + // validate is called with an explicit file list. // - // BUG DESCRIPTION: - // When validate is called with a file list, it validates ALL provided files - // without checking if they match owned_globs configuration. - // - // CONFIGURATION: - // valid_project has: owned_globs = "**/*.{rb,tsx,erb}" - // Notice: .rbi files (Sorbet interface files) are NOT in this pattern - // - // EXPECTED BEHAVIOR: - // - .rbi files should be SILENTLY SKIPPED (don't match owned_globs) - // - Only .rb files should be validated against CODEOWNERS - // - Command should SUCCEED because all validated files are owned - // - // ACTUAL BEHAVIOR (BUG): - // - ALL files are validated (including .rbi files) - // - .rbi files are not in CODEOWNERS (correctly excluded during generate) - // - .rbi files are reported as "Unowned" - // - Command FAILS with validation errors - // - // ROOT CAUSE: - // src/runner.rs lines 112-143: validate_files() iterates all file_paths - // without applying the owned_globs/unowned_globs filter that - // project_builder.rs:172 uses when no files are specified - // - // FIX NEEDED: - // Filter file_paths by owned_globs and unowned_globs before validation - // ============================================================================ + // valid_project owned_globs: "{gems,config,javascript,ruby,components}/**/*.{rb,tsx,erb}" + // .rbi files (Sorbet interface files) do NOT match this pattern and should be filtered. // Setup: Create a temporary copy of valid_project fixture let fixture_root = std::path::Path::new("tests/fixtures/valid_project"); @@ -219,23 +193,8 @@ fn test_validate_files_respects_owned_globs_with_excluded_extensions() -> Result "CODEOWNERS should NOT contain .rbi files (they don't match owned_globs)" ); - // Step 2: Run validate with BOTH .rb and .rbi files in the list - // EXPECTED: .rbi files are silently skipped, only .rb files validated, succeeds - // ACTUAL (BUG): All files validated, .rbi reported as unowned, command fails - // - // ============================================================================ - // THIS ASSERTION WILL FAIL ON MAIN (proving the bug exists) - // ============================================================================ - // - // The command should succeed because: - // 1. .rbi files should be filtered out (don't match owned_globs) - // 2. Only .rb files should be validated - // 3. All .rb files are properly owned in CODEOWNERS - // - // But it currently fails because: - // 1. ALL files (including .rbi) are validated - // 2. .rbi files are not in CODEOWNERS - // 3. Validation error: "Unowned files detected: ruby/app/models/bank_account.rbi ..." + // Step 2: Run validate with BOTH .rb and .rbi files in the list. + // .rbi files should be silently filtered; only .rb files validated; command succeeds. Command::cargo_bin("codeowners")? .arg("--project-root") .arg(project_root)