From 3ee3fb81e5bbad596e07aa9260e44b55579eb68d Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sat, 11 Apr 2026 19:38:42 +0100 Subject: [PATCH 01/11] test: audit pass - drop redundant unit tests, add last-failed + filter coverage Removes two non-snapshot fail-fast tests in basic.rs that duplicated the existing snapshot-based coverage, deletes trivial unit tests in function_kind.rs and a helper-formula test module in orchestration.rs that was testing a copy of the production logic, and adds an integration test covering the last-failed plus filterset workflow. --- crates/karva/tests/it/basic.rs | 69 +++++-------------- crates/karva/tests/it/last_failed.rs | 41 +++++++++++ .../src/function_kind.rs | 15 ---- crates/karva_runner/src/orchestration.rs | 47 ------------- 4 files changed, 57 insertions(+), 115 deletions(-) diff --git a/crates/karva/tests/it/basic.rs b/crates/karva/tests/it/basic.rs index d3ee6486..bcdd5a66 100644 --- a/crates/karva/tests/it/basic.rs +++ b/crates/karva/tests/it/basic.rs @@ -1574,70 +1574,33 @@ def test_third(): } #[test] -fn test_fail_fast() { - let context = TestContext::with_file( - "test.py", - r" -def test_1(): - assert True - -def test_2(): - assert False - -def test_3(): - assert True - ", - ); - - let output = context - .command_no_parallel() - .arg("--fail-fast") - .output() - .expect("failed to run"); - let stdout = String::from_utf8_lossy(&output.stdout); - assert!(!output.status.success()); - assert!( - stdout.contains("PASS") && stdout.contains("test::test_1"), - "first test should pass" - ); - assert!( - stdout.contains("FAIL") && stdout.contains("test::test_2"), - "second test should fail" - ); - assert!( - !stdout.contains("test::test_3"), - "third test should not run due to --fail-fast" - ); -} - -#[test] -fn test_fail_fast_across_modules() { +fn test_dry_run_nested_packages() { let context = TestContext::with_files([ ( - "test_a.py", + "tests/test_root.py", r" -def test_a_fail(): - assert False +def test_root(): pass ", ), ( - "test_b.py", + "tests/sub/test_nested.py", r" -def test_b_pass(): - assert True +def test_nested(): pass ", ), ]); - let output = context - .command_no_parallel() - .arg("--fail-fast") - .arg("-q") - .output() - .expect("failed to run"); - assert!(!output.status.success()); - let stdout = String::from_utf8_lossy(&output.stdout); - assert!(stdout.contains("failed"), "should report failure"); + assert_cmd_snapshot!(context.command().arg("--dry-run"), @r" + success: true + exit_code: 0 + ----- stdout ----- + tests.sub.test_nested::test_nested + tests.test_root::test_root + + 2 tests collected + + ----- stderr ----- + "); } #[test] diff --git a/crates/karva/tests/it/last_failed.rs b/crates/karva/tests/it/last_failed.rs index fabc92b3..39fbf8be 100644 --- a/crates/karva/tests/it/last_failed.rs +++ b/crates/karva/tests/it/last_failed.rs @@ -172,6 +172,47 @@ def test_fail_b(): assert False "); } +/// Combining `--last-failed` with `-E` should further narrow the rerun set +/// to only previously-failed tests that also match the filter expression. +#[test] +fn last_failed_with_filter_narrows_rerun_set() { + let context = TestContext::with_files([ + ( + "test_a.py", + " +def test_pass_a(): pass +def test_fail_a(): assert False + ", + ), + ( + "test_b.py", + " +def test_pass_b(): pass +def test_fail_b(): assert False + ", + ), + ]); + + context.command_no_parallel().output().unwrap(); + + assert_cmd_snapshot!( + context + .command_no_parallel() + .arg("--last-failed") + .args(["-E", "test(~fail_a)"]) + .arg("-q"), + @" + success: false + exit_code: 1 + ----- stdout ----- + ──────────── + Summary [TIME] 2 tests run: 0 passed, 1 failed, 1 skipped + + ----- stderr ----- + ", + ); +} + #[test] fn last_failed_fix_then_rerun() { let context = TestContext::with_file( diff --git a/crates/karva_python_semantic/src/function_kind.rs b/crates/karva_python_semantic/src/function_kind.rs index 2cb6faf5..6d8c3ccd 100644 --- a/crates/karva_python_semantic/src/function_kind.rs +++ b/crates/karva_python_semantic/src/function_kind.rs @@ -16,18 +16,3 @@ impl FunctionKind { } } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_capitalised_test() { - assert_eq!(FunctionKind::Test.capitalised(), "Test"); - } - - #[test] - fn test_capitalised_fixture() { - assert_eq!(FunctionKind::Fixture.capitalised(), "Fixture"); - } -} diff --git a/crates/karva_runner/src/orchestration.rs b/crates/karva_runner/src/orchestration.rs index 5032e5e5..9a3df46c 100644 --- a/crates/karva_runner/src/orchestration.rs +++ b/crates/karva_runner/src/orchestration.rs @@ -439,50 +439,3 @@ fn inner_cli_args(settings: &ProjectSettings, args: &SubTestCommand) -> Vec usize { - let max_useful = total_tests.div_ceil(MIN_TESTS_PER_WORKER).max(1); - num_workers.min(max_useful) - } - - #[test] - fn test_workers_capped_for_small_test_count() { - // 9 tests / 5 per worker = ceil(1.8) = 2 workers - assert_eq!(effective_workers(8, 9), 2); - } - - #[test] - fn test_workers_capped_for_medium_test_count() { - // 25 tests / 5 per worker = ceil(5) = 5 workers - assert_eq!(effective_workers(8, 25), 5); - } - - #[test] - fn test_workers_unchanged_when_test_count_is_high() { - // 100 tests / 5 per worker = ceil(20) = 20, but only 8 workers requested - assert_eq!(effective_workers(8, 100), 8); - } - - #[test] - fn test_at_least_one_worker_with_zero_tests() { - // 0 tests should still yield at least 1 worker - assert_eq!(effective_workers(8, 0), 1); - } - - #[test] - fn test_workers_capped_for_very_few_tests() { - // 3 tests / 5 per worker = ceil(0.6) = 1 worker - assert_eq!(effective_workers(8, 3), 1); - } - - #[test] - fn test_workers_exact_multiple() { - // 40 tests / 5 per worker = 8 workers exactly - assert_eq!(effective_workers(8, 40), 8); - } -} From da36b5f16060bd1d3361535daebe5bb4745859b0 Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sat, 11 Apr 2026 19:48:47 +0100 Subject: [PATCH 02/11] test: add unit tests for metadata, cache, project, and logging data paths --- crates/karva_cache/src/cache.rs | 192 +++++++++++++++++++++++++ crates/karva_logging/src/time.rs | 62 ++++++++ crates/karva_metadata/src/options.rs | 191 ++++++++++++++++++++++++ crates/karva_project/src/path/utils.rs | 47 ++++++ 4 files changed, 492 insertions(+) diff --git a/crates/karva_cache/src/cache.rs b/crates/karva_cache/src/cache.rs index 44dacabd..9561d36b 100644 --- a/crates/karva_cache/src/cache.rs +++ b/crates/karva_cache/src/cache.rs @@ -405,4 +405,196 @@ mod tests { assert_eq!(results.stats.total(), 0); assert!(results.diagnostics.is_empty()); } + + #[test] + fn write_last_failed_roundtrips_with_read() { + let tmp = tempfile::tempdir().unwrap(); + let cache_dir = Utf8PathBuf::try_from(tmp.path().to_path_buf()).unwrap(); + + let failed = vec!["mod::test_a".to_string(), "mod::test_b".to_string()]; + write_last_failed(&cache_dir, &failed).unwrap(); + + let read = read_last_failed(&cache_dir).unwrap(); + assert_eq!(read, failed); + } + + #[test] + fn read_last_failed_missing_file_returns_empty() { + let tmp = tempfile::tempdir().unwrap(); + let cache_dir = Utf8PathBuf::try_from(tmp.path().to_path_buf()).unwrap(); + + let read = read_last_failed(&cache_dir).unwrap(); + assert!(read.is_empty()); + } + + #[test] + fn write_last_failed_overwrites_previous_list() { + let tmp = tempfile::tempdir().unwrap(); + let cache_dir = Utf8PathBuf::try_from(tmp.path().to_path_buf()).unwrap(); + + write_last_failed(&cache_dir, &["old".to_string()]).unwrap(); + write_last_failed(&cache_dir, &["new".to_string()]).unwrap(); + + let read = read_last_failed(&cache_dir).unwrap(); + assert_eq!(read, vec!["new".to_string()]); + } + + #[test] + fn write_last_failed_creates_cache_dir() { + let tmp = tempfile::tempdir().unwrap(); + let cache_dir = Utf8PathBuf::try_from(tmp.path().join("nested").join("cache")).unwrap(); + assert!(!cache_dir.exists()); + + write_last_failed(&cache_dir, &["x".to_string()]).unwrap(); + + assert!(cache_dir.exists()); + assert_eq!(read_last_failed(&cache_dir).unwrap(), vec!["x".to_string()]); + } + + #[test] + fn read_last_failed_empty_json_list_parses() { + let tmp = tempfile::tempdir().unwrap(); + let cache_dir = Utf8PathBuf::try_from(tmp.path().to_path_buf()).unwrap(); + + write_last_failed(&cache_dir, &[]).unwrap(); + assert!(read_last_failed(&cache_dir).unwrap().is_empty()); + } + + #[test] + fn prune_cache_keeps_most_recent_run_only() { + let tmp = tempfile::tempdir().unwrap(); + let cache_dir = Utf8PathBuf::try_from(tmp.path().to_path_buf()).unwrap(); + + for ts in ["run-100", "run-200", "run-300"] { + fs::create_dir_all(tmp.path().join(ts)).unwrap(); + } + + let result = prune_cache(&cache_dir).unwrap(); + assert_eq!(result.removed.len(), 2); + assert!(result.removed.contains(&"run-100".to_string())); + assert!(result.removed.contains(&"run-200".to_string())); + assert!(cache_dir.join("run-300").exists()); + assert!(!cache_dir.join("run-100").exists()); + assert!(!cache_dir.join("run-200").exists()); + } + + #[test] + fn prune_cache_handles_missing_dir() { + let tmp = tempfile::tempdir().unwrap(); + let cache_dir = Utf8PathBuf::try_from(tmp.path().join("nope")).unwrap(); + + let result = prune_cache(&cache_dir).unwrap(); + assert!(result.removed.is_empty()); + } + + #[test] + fn prune_cache_ignores_non_run_directories() { + let tmp = tempfile::tempdir().unwrap(); + let cache_dir = Utf8PathBuf::try_from(tmp.path().to_path_buf()).unwrap(); + + fs::create_dir_all(tmp.path().join("run-10")).unwrap(); + fs::create_dir_all(tmp.path().join("run-20")).unwrap(); + fs::create_dir_all(tmp.path().join("not-a-run")).unwrap(); + fs::write(tmp.path().join("last-failed.json"), "[]").unwrap(); + + prune_cache(&cache_dir).unwrap(); + + assert!(cache_dir.join("not-a-run").exists()); + assert!(cache_dir.join("last-failed.json").exists()); + assert!(cache_dir.join("run-20").exists()); + assert!(!cache_dir.join("run-10").exists()); + } + + #[test] + fn prune_cache_keeps_newest_even_when_names_are_lexicographically_out_of_order() { + // `run-9` lexicographically sorts AFTER `run-100` but numerically it is + // older; pruning must use the numeric `sort_key` or it would delete the + // newest run directory. This test guards against a regression to naive + // string sorting. + let tmp = tempfile::tempdir().unwrap(); + let cache_dir = Utf8PathBuf::try_from(tmp.path().to_path_buf()).unwrap(); + + fs::create_dir_all(tmp.path().join("run-9")).unwrap(); + fs::create_dir_all(tmp.path().join("run-100")).unwrap(); + + prune_cache(&cache_dir).unwrap(); + + assert!(cache_dir.join("run-100").exists()); + assert!(!cache_dir.join("run-9").exists()); + } + + #[test] + fn clean_cache_removes_dir_and_returns_true() { + let tmp = tempfile::tempdir().unwrap(); + let cache_dir = Utf8PathBuf::try_from(tmp.path().to_path_buf()).unwrap(); + fs::create_dir_all(tmp.path().join("run-1")).unwrap(); + + assert!(clean_cache(&cache_dir).unwrap()); + assert!(!cache_dir.exists()); + } + + #[test] + fn clean_cache_missing_dir_returns_false() { + let tmp = tempfile::tempdir().unwrap(); + let cache_dir = Utf8PathBuf::try_from(tmp.path().join("nope")).unwrap(); + assert!(!clean_cache(&cache_dir).unwrap()); + } + + #[test] + fn aggregate_results_merges_failed_tests_and_durations_across_workers() { + let tmp = tempfile::tempdir().unwrap(); + let cache_dir = Utf8PathBuf::try_from(tmp.path().to_path_buf()).unwrap(); + let run_hash = RunHash::from_existing("run-700"); + + let run_dir = tmp.path().join("run-700"); + let worker0 = run_dir.join("worker-0"); + let worker1 = run_dir.join("worker-1"); + fs::create_dir_all(&worker0).unwrap(); + fs::create_dir_all(&worker1).unwrap(); + + fs::write(worker0.join(FAILED_TESTS_FILE), r#"["mod::test_a"]"#).unwrap(); + fs::write(worker1.join(FAILED_TESTS_FILE), r#"["mod::test_b"]"#).unwrap(); + + let mut d0 = HashMap::new(); + d0.insert("mod::test_a".to_string(), Duration::from_millis(10)); + let mut d1 = HashMap::new(); + d1.insert("mod::test_b".to_string(), Duration::from_millis(20)); + fs::write( + worker0.join(DURATIONS_FILE), + serde_json::to_string(&d0).unwrap(), + ) + .unwrap(); + fs::write( + worker1.join(DURATIONS_FILE), + serde_json::to_string(&d1).unwrap(), + ) + .unwrap(); + + let cache = Cache::new(&cache_dir, &run_hash); + let results = cache.aggregate_results().unwrap(); + + let mut failed = results.failed_tests.clone(); + failed.sort(); + assert_eq!( + failed, + vec!["mod::test_a".to_string(), "mod::test_b".to_string()] + ); + assert_eq!(results.durations.len(), 2); + assert_eq!( + results.durations.get("mod::test_a"), + Some(&Duration::from_millis(10)) + ); + } + + #[test] + fn fail_fast_signal_round_trip() { + let tmp = tempfile::tempdir().unwrap(); + let cache_dir = Utf8PathBuf::try_from(tmp.path().to_path_buf()).unwrap(); + let run_hash = RunHash::from_existing("run-800"); + let cache = Cache::new(&cache_dir, &run_hash); + + assert!(!cache.has_fail_fast_signal()); + cache.write_fail_fast_signal().unwrap(); + assert!(cache.has_fail_fast_signal()); + } } diff --git a/crates/karva_logging/src/time.rs b/crates/karva_logging/src/time.rs index 317117a4..ae8b056d 100644 --- a/crates/karva_logging/src/time.rs +++ b/crates/karva_logging/src/time.rs @@ -12,3 +12,65 @@ pub fn format_duration(duration: Duration) -> String { pub fn format_duration_bracketed(duration: Duration) -> String { format!("[{:>8.3}s]", duration.as_secs_f64()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn format_duration_zero_is_zero_ms() { + assert_eq!(format_duration(Duration::ZERO), "0ms"); + } + + #[test] + fn format_duration_sub_millisecond_truncates_to_zero_ms() { + assert_eq!(format_duration(Duration::from_micros(500)), "0ms"); + assert_eq!(format_duration(Duration::from_nanos(1)), "0ms"); + } + + #[test] + fn format_duration_exactly_one_ms() { + assert_eq!(format_duration(Duration::from_millis(1)), "1ms"); + } + + #[test] + fn format_duration_sub_two_seconds_uses_milliseconds() { + // The cutoff is `< 2s`, so anything under two full seconds stays in ms, + // including the exact one-second boundary and values like 1999 ms. + assert_eq!(format_duration(Duration::from_millis(1000)), "1000ms"); + assert_eq!(format_duration(Duration::from_millis(1999)), "1999ms"); + } + + #[test] + fn format_duration_two_seconds_switches_to_seconds() { + assert_eq!(format_duration(Duration::from_secs(2)), "2.00s"); + } + + #[test] + fn format_duration_rounds_to_two_decimals() { + assert_eq!(format_duration(Duration::from_millis(2346)), "2.35s"); + assert_eq!(format_duration(Duration::from_millis(2344)), "2.34s"); + } + + #[test] + fn format_duration_minutes_stay_in_seconds() { + assert_eq!(format_duration(Duration::from_secs(125)), "125.00s"); + } + + #[test] + fn format_duration_bracketed_pads_to_width_and_three_decimals() { + assert_eq!(format_duration_bracketed(Duration::ZERO), "[ 0.000s]"); + assert_eq!( + format_duration_bracketed(Duration::from_millis(15)), + "[ 0.015s]" + ); + } + + #[test] + fn format_duration_bracketed_handles_large_values() { + assert_eq!( + format_duration_bracketed(Duration::from_secs(12345)), + "[12345.000s]" + ); + } +} diff --git a/crates/karva_metadata/src/options.rs b/crates/karva_metadata/src/options.rs index 82701e24..820472c0 100644 --- a/crates/karva_metadata/src/options.rs +++ b/crates/karva_metadata/src/options.rs @@ -297,6 +297,197 @@ impl Combine for OutputFormat { } } +#[cfg(test)] +mod tests { + use std::num::NonZeroU32; + + use karva_combine::Combine; + + use super::*; + + #[test] + fn to_settings_defaults_when_empty() { + let settings = TestOptions::default().to_settings(); + assert_eq!(settings.test_function_prefix, "test"); + assert_eq!(settings.max_fail, MaxFail::unlimited()); + assert!(!settings.try_import_fixtures); + assert_eq!(settings.retry, 0); + } + + #[test] + fn to_settings_fail_fast_true_becomes_max_fail_one() { + let options = TestOptions { + fail_fast: Some(true), + ..TestOptions::default() + }; + assert_eq!(options.to_settings().max_fail, MaxFail::from_count(1)); + } + + #[test] + fn to_settings_fail_fast_false_is_unlimited() { + let options = TestOptions { + fail_fast: Some(false), + ..TestOptions::default() + }; + assert_eq!(options.to_settings().max_fail, MaxFail::unlimited()); + } + + #[test] + fn to_settings_max_fail_takes_precedence_over_fail_fast() { + let options = TestOptions { + fail_fast: Some(true), + max_fail: Some(MaxFail::from(NonZeroU32::new(5).expect("non-zero"))), + ..TestOptions::default() + }; + assert_eq!(options.to_settings().max_fail, MaxFail::from_count(5)); + } + + #[test] + fn from_toml_str_parses_nested_sections() { + let toml = r#" +[test] +test-function-prefix = "check" +max-fail = 3 +retry = 2 + +[terminal] +output-format = "concise" +show-python-output = true + +[src] +respect-ignore-files = false +include = ["tests", "more"] +"#; + let options = Options::from_toml_str(toml).expect("parse"); + let settings = options.to_settings(); + assert_eq!(settings.test().test_function_prefix, "check"); + assert_eq!(settings.test().max_fail, MaxFail::from_count(3)); + assert_eq!(settings.test().retry, 2); + assert_eq!(settings.terminal().output_format, OutputFormat::Concise); + assert!(settings.terminal().show_python_output); + assert!(!settings.src().respect_ignore_files); + assert_eq!(settings.src().include_paths, vec!["tests", "more"]); + } + + #[test] + fn from_toml_str_rejects_unknown_key() { + let toml = r" +[test] +fail-fast = true +nonsense = 42 +"; + let err = Options::from_toml_str(toml).expect_err("unknown field"); + assert!(matches!(err, KarvaTomlError::TomlSyntax(_))); + } + + #[test] + fn from_toml_str_rejects_unknown_top_level_section() { + let toml = r" +[bogus] +foo = 1 +"; + assert!(matches!( + Options::from_toml_str(toml), + Err(KarvaTomlError::TomlSyntax(_)) + )); + } + + #[test] + fn from_toml_str_empty_is_default() { + let options = Options::from_toml_str("").expect("parse"); + assert_eq!(options, Options::default()); + } + + #[test] + fn from_toml_str_rejects_max_fail_zero() { + // MaxFail wraps NonZeroU32 so the raw integer 0 must be rejected by the + // deserializer rather than silently producing `unlimited`. + let toml = r" +[test] +max-fail = 0 +"; + assert!(matches!( + Options::from_toml_str(toml), + Err(KarvaTomlError::TomlSyntax(_)) + )); + } + + #[test] + fn combine_prefers_self_for_scalars() { + let cli = TestOptions { + test_function_prefix: Some("cli_prefix".to_string()), + retry: Some(5), + ..TestOptions::default() + }; + let file = TestOptions { + test_function_prefix: Some("file_prefix".to_string()), + retry: Some(1), + try_import_fixtures: Some(true), + ..TestOptions::default() + }; + let merged = cli.combine(file); + assert_eq!(merged.test_function_prefix.as_deref(), Some("cli_prefix")); + assert_eq!(merged.retry, Some(5)); + assert_eq!(merged.try_import_fixtures, Some(true)); + } + + #[test] + fn combine_fills_missing_fields_from_other() { + let cli = TestOptions::default(); + let file = TestOptions { + test_function_prefix: Some("from_file".to_string()), + fail_fast: Some(true), + retry: Some(3), + ..TestOptions::default() + }; + let merged = cli.combine(file); + assert_eq!(merged.test_function_prefix.as_deref(), Some("from_file")); + assert_eq!(merged.fail_fast, Some(true)); + assert_eq!(merged.retry, Some(3)); + } + + #[test] + fn combine_merges_include_paths_with_cli_taking_precedence() { + let cli = SrcOptions { + include: Some(vec!["cli_only".to_string()]), + ..SrcOptions::default() + }; + let file = SrcOptions { + include: Some(vec!["file_only".to_string()]), + respect_ignore_files: Some(false), + }; + let merged = cli.combine(file); + // Vec combine appends `self` after `other`, so CLI entries take precedence at the tail. + let include = merged.include.expect("include set"); + assert_eq!(include, vec!["file_only", "cli_only"]); + assert_eq!(merged.respect_ignore_files, Some(false)); + } + + #[test] + fn project_overrides_apply_cli_over_file() { + let cli_options = Options { + test: Some(TestOptions { + test_function_prefix: Some("cli".to_string()), + ..TestOptions::default() + }), + ..Options::default() + }; + let file_options = Options { + test: Some(TestOptions { + test_function_prefix: Some("file".to_string()), + retry: Some(2), + ..TestOptions::default() + }), + ..Options::default() + }; + let overrides = ProjectOptionsOverrides::new(None, cli_options); + let merged = overrides.apply_to(file_options); + let test = merged.test.expect("test section set"); + assert_eq!(test.test_function_prefix.as_deref(), Some("cli")); + assert_eq!(test.retry, Some(2)); + } +} + #[derive(Debug, Default, PartialEq, Eq, Clone)] pub struct ProjectOptionsOverrides { pub config_file_override: Option, diff --git a/crates/karva_project/src/path/utils.rs b/crates/karva_project/src/path/utils.rs index 6fffcd9f..554f42bf 100644 --- a/crates/karva_project/src/path/utils.rs +++ b/crates/karva_project/src/path/utils.rs @@ -67,4 +67,51 @@ mod tests { let result = absolute("foo/../bar/./baz", "/cwd"); assert_eq!(result, Utf8PathBuf::from("/cwd/bar/baz")); } + + #[test] + fn empty_relative_path_returns_cwd() { + let result = absolute("", "/home/user"); + assert_eq!(result, Utf8PathBuf::from("/home/user")); + } + + #[test] + fn leading_parent_pops_cwd() { + let result = absolute("../../other", "/home/user"); + assert_eq!(result, Utf8PathBuf::from("/other")); + } + + #[test] + fn parent_past_root_stays_at_root() { + // `Utf8PathBuf::pop` on `/` returns false, so extra `..` components + // must not escape the filesystem root. + let result = absolute("../..", "/"); + assert_eq!(result, Utf8PathBuf::from("/")); + } + + #[test] + fn unicode_path_components_are_preserved() { + let result = absolute("カルヴァ/tests", "/home/ユーザー"); + assert_eq!(result, Utf8PathBuf::from("/home/ユーザー/カルヴァ/tests")); + } + + #[test] + fn path_with_spaces_is_preserved() { + let result = absolute("my tests/file.py", "/home/my user"); + assert_eq!(result, Utf8PathBuf::from("/home/my user/my tests/file.py")); + } + + #[test] + fn trailing_slash_on_relative_input_is_normalized() { + // `camino` components strip trailing slashes, so the result should + // match the same input without one. + let with = absolute("foo/bar/", "/cwd"); + let without = absolute("foo/bar", "/cwd"); + assert_eq!(with, without); + } + + #[test] + fn dot_only_path_is_cwd() { + let result = absolute(".", "/home/user"); + assert_eq!(result, Utf8PathBuf::from("/home/user")); + } } From 0f265b8be9f2d1b1ad16196524339e61834fd54e Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sat, 11 Apr 2026 19:53:45 +0100 Subject: [PATCH 03/11] test: expand integration coverage for CLI flags and orchestration Adds 32 new integration tests covering CLI flag combinations and orchestration behaviours that were either untested or only had a happy path. The additions are spread across basic, last_failed, configuration, and a new discovery/edge_cases module. --- crates/karva/tests/it/basic.rs | 464 ++++++++++++++++++ crates/karva/tests/it/configuration/mod.rs | 124 +++++ crates/karva/tests/it/discovery/edge_cases.rs | 184 +++++++ crates/karva/tests/it/discovery/mod.rs | 1 + crates/karva/tests/it/last_failed.rs | 155 +++++- 5 files changed, 907 insertions(+), 21 deletions(-) create mode 100644 crates/karva/tests/it/discovery/edge_cases.rs diff --git a/crates/karva/tests/it/basic.rs b/crates/karva/tests/it/basic.rs index bcdd5a66..c0288a39 100644 --- a/crates/karva/tests/it/basic.rs +++ b/crates/karva/tests/it/basic.rs @@ -1797,3 +1797,467 @@ def test_fail(): assert False ----- stderr ----- "); } + +#[test] +fn test_color_never_strips_ansi() { + let context = TestContext::with_file("test.py", "def test_1(): pass"); + + assert_cmd_snapshot!(context.command_no_parallel().args(["--color", "never"]), @" + success: true + exit_code: 0 + ----- stdout ----- + Starting 1 test across 1 worker + PASS [TIME] test::test_1 + + ──────────── + Summary [TIME] 1 test run: 1 passed, 0 skipped + + ----- stderr ----- + "); +} + +#[test] +fn test_color_invalid_value() { + let context = TestContext::with_file("test.py", "def test_1(): pass"); + + assert_cmd_snapshot!(context.command().args(["--color", "rainbow"]), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: invalid value 'rainbow' for '--color ' + [possible values: auto, always, never] + + For more information, try '--help'. + "); +} + +/// `--no-cache` disables reading duration history but the run should still succeed. +#[test] +fn test_no_cache_flag() { + let context = TestContext::with_file( + "test.py", + r" +def test_1(): pass +def test_2(): pass +", + ); + + assert_cmd_snapshot!(context.command_no_parallel().arg("--no-cache"), @" + success: true + exit_code: 0 + ----- stdout ----- + Starting 2 tests across 1 worker + PASS [TIME] test::test_1 + PASS [TIME] test::test_2 + + ──────────── + Summary [TIME] 2 tests run: 2 passed, 0 skipped + + ----- stderr ----- + "); +} + +#[test] +fn test_no_progress_hides_per_test_lines() { + let context = TestContext::with_file( + "test.py", + r" +def test_1(): pass +def test_2(): pass +def test_3(): pass +", + ); + + assert_cmd_snapshot!(context.command_no_parallel().arg("--no-progress"), @" + success: true + exit_code: 0 + ----- stdout ----- + + ──────────── + Summary [TIME] 3 tests run: 3 passed, 0 skipped + + ----- stderr ----- + "); +} + +/// `--no-progress` still emits diagnostics for failing tests. +#[test] +fn test_no_progress_with_failure_shows_diagnostics() { + let context = TestContext::with_file( + "test.py", + r" +def test_1(): pass +def test_2(): assert False +", + ); + + assert_cmd_snapshot!(context.command_no_parallel().arg("--no-progress"), @" + success: false + exit_code: 1 + ----- stdout ----- + + diagnostics: + + error[test-failure]: Test `test_2` failed + --> test.py:3:5 + | + 2 | def test_1(): pass + 3 | def test_2(): assert False + | ^^^^^^ + | + info: Test failed here + --> test.py:3:1 + | + 2 | def test_1(): pass + 3 | def test_2(): assert False + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + + ──────────── + Summary [TIME] 2 tests run: 1 passed, 1 failed, 0 skipped + + ----- stderr ----- + "); +} + +/// `--retry 0` is a no-op — failing tests still fail and are not re-run. +#[test] +fn test_retry_zero_is_noop() { + let context = TestContext::with_file( + "test.py", + r" +def test_fail(): assert False +", + ); + + assert_cmd_snapshot!(context.command_no_parallel().arg("--retry").arg("0"), @" + success: false + exit_code: 1 + ----- stdout ----- + Starting 1 test across 1 worker + FAIL [TIME] test::test_fail + + diagnostics: + + error[test-failure]: Test `test_fail` failed + --> test.py:2:5 + | + 2 | def test_fail(): assert False + | ^^^^^^^^^ + | + info: Test failed here + --> test.py:2:1 + | + 2 | def test_fail(): assert False + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + + ──────────── + Summary [TIME] 1 test run: 0 passed, 1 failed, 0 skipped + + ----- stderr ----- + "); +} + +/// A test that always fails exhausts retries and ends up reported as failed. +#[test] +fn test_retry_exhausts_on_always_failing_test() { + let context = TestContext::with_file( + "test.py", + r" +def test_always_fails(): assert False +", + ); + + assert_cmd_snapshot!(context.command_no_parallel().arg("--retry").arg("2"), @" + success: false + exit_code: 1 + ----- stdout ----- + Starting 1 test across 1 worker + FAIL [TIME] test::test_always_fails + + diagnostics: + + error[test-failure]: Test `test_always_fails` failed + --> test.py:2:5 + | + 2 | def test_always_fails(): assert False + | ^^^^^^^^^^^^^^^^^ + | + info: Test failed here + --> test.py:2:1 + | + 2 | def test_always_fails(): assert False + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + + ──────────── + Summary [TIME] 1 test run: 0 passed, 1 failed, 0 skipped + + ----- stderr ----- + "); +} + +/// `--max-fail` must reject zero because the underlying type is `NonZeroU32`. +#[test] +fn test_max_fail_zero_is_rejected() { + let context = TestContext::with_file("test.py", "def test_1(): pass"); + + assert_cmd_snapshot!(context.command().args(["--max-fail", "0"]), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: invalid value '0' for '--max-fail ': number would be zero for non-zero type + + For more information, try '--help'. + "); +} + +/// `--num-workers` followed by a non-numeric value should trigger clap's parser. +#[test] +fn test_num_workers_invalid_value() { + let context = TestContext::with_file("test.py", "def test_1(): pass"); + + assert_cmd_snapshot!(context.command().args(["--num-workers", "abc"]), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: invalid value 'abc' for '--num-workers ': invalid digit found in string + + For more information, try '--help'. + "); +} + +/// `--num-workers 1` behaves like `--no-parallel`: one worker handles every test. +#[test] +fn test_num_workers_one_matches_no_parallel() { + let context = TestContext::with_file( + "test.py", + r" +def test_1(): pass +def test_2(): pass +", + ); + + assert_cmd_snapshot!(context.command().args(["--num-workers", "1"]), @" + success: true + exit_code: 0 + ----- stdout ----- + Starting 2 tests across 1 worker + PASS [TIME] test::test_1 + PASS [TIME] test::test_2 + + ──────────── + Summary [TIME] 2 tests run: 2 passed, 0 skipped + + ----- stderr ----- + "); +} + +/// `--durations` requires a numeric argument. +#[test] +fn test_durations_invalid_value() { + let context = TestContext::with_file("test.py", "def test_1(): pass"); + + assert_cmd_snapshot!(context.command().args(["--durations", "abc"]), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: invalid value 'abc' for '--durations ': invalid digit found in string + + For more information, try '--help'. + "); +} + +/// `--dry-run` with `--num-workers` still only collects — it should not spawn workers. +#[test] +fn test_dry_run_with_num_workers_does_not_spawn() { + let context = TestContext::with_file( + "test.py", + r" +def test_1(): pass +def test_2(): pass +", + ); + + assert_cmd_snapshot!(context.command().args(["--dry-run", "--num-workers", "4"]), @r" + success: true + exit_code: 0 + ----- stdout ----- + test::test_1 + test::test_2 + + 2 tests collected + + ----- stderr ----- + "); +} + +/// When `--fail-fast` and `--no-fail-fast` are mixed, clap's `overrides_with` +/// wires them so that whichever flag appears last wins. +#[test] +fn test_no_fail_fast_after_fail_fast_wins() { + let context = TestContext::with_file( + "test.py", + r" +def test_1(): assert False +def test_2(): assert False +def test_3(): pass +", + ); + + assert_cmd_snapshot!( + context + .command_no_parallel() + .args(["--fail-fast", "--no-fail-fast", "-q"]), + @" + success: false + exit_code: 1 + ----- stdout ----- + ──────────── + Summary [TIME] 3 tests run: 1 passed, 2 failed, 0 skipped + + ----- stderr ----- + " + ); +} + +#[test] +fn test_fail_fast_after_no_fail_fast_wins() { + let context = TestContext::with_file( + "test.py", + r" +def test_1(): assert False +def test_2(): assert False +def test_3(): pass +", + ); + + assert_cmd_snapshot!( + context + .command_no_parallel() + .args(["--no-fail-fast", "--fail-fast", "-q"]), + @" + success: false + exit_code: 1 + ----- stdout ----- + ──────────── + Summary [TIME] 1 test run: 0 passed, 1 failed, 0 skipped + + ----- stderr ----- + " + ); +} + +/// `--max-fail` wins over `--no-fail-fast` regardless of order. +#[test] +fn test_max_fail_beats_no_fail_fast() { + let context = TestContext::with_file( + "test.py", + r" +def test_1(): assert False +def test_2(): assert False +def test_3(): assert False +", + ); + + assert_cmd_snapshot!( + context + .command_no_parallel() + .args(["--no-fail-fast", "--max-fail=2", "-q"]), + @" + success: false + exit_code: 1 + ----- stdout ----- + ──────────── + Summary [TIME] 2 tests run: 0 passed, 2 failed, 0 skipped + + ----- stderr ----- + " + ); +} + +/// `karva test nonexistent.py` should exit with code 2 and an error message +/// that points at the missing path. +#[test] +fn test_nonexistent_path_exits_nonzero() { + let context = TestContext::new(); + + assert_cmd_snapshot!(context.command().arg("missing.py"), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + Karva failed + Cause: path `/missing.py` could not be found + "); +} + +/// `karva` with no subcommand should print help and exit successfully. +#[test] +fn test_no_subcommand_prints_help() { + let context = TestContext::new(); + + let output = context + .karva_command_in(context.root()) + .output() + .expect("failed to run karva with no subcommand"); + + // No subcommand is a clap error; exit code is 2 and help goes to stderr. + assert_eq!(output.status.code(), Some(2)); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("Usage: karva "), + "expected usage line in stderr, got: {stderr}" + ); +} + +/// `karva testx` (typo of `test`) should suggest the closest subcommand. +#[test] +fn test_unknown_subcommand_suggests_correction() { + let context = TestContext::new(); + + let mut command = context.karva_command_in(context.root()); + command.arg("testx"); + + assert_cmd_snapshot!(command, @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: unrecognized subcommand 'testx' + + tip: a similar subcommand exists: 'test' + + Usage: karva + + For more information, try '--help'. + "); +} + +/// `--test-prefix` requires a value. +#[test] +fn test_test_prefix_requires_value() { + let context = TestContext::with_file("test.py", "def test_1(): pass"); + + assert_cmd_snapshot!(context.command().arg("--test-prefix"), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: a value is required for '--test-prefix ' but none was supplied + + For more information, try '--help'. + "); +} diff --git a/crates/karva/tests/it/configuration/mod.rs b/crates/karva/tests/it/configuration/mod.rs index 0a9f7b68..7b0d5398 100644 --- a/crates/karva/tests/it/configuration/mod.rs +++ b/crates/karva/tests/it/configuration/mod.rs @@ -1379,6 +1379,130 @@ def test_should_not_run(): pass "); } +/// The `KARVA_CONFIG_FILE` environment variable is equivalent to passing +/// `--config-file` on the command line. +#[test] +fn test_config_file_env_var() { + let context = TestContext::with_files([ + ( + "custom.toml", + r#" +[test] +test-function-prefix = "spec" +"#, + ), + ( + "test.py", + r" +def spec_example(): pass +def test_should_not_run(): pass +", + ), + ]); + + assert_cmd_snapshot!( + context.command().env("KARVA_CONFIG_FILE", "custom.toml"), + @" + success: true + exit_code: 0 + ----- stdout ----- + Starting 1 test across 1 worker + PASS [TIME] test::spec_example + + ──────────── + Summary [TIME] 1 test run: 1 passed, 0 skipped + + ----- stderr ----- + " + ); +} + +/// An explicit `--config-file` takes precedence over the `KARVA_CONFIG_FILE` +/// environment variable. +#[test] +fn test_cli_config_file_overrides_env() { + let context = TestContext::with_files([ + ( + "env.toml", + r#" +[test] +test-function-prefix = "env" +"#, + ), + ( + "cli.toml", + r#" +[test] +test-function-prefix = "cli" +"#, + ), + ( + "test.py", + r" +def env_should_not_run(): pass +def cli_should_run(): pass +", + ), + ]); + + assert_cmd_snapshot!( + context + .command() + .env("KARVA_CONFIG_FILE", "env.toml") + .args(["--config-file", "cli.toml"]), + @" + success: true + exit_code: 0 + ----- stdout ----- + Starting 1 test across 1 worker + PASS [TIME] test::cli_should_run + + ──────────── + Summary [TIME] 1 test run: 1 passed, 0 skipped + + ----- stderr ----- + " + ); +} + +/// `karva.toml` discovered from a parent directory should still apply when +/// karva is invoked from a subdirectory. +#[test] +fn test_karva_toml_discovered_from_subdirectory() { + let context = TestContext::with_files([ + ( + "karva.toml", + r#" +[test] +test-function-prefix = "check" +"#, + ), + ( + "tests/test_a.py", + r" +def check_found(): pass +def test_should_not_run(): pass +", + ), + ]); + + let mut cmd = context.karva_command_in(context.root().join("tests")); + cmd.arg("test"); + + assert_cmd_snapshot!(cmd, @" + success: true + exit_code: 0 + ----- stdout ----- + Starting 1 test across 1 worker + PASS [TIME] tests.test_a::check_found + + ──────────── + Summary [TIME] 1 test run: 1 passed, 0 skipped + + ----- stderr ----- + "); +} + #[test] #[cfg(unix)] fn test_config_file_flag_nonexistent_unix() { diff --git a/crates/karva/tests/it/discovery/edge_cases.rs b/crates/karva/tests/it/discovery/edge_cases.rs new file mode 100644 index 00000000..7859d27f --- /dev/null +++ b/crates/karva/tests/it/discovery/edge_cases.rs @@ -0,0 +1,184 @@ +use insta_cmd::assert_cmd_snapshot; + +use crate::common::TestContext; + +/// `__pycache__` directories and compiled `.pyc` files alongside source files +/// should not be picked up as tests. +#[test] +fn test_pyc_files_and_pycache_are_ignored() { + let context = TestContext::with_files([( + "test_real.py", + r" +def test_real(): pass +", + )]); + + let pycache = context.root().join("__pycache__"); + std::fs::create_dir_all(&pycache).expect("failed to create __pycache__"); + std::fs::write(pycache.join("test_real.cpython-313.pyc"), b"bogus") + .expect("failed to write .pyc"); + + assert_cmd_snapshot!(context.command_no_parallel(), @" + success: true + exit_code: 0 + ----- stdout ----- + Starting 1 test across 1 worker + PASS [TIME] test_real::test_real + + ──────────── + Summary [TIME] 1 test run: 1 passed, 0 skipped + + ----- stderr ----- + "); +} + +/// A package with `__init__.py` should have its tests discovered under the +/// package path, while standalone sibling files stay at the top level. +#[test] +fn test_package_init_and_standalone_siblings() { + let context = TestContext::with_files([ + ("pkg/__init__.py", ""), + ( + "pkg/test_in_pkg.py", + r" +def test_inside_package(): pass +", + ), + ( + "test_standalone.py", + r" +def test_at_root(): pass +", + ), + ]); + + assert_cmd_snapshot!(context.command_no_parallel().arg("-q"), @" + success: true + exit_code: 0 + ----- stdout ----- + ──────────── + Summary [TIME] 2 tests run: 2 passed, 0 skipped + + ----- stderr ----- + "); +} + +/// A test directory matching a `.gitignore` rule is skipped by default and +/// restored when `--no-ignore` is passed. +#[test] +fn test_gitignore_excludes_directory() { + let context = TestContext::with_files([ + (".gitignore", "ignored/\n"), + ( + "ignored/test_skipped.py", + r" +def test_skipped(): pass +", + ), + ( + "test_kept.py", + r" +def test_kept(): pass +", + ), + ]); + + assert_cmd_snapshot!(context.command_no_parallel(), @" + success: true + exit_code: 0 + ----- stdout ----- + Starting 1 test across 1 worker + PASS [TIME] test_kept::test_kept + + ──────────── + Summary [TIME] 1 test run: 1 passed, 0 skipped + + ----- stderr ----- + "); +} + +#[test] +fn test_no_ignore_includes_gitignored_directory() { + let context = TestContext::with_files([ + (".gitignore", "ignored/\n"), + ( + "ignored/test_skipped.py", + r" +def test_in_ignored(): pass +", + ), + ( + "test_kept.py", + r" +def test_kept(): pass +", + ), + ]); + + assert_cmd_snapshot!(context.command_no_parallel().args(["--no-ignore", "-q"]), @" + success: true + exit_code: 0 + ----- stdout ----- + ──────────── + Summary [TIME] 2 tests run: 2 passed, 0 skipped + + ----- stderr ----- + "); +} + +/// A python file that contains no test functions alongside a file that does +/// should be collected silently. +#[test] +fn test_python_file_without_test_functions_is_ignored() { + let context = TestContext::with_files([ + ( + "test_helpers.py", + r" +x = 1 +def helper(): + return 42 +", + ), + ( + "test_real.py", + r" +def test_one(): pass +", + ), + ]); + + assert_cmd_snapshot!(context.command_no_parallel(), @" + success: true + exit_code: 0 + ----- stdout ----- + Starting 1 test across 1 worker + PASS [TIME] test_real::test_one + + ──────────── + Summary [TIME] 1 test run: 1 passed, 0 skipped + + ----- stderr ----- + "); +} + +/// An empty subdirectory (no Python files at all) is discovered without error. +#[test] +fn test_empty_subdirectory_is_ignored() { + let context = TestContext::with_file("test_a.py", "def test_a(): pass"); + + std::fs::create_dir_all(context.root().join("empty_dir")) + .expect("failed to create empty directory"); + + assert_cmd_snapshot!(context.command_no_parallel(), @" + success: true + exit_code: 0 + ----- stdout ----- + Starting 1 test across 1 worker + PASS [TIME] test_a::test_a + + ──────────── + Summary [TIME] 1 test run: 1 passed, 0 skipped + + ----- stderr ----- + "); +} diff --git a/crates/karva/tests/it/discovery/mod.rs b/crates/karva/tests/it/discovery/mod.rs index 13113268..8350d1d4 100644 --- a/crates/karva/tests/it/discovery/mod.rs +++ b/crates/karva/tests/it/discovery/mod.rs @@ -1,2 +1,3 @@ +mod edge_cases; mod git_boundary; mod nested_layouts; diff --git a/crates/karva/tests/it/last_failed.rs b/crates/karva/tests/it/last_failed.rs index 39fbf8be..0adc9f9b 100644 --- a/crates/karva/tests/it/last_failed.rs +++ b/crates/karva/tests/it/last_failed.rs @@ -172,44 +172,157 @@ def test_fail_b(): assert False "); } -/// Combining `--last-failed` with `-E` should further narrow the rerun set -/// to only previously-failed tests that also match the filter expression. +/// `--dry-run` currently ignores `--last-failed` and prints every discovered +/// test. Pinning this so the behavior is intentional — see PR follow-ups. #[test] -fn last_failed_with_filter_narrows_rerun_set() { - let context = TestContext::with_files([ - ( - "test_a.py", - " -def test_pass_a(): pass +fn last_failed_with_dry_run_shows_all_tests() { + let context = TestContext::with_file( + "test_a.py", + " +def test_pass(): pass +def test_fail(): assert False + ", + ); + + context.command_no_parallel().output().unwrap(); + + assert_cmd_snapshot!( + context + .command_no_parallel() + .args(["--last-failed", "--dry-run"]), + @" + success: true + exit_code: 0 + ----- stdout ----- + test_a::test_fail + test_a::test_pass + + 2 tests collected + + ----- stderr ----- + " + ); +} + +/// A filter combined with `--last-failed` intersects: tests that were in the +/// last-failed set but are now filtered out are skipped. +#[test] +fn last_failed_with_filter_intersects() { + let context = TestContext::with_file( + "test_a.py", + " +def test_pass(): pass def test_fail_a(): assert False - ", - ), - ( - "test_b.py", - " -def test_pass_b(): pass def test_fail_b(): assert False - ", - ), - ]); + ", + ); context.command_no_parallel().output().unwrap(); assert_cmd_snapshot!( context .command_no_parallel() - .arg("--last-failed") - .args(["-E", "test(~fail_a)"]) - .arg("-q"), + .args(["--last-failed", "-E", "test(~fail_a)"]), @" success: false exit_code: 1 ----- stdout ----- + Starting 3 tests across 1 worker + FAIL [TIME] test_a::test_fail_a + SKIP [TIME] test_a::test_fail_b + + diagnostics: + + error[test-failure]: Test `test_fail_a` failed + --> test_a.py:3:5 + | + 2 | def test_pass(): pass + 3 | def test_fail_a(): assert False + | ^^^^^^^^^^^ + 4 | def test_fail_b(): assert False + | + info: Test failed here + --> test_a.py:3:1 + | + 2 | def test_pass(): pass + 3 | def test_fail_a(): assert False + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 4 | def test_fail_b(): assert False + | + ──────────── Summary [TIME] 2 tests run: 0 passed, 1 failed, 1 skipped ----- stderr ----- - ", + " + ); +} + +/// `--last-failed` + `--max-fail=1` still stops scheduling once a single test +/// in the rerun has failed. +#[test] +fn last_failed_with_max_fail_stops_early() { + let context = TestContext::with_file( + "test_a.py", + " +def test_pass(): pass +def test_fail_a(): assert False +def test_fail_b(): assert False + ", + ); + + context.command_no_parallel().output().unwrap(); + + assert_cmd_snapshot!( + context + .command_no_parallel() + .args(["--last-failed", "--max-fail=1", "-q"]), + @" + success: false + exit_code: 1 + ----- stdout ----- + ──────────── + Summary [TIME] 1 test run: 0 passed, 1 failed, 0 skipped + + ----- stderr ----- + " + ); +} + +/// Adding a brand new test after a run does not cause `--last-failed` to pick +/// it up — only previously-known failures are rerun. +#[test] +fn last_failed_ignores_newly_added_tests() { + let context = TestContext::with_file( + "test_a.py", + " +def test_pass(): pass +def test_fail(): assert False + ", + ); + + context.command_no_parallel().output().unwrap(); + + context.write_file( + "test_a.py", + " +def test_pass(): pass +def test_fail(): assert False +def test_new_fail(): assert False + ", + ); + + assert_cmd_snapshot!( + context.command_no_parallel().args(["--last-failed", "-q"]), + @" + success: false + exit_code: 1 + ----- stdout ----- + ──────────── + Summary [TIME] 1 test run: 0 passed, 1 failed, 0 skipped + + ----- stderr ----- + " ); } From a0b208ca2b06a536807150c27acdcdf60247568f Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sat, 11 Apr 2026 20:02:14 +0100 Subject: [PATCH 04/11] test: rewrite no-subcommand help test to use assert_cmd_snapshot --- crates/karva/tests/it/basic.rs | 35 +++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/crates/karva/tests/it/basic.rs b/crates/karva/tests/it/basic.rs index c0288a39..80744405 100644 --- a/crates/karva/tests/it/basic.rs +++ b/crates/karva/tests/it/basic.rs @@ -2202,23 +2202,32 @@ fn test_nonexistent_path_exits_nonzero() { "); } -/// `karva` with no subcommand should print help and exit successfully. +/// `karva` with no subcommand is a clap error: exit code 2, help on stderr. #[test] fn test_no_subcommand_prints_help() { let context = TestContext::new(); - let output = context - .karva_command_in(context.root()) - .output() - .expect("failed to run karva with no subcommand"); - - // No subcommand is a clap error; exit code is 2 and help goes to stderr. - assert_eq!(output.status.code(), Some(2)); - let stderr = String::from_utf8_lossy(&output.stderr); - assert!( - stderr.contains("Usage: karva "), - "expected usage line in stderr, got: {stderr}" - ); + assert_cmd_snapshot!(context.karva_command_in(context.root()), @" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + A Python test runner. + + Usage: karva + + Commands: + test Run tests + snapshot Manage snapshots created by `karva.assert_snapshot()` + cache Manage the karva cache + version Display Karva's version + help Print this message or the help of the given subcommand(s) + + Options: + -h, --help Print help + -V, --version Print version + "); } /// `karva testx` (typo of `test`) should suggest the closest subcommand. From fa4a55189c68e68ba28cfd1422e96e8b878919c3 Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sat, 11 Apr 2026 20:29:42 +0100 Subject: [PATCH 05/11] test: convert unit-test assertions to inline snapshots Follow-up to the consolidated audit: converts hand-written assert_eq! assertions in the newly added karva_logging, karva_project, karva_metadata, and karva_cache unit tests to insta inline snapshots where snapshots add value. Single-value primitive checks (booleans, matches!, length counts) and filesystem existence checks are left as-is. --- Cargo.lock | 4 + crates/karva_cache/Cargo.toml | 1 + crates/karva_cache/src/cache.rs | 81 +++++++--- crates/karva_logging/Cargo.toml | 3 + crates/karva_logging/src/time.rs | 38 ++--- crates/karva_metadata/Cargo.toml | 3 + crates/karva_metadata/src/options.rs | 207 +++++++++++++++++++------ crates/karva_project/Cargo.toml | 1 + crates/karva_project/src/path/utils.rs | 44 +++--- 9 files changed, 265 insertions(+), 117 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 09876199..74777c45 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1119,6 +1119,7 @@ version = "0.0.0" dependencies = [ "anyhow", "camino", + "insta", "karva_diagnostic", "ruff_db", "serde", @@ -1192,6 +1193,7 @@ dependencies = [ "chrono", "clap", "colored 3.1.1", + "insta", "tracing", "tracing-flame", "tracing-subscriber", @@ -1215,6 +1217,7 @@ version = "0.0.0" dependencies = [ "camino", "globset", + "insta", "karva_combine", "karva_macros", "regex", @@ -1233,6 +1236,7 @@ version = "0.0.0" dependencies = [ "anyhow", "camino", + "insta", "karva_metadata", "karva_python_semantic", "tempfile", diff --git a/crates/karva_cache/Cargo.toml b/crates/karva_cache/Cargo.toml index 68ae4cb1..e1f76518 100644 --- a/crates/karva_cache/Cargo.toml +++ b/crates/karva_cache/Cargo.toml @@ -19,6 +19,7 @@ serde = { workspace = true } serde_json = { workspace = true } [dev-dependencies] +insta = { workspace = true } tempfile = { workspace = true } [lints] diff --git a/crates/karva_cache/src/cache.rs b/crates/karva_cache/src/cache.rs index 9561d36b..52dc81ff 100644 --- a/crates/karva_cache/src/cache.rs +++ b/crates/karva_cache/src/cache.rs @@ -319,6 +319,7 @@ mod tests { use std::fs; use camino::Utf8PathBuf; + use insta::{assert_debug_snapshot, assert_snapshot}; use super::*; @@ -385,9 +386,16 @@ mod tests { let cache = Cache::new(&cache_dir, &run_hash); let results = cache.aggregate_results().unwrap(); - assert_eq!(results.stats.passed(), 5); - assert_eq!(results.stats.failed(), 1); - assert_eq!(results.stats.skipped(), 1); + assert_debug_snapshot!( + (results.stats.passed(), results.stats.failed(), results.stats.skipped()), + @r" + ( + 5, + 1, + 1, + ) + " + ); } #[test] @@ -402,8 +410,8 @@ mod tests { let cache = Cache::new(&cache_dir, &run_hash); let results = cache.aggregate_results().unwrap(); - assert_eq!(results.stats.total(), 0); - assert!(results.diagnostics.is_empty()); + assert_debug_snapshot!(results.stats.total(), @"0"); + assert_snapshot!(results.diagnostics, @""); } #[test] @@ -414,8 +422,12 @@ mod tests { let failed = vec!["mod::test_a".to_string(), "mod::test_b".to_string()]; write_last_failed(&cache_dir, &failed).unwrap(); - let read = read_last_failed(&cache_dir).unwrap(); - assert_eq!(read, failed); + assert_debug_snapshot!(read_last_failed(&cache_dir).unwrap(), @r#" + [ + "mod::test_a", + "mod::test_b", + ] + "#); } #[test] @@ -435,8 +447,11 @@ mod tests { write_last_failed(&cache_dir, &["old".to_string()]).unwrap(); write_last_failed(&cache_dir, &["new".to_string()]).unwrap(); - let read = read_last_failed(&cache_dir).unwrap(); - assert_eq!(read, vec!["new".to_string()]); + assert_debug_snapshot!(read_last_failed(&cache_dir).unwrap(), @r#" + [ + "new", + ] + "#); } #[test] @@ -448,7 +463,11 @@ mod tests { write_last_failed(&cache_dir, &["x".to_string()]).unwrap(); assert!(cache_dir.exists()); - assert_eq!(read_last_failed(&cache_dir).unwrap(), vec!["x".to_string()]); + assert_debug_snapshot!(read_last_failed(&cache_dir).unwrap(), @r#" + [ + "x", + ] + "#); } #[test] @@ -469,10 +488,14 @@ mod tests { fs::create_dir_all(tmp.path().join(ts)).unwrap(); } - let result = prune_cache(&cache_dir).unwrap(); - assert_eq!(result.removed.len(), 2); - assert!(result.removed.contains(&"run-100".to_string())); - assert!(result.removed.contains(&"run-200".to_string())); + let mut removed = prune_cache(&cache_dir).unwrap().removed; + removed.sort(); + assert_debug_snapshot!(removed, @r#" + [ + "run-100", + "run-200", + ] + "#); assert!(cache_dir.join("run-300").exists()); assert!(!cache_dir.join("run-100").exists()); assert!(!cache_dir.join("run-200").exists()); @@ -575,15 +598,27 @@ mod tests { let mut failed = results.failed_tests.clone(); failed.sort(); - assert_eq!( - failed, - vec!["mod::test_a".to_string(), "mod::test_b".to_string()] - ); - assert_eq!(results.durations.len(), 2); - assert_eq!( - results.durations.get("mod::test_a"), - Some(&Duration::from_millis(10)) - ); + assert_debug_snapshot!(failed, @r#" + [ + "mod::test_a", + "mod::test_b", + ] + "#); + + let mut durations: Vec<(String, Duration)> = results.durations.into_iter().collect(); + durations.sort(); + assert_debug_snapshot!(durations, @r#" + [ + ( + "mod::test_a", + 10ms, + ), + ( + "mod::test_b", + 20ms, + ), + ] + "#); } #[test] diff --git a/crates/karva_logging/Cargo.toml b/crates/karva_logging/Cargo.toml index ed1a121c..6e64f905 100644 --- a/crates/karva_logging/Cargo.toml +++ b/crates/karva_logging/Cargo.toml @@ -18,5 +18,8 @@ tracing-flame = { workspace = true } tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"] } tracing-tree = { workspace = true } +[dev-dependencies] +insta = { workspace = true } + [lints] workspace = true diff --git a/crates/karva_logging/src/time.rs b/crates/karva_logging/src/time.rs index ae8b056d..51c54f33 100644 --- a/crates/karva_logging/src/time.rs +++ b/crates/karva_logging/src/time.rs @@ -15,62 +15,58 @@ pub fn format_duration_bracketed(duration: Duration) -> String { #[cfg(test)] mod tests { + use insta::assert_snapshot; + use super::*; #[test] fn format_duration_zero_is_zero_ms() { - assert_eq!(format_duration(Duration::ZERO), "0ms"); + assert_snapshot!(format_duration(Duration::ZERO), @"0ms"); } #[test] fn format_duration_sub_millisecond_truncates_to_zero_ms() { - assert_eq!(format_duration(Duration::from_micros(500)), "0ms"); - assert_eq!(format_duration(Duration::from_nanos(1)), "0ms"); + assert_snapshot!(format_duration(Duration::from_micros(500)), @"0ms"); + assert_snapshot!(format_duration(Duration::from_nanos(1)), @"0ms"); } #[test] fn format_duration_exactly_one_ms() { - assert_eq!(format_duration(Duration::from_millis(1)), "1ms"); + assert_snapshot!(format_duration(Duration::from_millis(1)), @"1ms"); } + /// The cutoff is `< 2s`, so anything under two full seconds stays in ms, + /// including the exact one-second boundary and values like 1999 ms. #[test] fn format_duration_sub_two_seconds_uses_milliseconds() { - // The cutoff is `< 2s`, so anything under two full seconds stays in ms, - // including the exact one-second boundary and values like 1999 ms. - assert_eq!(format_duration(Duration::from_millis(1000)), "1000ms"); - assert_eq!(format_duration(Duration::from_millis(1999)), "1999ms"); + assert_snapshot!(format_duration(Duration::from_millis(1000)), @"1000ms"); + assert_snapshot!(format_duration(Duration::from_millis(1999)), @"1999ms"); } #[test] fn format_duration_two_seconds_switches_to_seconds() { - assert_eq!(format_duration(Duration::from_secs(2)), "2.00s"); + assert_snapshot!(format_duration(Duration::from_secs(2)), @"2.00s"); } #[test] fn format_duration_rounds_to_two_decimals() { - assert_eq!(format_duration(Duration::from_millis(2346)), "2.35s"); - assert_eq!(format_duration(Duration::from_millis(2344)), "2.34s"); + assert_snapshot!(format_duration(Duration::from_millis(2346)), @"2.35s"); + assert_snapshot!(format_duration(Duration::from_millis(2344)), @"2.34s"); } #[test] fn format_duration_minutes_stay_in_seconds() { - assert_eq!(format_duration(Duration::from_secs(125)), "125.00s"); + assert_snapshot!(format_duration(Duration::from_secs(125)), @"125.00s"); } #[test] fn format_duration_bracketed_pads_to_width_and_three_decimals() { - assert_eq!(format_duration_bracketed(Duration::ZERO), "[ 0.000s]"); - assert_eq!( - format_duration_bracketed(Duration::from_millis(15)), - "[ 0.015s]" - ); + assert_snapshot!(format_duration_bracketed(Duration::ZERO), @"[ 0.000s]"); + assert_snapshot!(format_duration_bracketed(Duration::from_millis(15)), @"[ 0.015s]"); } #[test] fn format_duration_bracketed_handles_large_values() { - assert_eq!( - format_duration_bracketed(Duration::from_secs(12345)), - "[12345.000s]" - ); + assert_snapshot!(format_duration_bracketed(Duration::from_secs(12345)), @"[12345.000s]"); } } diff --git a/crates/karva_metadata/Cargo.toml b/crates/karva_metadata/Cargo.toml index 97133cb1..120f24d9 100644 --- a/crates/karva_metadata/Cargo.toml +++ b/crates/karva_metadata/Cargo.toml @@ -27,5 +27,8 @@ thiserror = { workspace = true } toml = { workspace = true } tracing = { workspace = true } +[dev-dependencies] +insta = { workspace = true } + [lints] workspace = true diff --git a/crates/karva_metadata/src/options.rs b/crates/karva_metadata/src/options.rs index 820472c0..71c0f191 100644 --- a/crates/karva_metadata/src/options.rs +++ b/crates/karva_metadata/src/options.rs @@ -301,17 +301,26 @@ impl Combine for OutputFormat { mod tests { use std::num::NonZeroU32; + use insta::{assert_debug_snapshot, assert_snapshot}; use karva_combine::Combine; use super::*; #[test] fn to_settings_defaults_when_empty() { - let settings = TestOptions::default().to_settings(); - assert_eq!(settings.test_function_prefix, "test"); - assert_eq!(settings.max_fail, MaxFail::unlimited()); - assert!(!settings.try_import_fixtures); - assert_eq!(settings.retry, 0); + assert_debug_snapshot!(TestOptions::default().to_settings(), @r#" + TestSettings { + test_function_prefix: "test", + max_fail: MaxFail( + None, + ), + try_import_fixtures: false, + retry: 0, + filter: FiltersetSet { + filters: [], + }, + } + "#); } #[test] @@ -320,7 +329,13 @@ mod tests { fail_fast: Some(true), ..TestOptions::default() }; - assert_eq!(options.to_settings().max_fail, MaxFail::from_count(1)); + assert_debug_snapshot!(options.to_settings().max_fail, @" + MaxFail( + Some( + 1, + ), + ) + "); } #[test] @@ -329,7 +344,11 @@ mod tests { fail_fast: Some(false), ..TestOptions::default() }; - assert_eq!(options.to_settings().max_fail, MaxFail::unlimited()); + assert_debug_snapshot!(options.to_settings().max_fail, @" + MaxFail( + None, + ) + "); } #[test] @@ -339,7 +358,13 @@ mod tests { max_fail: Some(MaxFail::from(NonZeroU32::new(5).expect("non-zero"))), ..TestOptions::default() }; - assert_eq!(options.to_settings().max_fail, MaxFail::from_count(5)); + assert_debug_snapshot!(options.to_settings().max_fail, @" + MaxFail( + Some( + 5, + ), + ) + "); } #[test] @@ -359,14 +384,34 @@ respect-ignore-files = false include = ["tests", "more"] "#; let options = Options::from_toml_str(toml).expect("parse"); - let settings = options.to_settings(); - assert_eq!(settings.test().test_function_prefix, "check"); - assert_eq!(settings.test().max_fail, MaxFail::from_count(3)); - assert_eq!(settings.test().retry, 2); - assert_eq!(settings.terminal().output_format, OutputFormat::Concise); - assert!(settings.terminal().show_python_output); - assert!(!settings.src().respect_ignore_files); - assert_eq!(settings.src().include_paths, vec!["tests", "more"]); + assert_debug_snapshot!(options.to_settings(), @r#" + ProjectSettings { + terminal: TerminalSettings { + output_format: Concise, + show_python_output: true, + }, + src: SrcSettings { + respect_ignore_files: false, + include_paths: [ + "tests", + "more", + ], + }, + test: TestSettings { + test_function_prefix: "check", + max_fail: MaxFail( + Some( + 3, + ), + ), + try_import_fixtures: false, + retry: 2, + filter: FiltersetSet { + filters: [], + }, + }, + } + "#); } #[test] @@ -376,8 +421,16 @@ include = ["tests", "more"] fail-fast = true nonsense = 42 "; - let err = Options::from_toml_str(toml).expect_err("unknown field"); - assert!(matches!(err, KarvaTomlError::TomlSyntax(_))); + assert_snapshot!( + Options::from_toml_str(toml).expect_err("unknown field"), + @" + TOML parse error at line 4, column 1 + | + 4 | nonsense = 42 + | ^^^^^^^^ + unknown field `nonsense`, expected one of `test-function-prefix`, `fail-fast`, `max-fail`, `try-import-fixtures`, `retry` + " + ); } #[test] @@ -386,30 +439,47 @@ nonsense = 42 [bogus] foo = 1 "; - assert!(matches!( - Options::from_toml_str(toml), - Err(KarvaTomlError::TomlSyntax(_)) - )); + assert_snapshot!( + Options::from_toml_str(toml).expect_err("unknown section"), + @" + TOML parse error at line 2, column 2 + | + 2 | [bogus] + | ^^^^^ + unknown field `bogus`, expected one of `src`, `terminal`, `test` + " + ); } #[test] fn from_toml_str_empty_is_default() { - let options = Options::from_toml_str("").expect("parse"); - assert_eq!(options, Options::default()); + assert_debug_snapshot!(Options::from_toml_str("").expect("parse"), @" + Options { + src: None, + terminal: None, + test: None, + } + "); } + /// `MaxFail` wraps `NonZeroU32`, so raw `0` must be rejected by the + /// deserializer rather than silently producing `unlimited`. #[test] fn from_toml_str_rejects_max_fail_zero() { - // MaxFail wraps NonZeroU32 so the raw integer 0 must be rejected by the - // deserializer rather than silently producing `unlimited`. let toml = r" [test] max-fail = 0 "; - assert!(matches!( - Options::from_toml_str(toml), - Err(KarvaTomlError::TomlSyntax(_)) - )); + assert_snapshot!( + Options::from_toml_str(toml).expect_err("zero rejected"), + @" + TOML parse error at line 3, column 12 + | + 3 | max-fail = 0 + | ^ + invalid value: integer `0`, expected a nonzero u32 + " + ); } #[test] @@ -425,10 +495,21 @@ max-fail = 0 try_import_fixtures: Some(true), ..TestOptions::default() }; - let merged = cli.combine(file); - assert_eq!(merged.test_function_prefix.as_deref(), Some("cli_prefix")); - assert_eq!(merged.retry, Some(5)); - assert_eq!(merged.try_import_fixtures, Some(true)); + assert_debug_snapshot!(cli.combine(file), @r#" + TestOptions { + test_function_prefix: Some( + "cli_prefix", + ), + fail_fast: None, + max_fail: None, + try_import_fixtures: Some( + true, + ), + retry: Some( + 5, + ), + } + "#); } #[test] @@ -440,12 +521,25 @@ max-fail = 0 retry: Some(3), ..TestOptions::default() }; - let merged = cli.combine(file); - assert_eq!(merged.test_function_prefix.as_deref(), Some("from_file")); - assert_eq!(merged.fail_fast, Some(true)); - assert_eq!(merged.retry, Some(3)); + assert_debug_snapshot!(cli.combine(file), @r#" + TestOptions { + test_function_prefix: Some( + "from_file", + ), + fail_fast: Some( + true, + ), + max_fail: None, + try_import_fixtures: None, + retry: Some( + 3, + ), + } + "#); } + /// `Vec::combine` appends `self` after `other`, so CLI entries take + /// precedence at the tail. #[test] fn combine_merges_include_paths_with_cli_taking_precedence() { let cli = SrcOptions { @@ -456,11 +550,19 @@ max-fail = 0 include: Some(vec!["file_only".to_string()]), respect_ignore_files: Some(false), }; - let merged = cli.combine(file); - // Vec combine appends `self` after `other`, so CLI entries take precedence at the tail. - let include = merged.include.expect("include set"); - assert_eq!(include, vec!["file_only", "cli_only"]); - assert_eq!(merged.respect_ignore_files, Some(false)); + assert_debug_snapshot!(cli.combine(file), @r#" + SrcOptions { + respect_ignore_files: Some( + false, + ), + include: Some( + [ + "file_only", + "cli_only", + ], + ), + } + "#); } #[test] @@ -481,10 +583,21 @@ max-fail = 0 ..Options::default() }; let overrides = ProjectOptionsOverrides::new(None, cli_options); - let merged = overrides.apply_to(file_options); - let test = merged.test.expect("test section set"); - assert_eq!(test.test_function_prefix.as_deref(), Some("cli")); - assert_eq!(test.retry, Some(2)); + assert_debug_snapshot!(overrides.apply_to(file_options).test, @r#" + Some( + TestOptions { + test_function_prefix: Some( + "cli", + ), + fail_fast: None, + max_fail: None, + try_import_fixtures: None, + retry: Some( + 2, + ), + }, + ) + "#); } } diff --git a/crates/karva_project/Cargo.toml b/crates/karva_project/Cargo.toml index e8fcfd55..3f74d232 100644 --- a/crates/karva_project/Cargo.toml +++ b/crates/karva_project/Cargo.toml @@ -18,6 +18,7 @@ karva_python_semantic = { workspace = true } thiserror = { workspace = true } [dev-dependencies] +insta = { workspace = true } tempfile = { workspace = true } [lints] diff --git a/crates/karva_project/src/path/utils.rs b/crates/karva_project/src/path/utils.rs index 554f42bf..4346e79e 100644 --- a/crates/karva_project/src/path/utils.rs +++ b/crates/karva_project/src/path/utils.rs @@ -36,82 +36,74 @@ pub fn absolute(path: impl AsRef, cwd: impl AsRef) -> Utf8Pa #[cfg(test)] mod tests { + use insta::assert_snapshot; + use super::*; #[test] fn relative_path_is_joined_to_cwd() { - let result = absolute("foo/bar", "/home/user"); - assert_eq!(result, Utf8PathBuf::from("/home/user/foo/bar")); + assert_snapshot!(absolute("foo/bar", "/home/user"), @"/home/user/foo/bar"); } #[test] fn absolute_path_ignores_cwd() { - let result = absolute("/absolute/path", "/home/user"); - assert_eq!(result, Utf8PathBuf::from("/absolute/path")); + assert_snapshot!(absolute("/absolute/path", "/home/user"), @"/absolute/path"); } #[test] fn parent_dir_pops_component() { - let result = absolute("../sibling", "/home/user"); - assert_eq!(result, Utf8PathBuf::from("/home/sibling")); + assert_snapshot!(absolute("../sibling", "/home/user"), @"/home/sibling"); } #[test] fn current_dir_is_ignored() { - let result = absolute("./foo", "/home/user"); - assert_eq!(result, Utf8PathBuf::from("/home/user/foo")); + assert_snapshot!(absolute("./foo", "/home/user"), @"/home/user/foo"); } #[test] fn mixed_components() { - let result = absolute("foo/../bar/./baz", "/cwd"); - assert_eq!(result, Utf8PathBuf::from("/cwd/bar/baz")); + assert_snapshot!(absolute("foo/../bar/./baz", "/cwd"), @"/cwd/bar/baz"); } #[test] fn empty_relative_path_returns_cwd() { - let result = absolute("", "/home/user"); - assert_eq!(result, Utf8PathBuf::from("/home/user")); + assert_snapshot!(absolute("", "/home/user"), @"/home/user"); } #[test] fn leading_parent_pops_cwd() { - let result = absolute("../../other", "/home/user"); - assert_eq!(result, Utf8PathBuf::from("/other")); + assert_snapshot!(absolute("../../other", "/home/user"), @"/other"); } + /// `Utf8PathBuf::pop` on `/` returns false, so extra `..` components + /// must not escape the filesystem root. #[test] fn parent_past_root_stays_at_root() { - // `Utf8PathBuf::pop` on `/` returns false, so extra `..` components - // must not escape the filesystem root. - let result = absolute("../..", "/"); - assert_eq!(result, Utf8PathBuf::from("/")); + assert_snapshot!(absolute("../..", "/"), @"/"); } #[test] fn unicode_path_components_are_preserved() { - let result = absolute("カルヴァ/tests", "/home/ユーザー"); - assert_eq!(result, Utf8PathBuf::from("/home/ユーザー/カルヴァ/tests")); + assert_snapshot!(absolute("カルヴァ/tests", "/home/ユーザー"), @"/home/ユーザー/カルヴァ/tests"); } #[test] fn path_with_spaces_is_preserved() { - let result = absolute("my tests/file.py", "/home/my user"); - assert_eq!(result, Utf8PathBuf::from("/home/my user/my tests/file.py")); + assert_snapshot!(absolute("my tests/file.py", "/home/my user"), @"/home/my user/my tests/file.py"); } + /// `camino` components strip trailing slashes, so the result should + /// match the same input without one. #[test] fn trailing_slash_on_relative_input_is_normalized() { - // `camino` components strip trailing slashes, so the result should - // match the same input without one. let with = absolute("foo/bar/", "/cwd"); let without = absolute("foo/bar", "/cwd"); assert_eq!(with, without); + assert_snapshot!(with, @"/cwd/foo/bar"); } #[test] fn dot_only_path_is_cwd() { - let result = absolute(".", "/home/user"); - assert_eq!(result, Utf8PathBuf::from("/home/user")); + assert_snapshot!(absolute(".", "/home/user"), @"/home/user"); } } From 64c1eb99b8ebdd544dfc74897b103c47a2afa862 Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sat, 11 Apr 2026 20:13:44 +0100 Subject: [PATCH 06/11] test(diagnostic): snapshot traceback parser assertions Converts `filter_traceback`, `parse_traceback_line`, `get_traceback_location`, and `calculate_line_range` unit tests to inline snapshots. These assert on multi-line strings and `Option` / `Option` structs where seeing the full parsed shape is much easier to review than hand-built expected values. --- crates/karva_diagnostic/Cargo.toml | 1 + crates/karva_diagnostic/src/traceback.rs | 157 +++++++++++++---------- 2 files changed, 90 insertions(+), 68 deletions(-) diff --git a/crates/karva_diagnostic/Cargo.toml b/crates/karva_diagnostic/Cargo.toml index ce38c559..252615cf 100644 --- a/crates/karva_diagnostic/Cargo.toml +++ b/crates/karva_diagnostic/Cargo.toml @@ -25,6 +25,7 @@ ruff_text_size = { workspace = true } serde = { workspace = true } [dev-dependencies] +insta = { workspace = true } serde_json = { workspace = true } [lints] diff --git a/crates/karva_diagnostic/src/traceback.rs b/crates/karva_diagnostic/src/traceback.rs index 81f86cf8..a0dc955c 100644 --- a/crates/karva_diagnostic/src/traceback.rs +++ b/crates/karva_diagnostic/src/traceback.rs @@ -156,20 +156,16 @@ File "test.py", line 1, in raise Exception('Test error') Exception: Test error "#; - let filtered = filter_traceback(traceback); - assert_eq!( - filtered, - r#"File "test.py", line 1, in - raise Exception('Test error') -Exception: Test error"# - ); + insta::assert_snapshot!(filter_traceback(traceback), @r#" + File "test.py", line 1, in + raise Exception('Test error') + Exception: Test error + "#); } #[test] fn test_filter_traceback_empty() { - let traceback = ""; - let filtered = filter_traceback(traceback); - assert_eq!(filtered, ""); + insta::assert_snapshot!(filter_traceback(""), @""); } } @@ -179,55 +175,63 @@ Exception: Test error"# #[test] fn test_parse_traceback_line_valid() { let line = r#" File "test.py", line 10, in "#; - let location = parse_traceback_line(line); - let expected_location = Some(TracebackLocation { - file_path: "test.py".into(), - line_number: OneIndexed::new(10).unwrap(), - }); - assert_eq!(location, expected_location); + insta::assert_debug_snapshot!(parse_traceback_line(line), @r#" + Some( + TracebackLocation { + file_path: "test.py", + line_number: OneIndexed( + 10, + ), + }, + ) + "#); } #[test] fn test_parse_traceback_line_with_path() { let line = r#" File "/path/to/script.py", line 42, in function_name"#; - let location = parse_traceback_line(line); - let expected_location = Some(TracebackLocation { - file_path: "/path/to/script.py".into(), - line_number: OneIndexed::new(42).unwrap(), - }); - assert_eq!(location, expected_location); + insta::assert_debug_snapshot!(parse_traceback_line(line), @r#" + Some( + TracebackLocation { + file_path: "/path/to/script.py", + line_number: OneIndexed( + 42, + ), + }, + ) + "#); } #[test] fn test_parse_traceback_line_no_file_prefix() { - let line = "Some random line"; - let location = parse_traceback_line(line); - assert_eq!(location, None); + insta::assert_debug_snapshot!(parse_traceback_line("Some random line"), @"None"); } #[test] fn test_parse_traceback_line_missing_line_number() { let line = r#" File "test.py", in "#; - let location = parse_traceback_line(line); - assert_eq!(location, None); + insta::assert_debug_snapshot!(parse_traceback_line(line), @"None"); } #[test] fn test_parse_traceback_line_malformed_quote() { let line = r#" File "test.py, line 10, in "#; - let location = parse_traceback_line(line); - assert_eq!(location, None); + insta::assert_debug_snapshot!(parse_traceback_line(line), @"None"); } #[test] fn test_parse_traceback_line_large_line_number() { let line = r#" File "test.py", line 99999, in "#; - let location = parse_traceback_line(line); - let expected_location = Some(TracebackLocation { - file_path: "test.py".into(), - line_number: OneIndexed::new(99999).unwrap(), - }); - assert_eq!(location, expected_location); + insta::assert_debug_snapshot!(parse_traceback_line(line), @r#" + Some( + TracebackLocation { + file_path: "test.py", + line_number: OneIndexed( + 99999, + ), + }, + ) + "#); } } @@ -240,12 +244,16 @@ Exception: Test error"# File "test.py", line 10, in raise Exception('Test error') Exception: Test error"#; - let location = get_traceback_location(traceback); - let expected_location = Some(TracebackLocation { - file_path: "test.py".into(), - line_number: OneIndexed::new(10).unwrap(), - }); - assert_eq!(location, expected_location); + insta::assert_debug_snapshot!(get_traceback_location(traceback), @r#" + Some( + TracebackLocation { + file_path: "test.py", + line_number: OneIndexed( + 10, + ), + }, + ) + "#); } #[test] @@ -256,26 +264,26 @@ Exception: Test error"#; File "helper.py", line 15, in foo bar() ValueError: Invalid value"#; - let location = get_traceback_location(traceback); - let expected_location = Some(TracebackLocation { - file_path: "helper.py".into(), - line_number: OneIndexed::new(15).unwrap(), - }); - assert_eq!(location, expected_location); + insta::assert_debug_snapshot!(get_traceback_location(traceback), @r#" + Some( + TracebackLocation { + file_path: "helper.py", + line_number: OneIndexed( + 15, + ), + }, + ) + "#); } #[test] fn test_get_traceback_location_empty() { - let traceback = ""; - let location = get_traceback_location(traceback); - assert_eq!(location, None); + insta::assert_debug_snapshot!(get_traceback_location(""), @"None"); } #[test] fn test_get_traceback_location_no_file_lines() { - let traceback = "Exception: Test error"; - let location = get_traceback_location(traceback); - assert_eq!(location, None); + insta::assert_debug_snapshot!(get_traceback_location("Exception: Test error"), @"None"); } } @@ -285,45 +293,58 @@ ValueError: Invalid value"#; #[test] fn test_calculate_line_range_first_line() { let source = "line 1\nline 2\nline 3"; - let range = calculate_line_range(source, OneIndexed::new(1).unwrap()); - assert_eq!(range, Some(TextRange::new(0.into(), 6.into()))); + insta::assert_debug_snapshot!(calculate_line_range(source, OneIndexed::new(1).unwrap()), @" + Some( + 0..6, + ) + "); } #[test] fn test_calculate_line_range_middle_line() { let source = "line 1\nline 2\nline 3"; - let range = calculate_line_range(source, OneIndexed::new(2).unwrap()); - assert_eq!(range, Some(TextRange::new(7.into(), 13.into()))); + insta::assert_debug_snapshot!(calculate_line_range(source, OneIndexed::new(2).unwrap()), @" + Some( + 7..13, + ) + "); } #[test] fn test_calculate_line_range_last_line() { let source = "line 1\nline 2\nline 3"; - let range = calculate_line_range(source, OneIndexed::new(3).unwrap()); - assert_eq!(range, Some(TextRange::new(14.into(), 20.into()))); + insta::assert_debug_snapshot!(calculate_line_range(source, OneIndexed::new(3).unwrap()), @" + Some( + 14..20, + ) + "); } + /// "indented line" is 13 characters, starting at position 11 (after 4 spaces), + /// and leading/trailing whitespace should be trimmed from the range. #[test] fn test_calculate_line_range_with_whitespace() { let source = "line 1\n indented line\nline 3"; - let range = calculate_line_range(source, OneIndexed::new(2).unwrap()); - // Should trim leading/trailing whitespace - // "indented line" is 13 characters, starting at position 11 (after 4 spaces) - assert_eq!(range, Some(TextRange::new(11.into(), 24.into()))); + insta::assert_debug_snapshot!(calculate_line_range(source, OneIndexed::new(2).unwrap()), @" + Some( + 11..24, + ) + "); } #[test] fn test_calculate_line_range_out_of_bounds() { let source = "line 1\nline 2"; - let range = calculate_line_range(source, OneIndexed::new(10).unwrap()); - assert_eq!(range, None); + insta::assert_debug_snapshot!(calculate_line_range(source, OneIndexed::new(10).unwrap()), @"None"); } #[test] fn test_calculate_line_range_single_line() { - let source = "single line"; - let range = calculate_line_range(source, OneIndexed::new(1).unwrap()); - assert_eq!(range, Some(TextRange::new(0.into(), 11.into()))); + insta::assert_debug_snapshot!(calculate_line_range("single line", OneIndexed::new(1).unwrap()), @" + Some( + 0..11, + ) + "); } } } From 0ba33f21dbc1884ad21c08cb492de8c8bbc4ae51 Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sun, 12 Apr 2026 01:16:22 +0100 Subject: [PATCH 07/11] fix(ci): regenerate Cargo.lock and normalize path snapshots for Windows The cherry-picked karva_diagnostic insta dev-dep needed a Cargo.lock update that didn't carry across. Path snapshots in karva_project also need to be normalized to forward slashes via a small posix() helper since Windows formats Utf8PathBuf with backslashes. --- Cargo.lock | 1 + crates/karva_project/src/path/utils.rs | 30 +++++++++++++++----------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 74777c45..1443f130 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1176,6 +1176,7 @@ version = "0.0.0" dependencies = [ "camino", "colored 3.1.1", + "insta", "karva_logging", "karva_python_semantic", "pyo3", diff --git a/crates/karva_project/src/path/utils.rs b/crates/karva_project/src/path/utils.rs index 4346e79e..3186de50 100644 --- a/crates/karva_project/src/path/utils.rs +++ b/crates/karva_project/src/path/utils.rs @@ -40,56 +40,62 @@ mod tests { use super::*; + /// Render the path with forward slashes so the same snapshot pins down + /// behaviour on Windows (which would otherwise emit backslashes). + fn posix(path: &Utf8Path) -> String { + path.as_str().replace('\\', "/") + } + #[test] fn relative_path_is_joined_to_cwd() { - assert_snapshot!(absolute("foo/bar", "/home/user"), @"/home/user/foo/bar"); + assert_snapshot!(posix(&absolute("foo/bar", "/home/user")), @"/home/user/foo/bar"); } #[test] fn absolute_path_ignores_cwd() { - assert_snapshot!(absolute("/absolute/path", "/home/user"), @"/absolute/path"); + assert_snapshot!(posix(&absolute("/absolute/path", "/home/user")), @"/absolute/path"); } #[test] fn parent_dir_pops_component() { - assert_snapshot!(absolute("../sibling", "/home/user"), @"/home/sibling"); + assert_snapshot!(posix(&absolute("../sibling", "/home/user")), @"/home/sibling"); } #[test] fn current_dir_is_ignored() { - assert_snapshot!(absolute("./foo", "/home/user"), @"/home/user/foo"); + assert_snapshot!(posix(&absolute("./foo", "/home/user")), @"/home/user/foo"); } #[test] fn mixed_components() { - assert_snapshot!(absolute("foo/../bar/./baz", "/cwd"), @"/cwd/bar/baz"); + assert_snapshot!(posix(&absolute("foo/../bar/./baz", "/cwd")), @"/cwd/bar/baz"); } #[test] fn empty_relative_path_returns_cwd() { - assert_snapshot!(absolute("", "/home/user"), @"/home/user"); + assert_snapshot!(posix(&absolute("", "/home/user")), @"/home/user"); } #[test] fn leading_parent_pops_cwd() { - assert_snapshot!(absolute("../../other", "/home/user"), @"/other"); + assert_snapshot!(posix(&absolute("../../other", "/home/user")), @"/other"); } /// `Utf8PathBuf::pop` on `/` returns false, so extra `..` components /// must not escape the filesystem root. #[test] fn parent_past_root_stays_at_root() { - assert_snapshot!(absolute("../..", "/"), @"/"); + assert_snapshot!(posix(&absolute("../..", "/")), @"/"); } #[test] fn unicode_path_components_are_preserved() { - assert_snapshot!(absolute("カルヴァ/tests", "/home/ユーザー"), @"/home/ユーザー/カルヴァ/tests"); + assert_snapshot!(posix(&absolute("カルヴァ/tests", "/home/ユーザー")), @"/home/ユーザー/カルヴァ/tests"); } #[test] fn path_with_spaces_is_preserved() { - assert_snapshot!(absolute("my tests/file.py", "/home/my user"), @"/home/my user/my tests/file.py"); + assert_snapshot!(posix(&absolute("my tests/file.py", "/home/my user")), @"/home/my user/my tests/file.py"); } /// `camino` components strip trailing slashes, so the result should @@ -99,11 +105,11 @@ mod tests { let with = absolute("foo/bar/", "/cwd"); let without = absolute("foo/bar", "/cwd"); assert_eq!(with, without); - assert_snapshot!(with, @"/cwd/foo/bar"); + assert_snapshot!(posix(&with), @"/cwd/foo/bar"); } #[test] fn dot_only_path_is_cwd() { - assert_snapshot!(absolute(".", "/home/user"), @"/home/user"); + assert_snapshot!(posix(&absolute(".", "/home/user")), @"/home/user"); } } From 8badc5e1cd15096808dabd961d3d145c0dffc1b9 Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sun, 12 Apr 2026 14:19:03 +0100 Subject: [PATCH 08/11] fix(ci): filter karva.exe to karva in test snapshots for Windows Clap embeds the binary name in help/error output. On Windows this includes the .exe suffix, causing snapshot mismatches. The test infrastructure already normalizes other platform differences (backslash paths, ANSI codes), so adding one more filter is the natural fix. --- crates/karva/tests/it/common/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/karva/tests/it/common/mod.rs b/crates/karva/tests/it/common/mod.rs index 18f0c1ef..1da708f9 100644 --- a/crates/karva/tests/it/common/mod.rs +++ b/crates/karva/tests/it/common/mod.rs @@ -68,6 +68,7 @@ impl TestContext { settings.add_filter(r"run-\d+", "run-[TIMESTAMP]"); settings.add_filter(r"[-─]{30,}", "[LONG-LINE]"); settings.add_filter(r"karva \d+\.\d+\.\d+[a-zA-Z0-9._-]*", "karva [VERSION]"); + settings.add_filter(r"karva\.exe", "karva"); let settings_scope = settings.bind_to_scope(); From 4567a4fda0e412d6484638ae92241fa64d87cb51 Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Mon, 13 Apr 2026 00:19:06 +0100 Subject: [PATCH 09/11] . --- crates/karva_cache/src/cache.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/crates/karva_cache/src/cache.rs b/crates/karva_cache/src/cache.rs index 52dc81ff..155ba4f2 100644 --- a/crates/karva_cache/src/cache.rs +++ b/crates/karva_cache/src/cache.rs @@ -386,16 +386,9 @@ mod tests { let cache = Cache::new(&cache_dir, &run_hash); let results = cache.aggregate_results().unwrap(); - assert_debug_snapshot!( - (results.stats.passed(), results.stats.failed(), results.stats.skipped()), - @r" - ( - 5, - 1, - 1, - ) - " - ); + assert_eq!(results.stats.passed(), 5); + assert_eq!(results.stats.failed(), 1); + assert_eq!(results.stats.skipped(), 1); } #[test] @@ -410,8 +403,8 @@ mod tests { let cache = Cache::new(&cache_dir, &run_hash); let results = cache.aggregate_results().unwrap(); - assert_debug_snapshot!(results.stats.total(), @"0"); - assert_snapshot!(results.diagnostics, @""); + assert_eq!(results.stats.total(), 0); + assert!(results.diagnostics.is_empty()); } #[test] From 5f9c88522037193d29f5525561a7239eced3be4c Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Mon, 13 Apr 2026 00:19:20 +0100 Subject: [PATCH 10/11] . --- crates/karva_cache/src/cache.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/karva_cache/src/cache.rs b/crates/karva_cache/src/cache.rs index 155ba4f2..1ae1dbe6 100644 --- a/crates/karva_cache/src/cache.rs +++ b/crates/karva_cache/src/cache.rs @@ -319,7 +319,7 @@ mod tests { use std::fs; use camino::Utf8PathBuf; - use insta::{assert_debug_snapshot, assert_snapshot}; + use insta::assert_debug_snapshot; use super::*; From 17610452fa79483264244c9d5e43aaf1490ad82f Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Mon, 13 Apr 2026 00:33:34 +0100 Subject: [PATCH 11/11] . --- crates/karva/tests/it/basic.rs | 54 ----------------------- crates/karva/tests/it/last_failed.rs | 32 -------------- crates/karva_metadata/src/options.rs | 64 ---------------------------- 3 files changed, 150 deletions(-) diff --git a/crates/karva/tests/it/basic.rs b/crates/karva/tests/it/basic.rs index 80744405..2193647e 100644 --- a/crates/karva/tests/it/basic.rs +++ b/crates/karva/tests/it/basic.rs @@ -1573,36 +1573,6 @@ def test_third(): "#); } -#[test] -fn test_dry_run_nested_packages() { - let context = TestContext::with_files([ - ( - "tests/test_root.py", - r" -def test_root(): pass - ", - ), - ( - "tests/sub/test_nested.py", - r" -def test_nested(): pass - ", - ), - ]); - - assert_cmd_snapshot!(context.command().arg("--dry-run"), @r" - success: true - exit_code: 0 - ----- stdout ----- - tests.sub.test_nested::test_nested - tests.test_root::test_root - - 2 tests collected - - ----- stderr ----- - "); -} - #[test] fn test_show_python_output() { let context = TestContext::with_file( @@ -2077,30 +2047,6 @@ fn test_durations_invalid_value() { "); } -/// `--dry-run` with `--num-workers` still only collects — it should not spawn workers. -#[test] -fn test_dry_run_with_num_workers_does_not_spawn() { - let context = TestContext::with_file( - "test.py", - r" -def test_1(): pass -def test_2(): pass -", - ); - - assert_cmd_snapshot!(context.command().args(["--dry-run", "--num-workers", "4"]), @r" - success: true - exit_code: 0 - ----- stdout ----- - test::test_1 - test::test_2 - - 2 tests collected - - ----- stderr ----- - "); -} - /// When `--fail-fast` and `--no-fail-fast` are mixed, clap's `overrides_with` /// wires them so that whichever flag appears last wins. #[test] diff --git a/crates/karva/tests/it/last_failed.rs b/crates/karva/tests/it/last_failed.rs index 0adc9f9b..a1c747df 100644 --- a/crates/karva/tests/it/last_failed.rs +++ b/crates/karva/tests/it/last_failed.rs @@ -172,38 +172,6 @@ def test_fail_b(): assert False "); } -/// `--dry-run` currently ignores `--last-failed` and prints every discovered -/// test. Pinning this so the behavior is intentional — see PR follow-ups. -#[test] -fn last_failed_with_dry_run_shows_all_tests() { - let context = TestContext::with_file( - "test_a.py", - " -def test_pass(): pass -def test_fail(): assert False - ", - ); - - context.command_no_parallel().output().unwrap(); - - assert_cmd_snapshot!( - context - .command_no_parallel() - .args(["--last-failed", "--dry-run"]), - @" - success: true - exit_code: 0 - ----- stdout ----- - test_a::test_fail - test_a::test_pass - - 2 tests collected - - ----- stderr ----- - " - ); -} - /// A filter combined with `--last-failed` intersects: tests that were in the /// last-failed set but are now filtered out are skipped. #[test] diff --git a/crates/karva_metadata/src/options.rs b/crates/karva_metadata/src/options.rs index 71c0f191..bec623a9 100644 --- a/crates/karva_metadata/src/options.rs +++ b/crates/karva_metadata/src/options.rs @@ -306,23 +306,6 @@ mod tests { use super::*; - #[test] - fn to_settings_defaults_when_empty() { - assert_debug_snapshot!(TestOptions::default().to_settings(), @r#" - TestSettings { - test_function_prefix: "test", - max_fail: MaxFail( - None, - ), - try_import_fixtures: false, - retry: 0, - filter: FiltersetSet { - filters: [], - }, - } - "#); - } - #[test] fn to_settings_fail_fast_true_becomes_max_fail_one() { let options = TestOptions { @@ -367,53 +350,6 @@ mod tests { "); } - #[test] - fn from_toml_str_parses_nested_sections() { - let toml = r#" -[test] -test-function-prefix = "check" -max-fail = 3 -retry = 2 - -[terminal] -output-format = "concise" -show-python-output = true - -[src] -respect-ignore-files = false -include = ["tests", "more"] -"#; - let options = Options::from_toml_str(toml).expect("parse"); - assert_debug_snapshot!(options.to_settings(), @r#" - ProjectSettings { - terminal: TerminalSettings { - output_format: Concise, - show_python_output: true, - }, - src: SrcSettings { - respect_ignore_files: false, - include_paths: [ - "tests", - "more", - ], - }, - test: TestSettings { - test_function_prefix: "check", - max_fail: MaxFail( - Some( - 3, - ), - ), - try_import_fixtures: false, - retry: 2, - filter: FiltersetSet { - filters: [], - }, - }, - } - "#); - } - #[test] fn from_toml_str_rejects_unknown_key() { let toml = r"