From 33e3c256df5d61f0b966ec407dffc4bc50247b0f Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Thu, 12 Nov 2026 15:46:07 -0800 Subject: [PATCH] Give the validate_files tests an unowned file that exists Three tests in validate_files_test.rs named valid_project's ruby/app/unowned.rb, which doesn't exist: valid_project has to validate cleanly, so it has no unowned file. They passed only because a path that doesn't exist is reported as unowned, so they covered that rather than an unowned file, and would have started failing if nonexistent paths were ever skipped, as #126 proposed. They now write ruby/app/unowned.rb into a temp copy of valid_project and stage it. The invalid_project fixture, which #126 pointed them at, has an empty CODEOWNERS, so every file there is reported as unowned and the mixed test couldn't tell an owned file from an unowned one. Each test now checks the whole output, so the mixed test also checks that ruby/app/models/payroll.rb isn't reported. --- tests/validate_files_test.rs | 40 +++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/tests/validate_files_test.rs b/tests/validate_files_test.rs index 541b5bf..aca6a2c 100644 --- a/tests/validate_files_test.rs +++ b/tests/validate_files_test.rs @@ -6,6 +6,15 @@ mod common; use common::*; +// valid_project has to validate cleanly, so it has no unowned file. A path that doesn't exist is also reported as unowned, so +// naming one would test that instead. +fn valid_project_with_unowned_file() -> tempfile::TempDir { + let temp_dir = setup_fixture_repo(std::path::Path::new("tests/fixtures/valid_project")); + std::fs::write(temp_dir.path().join("ruby/app/unowned.rb"), "class Unowned; end\n").expect("failed to write unowned file"); + git_add_all_files(temp_dir.path()); + temp_dir +} + #[test] fn test_validate_with_owned_files() -> Result<(), Box> { run_codeowners( @@ -21,26 +30,22 @@ fn test_validate_with_owned_files() -> Result<(), Box> { #[test] fn test_validate_with_unowned_file() -> Result<(), Box> { - run_codeowners( - "valid_project", - &["validate", "ruby/app/unowned.rb"], - false, - OutputStream::Stdout, - predicate::str::contains("ruby/app/unowned.rb").and(predicate::str::contains("Unowned")), - )?; + let project = valid_project_with_unowned_file(); + + codeowners(project.path(), &["validate", "ruby/app/unowned.rb"])? + .failure() + .stdout(predicate::eq("Unowned files detected:\n ruby/app/unowned.rb\n")); Ok(()) } #[test] fn test_validate_with_mixed_files() -> Result<(), Box> { - 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")), - )?; + let project = valid_project_with_unowned_file(); + + codeowners(project.path(), &["validate", "ruby/app/models/payroll.rb", "ruby/app/unowned.rb"])? + .failure() + .stdout(predicate::eq("Unowned files detected:\n ruby/app/unowned.rb\n")); Ok(()) } @@ -79,10 +84,8 @@ fn test_generate_and_validate_with_owned_files() -> Result<(), Box> { #[test] fn test_generate_and_validate_with_unowned_file() -> Result<(), Box> { - let fixture_root = std::path::Path::new("tests/fixtures/valid_project"); - let temp_dir = setup_fixture_repo(fixture_root); + let temp_dir = valid_project_with_unowned_file(); let project_root = temp_dir.path(); - git_add_all_files(project_root); let codeowners_path = project_root.join("tmp/CODEOWNERS"); @@ -96,8 +99,7 @@ fn test_generate_and_validate_with_unowned_file() -> Result<(), Box> .arg("ruby/app/unowned.rb") .assert() .failure() - .stdout(predicate::str::contains("ruby/app/unowned.rb")) - .stdout(predicate::str::contains("Unowned")); + .stdout(predicate::eq("Unowned files detected:\n ruby/app/unowned.rb\n")); Ok(()) }