Skip to content

Commit 991988e

Browse files
Split print_test_output() into focused helpers (#536)
## Summary `print_test_output()` was a 74-line function in `karva/src/commands/test/mod.rs` that mixed four concerns: diagnostic printing, duration display, blank-line spacing decisions, and result summary formatting. This made the spacing logic especially hard to follow, since blank lines depended on what the *other* sections had printed. This PR extracts two focused helpers: - **`print_diagnostics_section()`** — prints discovery diagnostics and regular diagnostics, each with their concise-mode trailing newline. The caller no longer needs to know about the internal structure of the two diagnostic blocks. - **`print_durations_section()`** — prints the N-slowest-tests table and returns `bool` indicating whether anything was printed. This lets the caller's spacing logic read as a simple condition (`!has_diagnostics && !durations_printed`) instead of recomputing the durations check inline. With these extracted, `print_test_output()` shrinks to a clear sequence: leading blank line (if diagnostics), diagnostics, durations, trailing blank line (if nothing else printed one), result summary. Also simplifies `test()` by resolving `num_workers` in one step (folding the `no_parallel` check into the fallback) and moving the `--watch`/`--dry-run` conflict check before the local variable extraction block. Terminal output is unchanged — all 685 snapshot and integration tests pass. ## Test plan - [x] `just test` — all 685 tests pass - [x] `uvx prek run -a` — all pre-commit checks pass (including clippy) - [x] No snapshot changes, confirming terminal output is identical 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 9c29a42 commit 991988e

File tree

1 file changed

+68
-53
lines changed
  • crates/karva/src/commands/test

1 file changed

+68
-53
lines changed

crates/karva/src/commands/test/mod.rs

Lines changed: 68 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
mod watch;
22

3+
use std::collections::HashMap;
34
use std::fmt::Write;
4-
use std::time::Instant;
5+
use std::time::{Duration, Instant};
56

67
use anyhow::Result;
78
use karva_cache::AggregatedResults;
89
use karva_cli::{OutputFormat, TestCommand};
910
use karva_collector::CollectedPackage;
10-
use karva_logging::{Printer, set_colored_override, setup_tracing};
11+
use karva_logging::{Printer, Stdout, set_colored_override, setup_tracing};
1112
use karva_metadata::filter::{NameFilterSet, TagFilterSet};
1213
use karva_metadata::{ProjectMetadata, ProjectOptionsOverrides};
1314
use karva_project::Project;
@@ -44,19 +45,22 @@ pub fn test(args: TestCommand) -> Result<ExitStatus> {
4445
ProjectMetadata::discover(&cwd, python_version)?
4546
};
4647

47-
let sub_command = args.sub_command.clone();
48+
if args.watch && args.dry_run {
49+
anyhow::bail!("`--watch` and `--dry-run` cannot be used together");
50+
}
4851

49-
let no_parallel = args.no_parallel.unwrap_or(false);
50-
let no_cache = args.no_cache.unwrap_or(false);
51-
let num_workers = args.num_workers;
52+
let sub_command = args.sub_command.clone();
5253
let dry_run = args.dry_run;
5354
let watch = args.watch;
54-
let last_failed = args.last_failed;
5555
let durations = args.durations;
56-
57-
if watch && dry_run {
58-
anyhow::bail!("`--watch` and `--dry-run` cannot be used together");
59-
}
56+
let last_failed = args.last_failed;
57+
let no_cache = args.no_cache.unwrap_or(false);
58+
let num_workers = if args.no_parallel.unwrap_or(false) {
59+
1
60+
} else {
61+
args.num_workers
62+
.unwrap_or_else(|| karva_static::max_parallelism().get())
63+
};
6064

6165
let project_options_overrides = ProjectOptionsOverrides::new(config_file, args.into_options());
6266
project_metadata.apply_overrides(&project_options_overrides);
@@ -69,12 +73,6 @@ pub fn test(args: TestCommand) -> Result<ExitStatus> {
6973
return Ok(ExitStatus::Success);
7074
}
7175

72-
let num_workers = if no_parallel {
73-
1
74-
} else {
75-
num_workers.unwrap_or_else(|| karva_static::max_parallelism().get())
76-
};
77-
7876
TagFilterSet::new(&sub_command.tag_expressions)?;
7977
NameFilterSet::new(&sub_command.name_patterns)?;
8078

@@ -109,7 +107,7 @@ pub fn test(args: TestCommand) -> Result<ExitStatus> {
109107
}
110108
}
111109

112-
/// Print test output.
110+
/// Print test output: diagnostics, durations, and result summary.
113111
pub fn print_test_output(
114112
printer: Printer,
115113
start_time: Instant,
@@ -118,16 +116,35 @@ pub fn print_test_output(
118116
durations: Option<usize>,
119117
) -> Result<()> {
120118
let mut stdout = printer.stream_for_details().lock();
121-
122119
let is_concise = matches!(output_format, Some(OutputFormat::Concise));
123120

124-
if (!result.diagnostics.is_empty() || !result.discovery_diagnostics.is_empty())
125-
&& result.stats.total() > 0
126-
&& stdout.is_enabled()
127-
{
121+
let has_diagnostics =
122+
!result.diagnostics.is_empty() || !result.discovery_diagnostics.is_empty();
123+
124+
if has_diagnostics && result.stats.total() > 0 && stdout.is_enabled() {
125+
writeln!(stdout)?;
126+
}
127+
128+
print_diagnostics_section(&mut stdout, result, is_concise)?;
129+
130+
let durations_printed = print_durations_section(&mut stdout, &result.durations, durations)?;
131+
132+
if !has_diagnostics && !durations_printed && result.stats.total() > 0 && stdout.is_enabled() {
128133
writeln!(stdout)?;
129134
}
130135

136+
let mut result_stdout = printer.stream_for_failure_summary().lock();
137+
write!(result_stdout, "{}", result.stats.display(start_time))?;
138+
139+
Ok(())
140+
}
141+
142+
/// Print discovery diagnostics and regular diagnostics, with concise-mode spacing.
143+
fn print_diagnostics_section(
144+
stdout: &mut Stdout,
145+
result: &AggregatedResults,
146+
is_concise: bool,
147+
) -> Result<()> {
131148
if !result.discovery_diagnostics.is_empty() {
132149
writeln!(stdout, "discovery diagnostics:")?;
133150
writeln!(stdout)?;
@@ -148,41 +165,39 @@ pub fn print_test_output(
148165
}
149166
}
150167

151-
if let Some(n) = durations
152-
&& n > 0
153-
&& !result.durations.is_empty()
154-
{
155-
let mut sorted: Vec<_> = result.durations.iter().collect();
156-
sorted.sort_by(|a, b| b.1.cmp(a.1));
157-
let count = n.min(sorted.len());
168+
Ok(())
169+
}
158170

159-
writeln!(stdout)?;
160-
writeln!(stdout, "{count} slowest tests:")?;
161-
for (name, duration) in sorted.into_iter().take(n) {
162-
writeln!(
163-
stdout,
164-
" {} ({})",
165-
name,
166-
karva_logging::time::format_duration(*duration)
167-
)?;
168-
}
169-
writeln!(stdout)?;
171+
/// Print the N slowest test durations. Returns whether anything was printed.
172+
fn print_durations_section(
173+
stdout: &mut Stdout,
174+
test_durations: &HashMap<String, Duration>,
175+
durations: Option<usize>,
176+
) -> Result<bool> {
177+
let Some(n) = durations else {
178+
return Ok(false);
179+
};
180+
if n == 0 || test_durations.is_empty() {
181+
return Ok(false);
170182
}
171183

172-
let durations_printed = durations.is_some_and(|n| n > 0 && !result.durations.is_empty());
173-
if (result.diagnostics.is_empty() && result.discovery_diagnostics.is_empty())
174-
&& !durations_printed
175-
&& result.stats.total() > 0
176-
&& stdout.is_enabled()
177-
{
178-
writeln!(stdout)?;
184+
let mut sorted: Vec<_> = test_durations.iter().collect();
185+
sorted.sort_by(|a, b| b.1.cmp(a.1));
186+
let count = n.min(sorted.len());
187+
188+
writeln!(stdout)?;
189+
writeln!(stdout, "{count} slowest tests:")?;
190+
for (name, duration) in sorted.into_iter().take(n) {
191+
writeln!(
192+
stdout,
193+
" {} ({})",
194+
name,
195+
karva_logging::time::format_duration(*duration)
196+
)?;
179197
}
198+
writeln!(stdout)?;
180199

181-
let mut result_stdout = printer.stream_for_failure_summary().lock();
182-
183-
write!(result_stdout, "{}", result.stats.display(start_time))?;
184-
185-
Ok(())
200+
Ok(true)
186201
}
187202

188203
/// Recursively collect test names from a `CollectedPackage` as `(module_name, function_name)` pairs.

0 commit comments

Comments
 (0)