From 93c0f7f2a6cee8c5ff56208817ac7d1b014cada1 Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sat, 11 Apr 2026 19:38:42 +0100 Subject: [PATCH 1/7] 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 | 67 ------------------- crates/karva/tests/it/last_failed.rs | 41 ++++++++++++ .../src/function_kind.rs | 15 ----- crates/karva_runner/src/orchestration.rs | 47 ------------- 4 files changed, 41 insertions(+), 129 deletions(-) diff --git a/crates/karva/tests/it/basic.rs b/crates/karva/tests/it/basic.rs index a75370ca..7d0f3ba7 100644 --- a/crates/karva/tests/it/basic.rs +++ b/crates/karva/tests/it/basic.rs @@ -1650,73 +1650,6 @@ 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() { - let context = TestContext::with_files([ - ( - "test_a.py", - r" -def test_a_fail(): - assert False - ", - ), - ( - "test_b.py", - r" -def test_b_pass(): - assert True - ", - ), - ]); - - 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"); -} - #[test] fn test_dry_run_nested_packages() { let context = TestContext::with_files([ 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 41109f41..daa47380 100644 --- a/crates/karva_runner/src/orchestration.rs +++ b/crates/karva_runner/src/orchestration.rs @@ -434,50 +434,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 749efb4a874b1cadebddb4c32268ed393ee32853 Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sat, 11 Apr 2026 19:48:47 +0100 Subject: [PATCH 2/7] 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 7efd2a00..9fb75325 100644 --- a/crates/karva_metadata/src/options.rs +++ b/crates/karva_metadata/src/options.rs @@ -294,6 +294,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 78ad0ac708c50689e485c1c550524ea8916d82d4 Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sat, 11 Apr 2026 19:53:45 +0100 Subject: [PATCH 3/7] 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 7d0f3ba7..9e2b8e5b 100644 --- a/crates/karva/tests/it/basic.rs +++ b/crates/karva/tests/it/basic.rs @@ -1853,3 +1853,467 @@ def test_2(): pass ----- 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 c312ef1e825377d64ba8d1d82b96b9d294aa6437 Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sat, 11 Apr 2026 20:02:14 +0100 Subject: [PATCH 4/7] 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 9e2b8e5b..ea6964a0 100644 --- a/crates/karva/tests/it/basic.rs +++ b/crates/karva/tests/it/basic.rs @@ -2258,23 +2258,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 268008e11ebdcd3ad10e61461e623cc17597b271 Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sat, 11 Apr 2026 20:13:38 +0100 Subject: [PATCH 5/7] test(logging): snapshot format_duration assertions Converts the hand-written equality checks in `format_duration` and `format_duration_bracketed` tests to inline snapshots so intentional format changes can be updated in bulk via `cargo insta review`. --- Cargo.lock | 3 +++ crates/karva_logging/Cargo.toml | 3 +++ crates/karva_logging/src/time.rs | 36 +++++++++++++------------------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a2ca48ee..7adb5749 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1175,6 +1175,7 @@ version = "0.0.0" dependencies = [ "camino", "colored 3.1.1", + "insta", "karva_logging", "karva_python_semantic", "pyo3", @@ -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", 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..00f5f930 100644 --- a/crates/karva_logging/src/time.rs +++ b/crates/karva_logging/src/time.rs @@ -19,58 +19,52 @@ mod tests { #[test] fn format_duration_zero_is_zero_ms() { - assert_eq!(format_duration(Duration::ZERO), "0ms"); + insta::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"); + insta::assert_snapshot!(format_duration(Duration::from_micros(500)), @"0ms"); + insta::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"); + insta::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"); + insta::assert_snapshot!(format_duration(Duration::from_millis(1000)), @"1000ms"); + insta::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"); + insta::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"); + insta::assert_snapshot!(format_duration(Duration::from_millis(2346)), @"2.35s"); + insta::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"); + insta::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]" - ); + insta::assert_snapshot!(format_duration_bracketed(Duration::ZERO), @"[ 0.000s]"); + insta::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]" - ); + insta::assert_snapshot!(format_duration_bracketed(Duration::from_secs(12345)), @"[12345.000s]"); } } From 27cb3391eb31e7ab7f7f29f07d9883b411856efb Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sat, 11 Apr 2026 20:13:44 +0100 Subject: [PATCH 6/7] 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 92e25c2905c5cf92e41c0954c52bfd17cc1c9d4f Mon Sep 17 00:00:00 2001 From: MatthewMckee4 Date: Sat, 11 Apr 2026 20:13:50 +0100 Subject: [PATCH 7/7] test(metadata): snapshot Options and combine merge assertions Converts the TOML parsing and `Combine` merge tests on `Options`, `TestOptions`, and `SrcOptions` to debug snapshots of the full struct. The hand-written per-field asserts previously checked a handful of fields in each merged struct; the snapshot form pins down every field at once so intentional schema changes regenerate cleanly. --- crates/karva_metadata/Cargo.toml | 3 + crates/karva_metadata/src/options.rs | 134 ++++++++++++++++++++++----- 2 files changed, 112 insertions(+), 25 deletions(-) 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 9fb75325..54e8d510 100644 --- a/crates/karva_metadata/src/options.rs +++ b/crates/karva_metadata/src/options.rs @@ -356,14 +356,52 @@ 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"]); + insta::assert_debug_snapshot!(options, @r#" + Options { + src: Some( + SrcOptions { + respect_ignore_files: Some( + false, + ), + include: Some( + [ + "tests", + "more", + ], + ), + }, + ), + terminal: Some( + TerminalOptions { + output_format: Some( + Concise, + ), + show_python_output: Some( + true, + ), + }, + ), + test: Some( + TestOptions { + test_function_prefix: Some( + "check", + ), + fail_fast: None, + max_fail: Some( + MaxFail( + Some( + 3, + ), + ), + ), + try_import_fixtures: None, + retry: Some( + 2, + ), + }, + ), + } + "#); } #[test] @@ -422,10 +460,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)); + insta::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] @@ -437,12 +486,24 @@ 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)); + insta::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 { @@ -453,11 +514,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)); + insta::assert_debug_snapshot!(cli.combine(file), @r#" + SrcOptions { + respect_ignore_files: Some( + false, + ), + include: Some( + [ + "file_only", + "cli_only", + ], + ), + } + "#); } #[test] @@ -478,10 +547,25 @@ 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)); + insta::assert_debug_snapshot!(overrides.apply_to(file_options), @r#" + Options { + src: None, + terminal: None, + test: Some( + TestOptions { + test_function_prefix: Some( + "cli", + ), + fail_fast: None, + max_fail: None, + try_import_fixtures: None, + retry: Some( + 2, + ), + }, + ), + } + "#); } }