From 92260297b8d7add5b9402deeedf715becc788816 Mon Sep 17 00:00:00 2001 From: Eric Soler Date: Sat, 3 Oct 2026 15:38:51 +0000 Subject: [PATCH 1/2] fix: preserve flame graphs when export fails --- src/main.rs | 21 ++++++++++++--- tests/stack_commands.rs | 58 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index 12ab1d60..04e16528 100644 --- a/src/main.rs +++ b/src/main.rs @@ -567,10 +567,25 @@ fn write_flamegraph( ) -> Result { let stacks = String::from_utf8(stacks).expect("folded stacks are valid UTF-8"); if let Some(path) = output { - let file = fs::File::create(&path) - .with_context(|| format!("Could not create flame graph {}", path.display()))?; - inferno::flamegraph::from_lines(options, stacks.lines(), file) + let parent = path + .parent() + .filter(|parent| !parent.as_os_str().is_empty()) + .unwrap_or_else(|| Path::new(".")); + let mut temporary = tempfile::NamedTempFile::new_in(parent).with_context(|| { + format!( + "Could not create temporary flame graph for {}", + path.display() + ) + })?; + inferno::flamegraph::from_lines(options, stacks.lines(), &mut temporary) .with_context(|| format!("Could not write flame graph {}", path.display()))?; + temporary.as_file_mut().flush()?; + temporary.as_file().sync_all()?; + temporary + .into_temp_path() + .persist(&path) + .map_err(|err| err.error) + .with_context(|| format!("Could not install flame graph {}", path.display()))?; Ok(path) } else { let mut file = tempfile::Builder::new() diff --git a/tests/stack_commands.rs b/tests/stack_commands.rs index 5d11d018..3650e767 100644 --- a/tests/stack_commands.rs +++ b/tests/stack_commands.rs @@ -48,3 +48,61 @@ fn stacks_and_flamegraph_commands_use_the_folded_output() { assert!(svg.contains("Path:")); assert!(svg.contains("payload")); } + +#[test] +fn failed_flamegraph_export_preserves_the_previous_report() { + let fixture = tempfile::tempdir().unwrap(); + let input = fixture.path().join("input"); + let empty = fixture.path().join("empty"); + let report = fixture.path().join("usage.svg"); + fs::create_dir(&input).unwrap(); + fs::create_dir(&empty).unwrap(); + fs::write(input.join("payload"), b"data").unwrap(); + + let export = |input: &str, title: &str| { + Command::new(env!("CARGO_BIN_EXE_dua")) + .current_dir(fixture.path()) + .args([ + "flamegraph", + "--output", + "usage.svg", + "--title", + title, + input, + ]) + .output() + .unwrap() + }; + assert!(export("input", "Original report").status.success()); + let original = fs::read(&report).unwrap(); + + let failed = export("empty", "Failed report"); + assert!(!failed.status.success()); + assert!(String::from_utf8_lossy(&failed.stderr).contains("No stack counts found")); + assert_eq!(fs::read(&report).unwrap(), original); + assert_eq!(fs::read_dir(fixture.path()).unwrap().count(), 3); + + // A successful export must still replace an existing report. + assert!(export("input", "Updated report").status.success()); + let updated = fs::read_to_string(&report).unwrap(); + assert!(updated.contains(">Updated report")); + assert!(!updated.contains("Original report")); + assert_eq!(fs::read_dir(fixture.path()).unwrap().count(), 3); +} + +#[test] +fn failed_flamegraph_export_does_not_leave_a_partial_report() { + let fixture = tempfile::tempdir().unwrap(); + fs::create_dir(fixture.path().join("empty")).unwrap(); + + let output = Command::new(env!("CARGO_BIN_EXE_dua")) + .current_dir(fixture.path()) + .args(["flamegraph", "--output", "usage.svg", "empty"]) + .output() + .unwrap(); + + assert!(!output.status.success()); + assert!(String::from_utf8_lossy(&output.stderr).contains("No stack counts found")); + assert!(!fixture.path().join("usage.svg").exists()); + assert_eq!(fs::read_dir(fixture.path()).unwrap().count(), 1); +} From c0c749a483d30a67a09f406e5d5b0088639121f6 Mon Sep 17 00:00:00 2001 From: Byron Date: Mon, 5 Oct 2026 04:49:54 +0200 Subject: [PATCH 2/2] review - simplify. Notably a lot of tests were removed as well, keeping only the essential one. Assisted-by: GPT 6.1 Sol Co-authored-by: GPT 6.1 Sol --- src/main.rs | 43 +++++++++++++++++------------- tests/stack_commands.rs | 58 ----------------------------------------- 2 files changed, 25 insertions(+), 76 deletions(-) diff --git a/src/main.rs b/src/main.rs index 04e16528..3f6f0fab 100644 --- a/src/main.rs +++ b/src/main.rs @@ -565,27 +565,16 @@ fn write_flamegraph( output: Option, options: &mut inferno::flamegraph::Options<'_>, ) -> Result { + // Zero-sized entries are omitted from folded stacks, and inferno writes an error SVG for empty input. + if stacks.is_empty() { + bail!("No disk usage to render as a flame graph"); + } let stacks = String::from_utf8(stacks).expect("folded stacks are valid UTF-8"); if let Some(path) = output { - let parent = path - .parent() - .filter(|parent| !parent.as_os_str().is_empty()) - .unwrap_or_else(|| Path::new(".")); - let mut temporary = tempfile::NamedTempFile::new_in(parent).with_context(|| { - format!( - "Could not create temporary flame graph for {}", - path.display() - ) - })?; - inferno::flamegraph::from_lines(options, stacks.lines(), &mut temporary) + let file = fs::File::create(&path) + .with_context(|| format!("Could not create flame graph {}", path.display()))?; + inferno::flamegraph::from_lines(options, stacks.lines(), file) .with_context(|| format!("Could not write flame graph {}", path.display()))?; - temporary.as_file_mut().flush()?; - temporary.as_file().sync_all()?; - temporary - .into_temp_path() - .persist(&path) - .map_err(|err| err.error) - .with_context(|| format!("Could not install flame graph {}", path.display()))?; Ok(path) } else { let mut file = tempfile::Builder::new() @@ -1176,6 +1165,24 @@ mod tests { ); } + #[test] + fn empty_flamegraphs_are_rejected_with_and_without_explicit_output() { + let dir = tempfile::tempdir().expect("temporary directory"); + for output in [None, Some(dir.path().join("usage.svg"))] { + let error = write_flamegraph( + Vec::new(), + output, + &mut inferno::flamegraph::Options::default(), + ) + .expect_err("empty stacks cannot be rendered"); + assert_eq!( + error.to_string(), + "No disk usage to render as a flame graph" + ); + } + assert_eq!(fs::read_dir(dir.path()).unwrap().count(), 0); + } + #[test] fn writes_explicit_and_temporary_flamegraphs() { let dir = tempfile::tempdir().expect("temporary directory"); diff --git a/tests/stack_commands.rs b/tests/stack_commands.rs index 3650e767..5d11d018 100644 --- a/tests/stack_commands.rs +++ b/tests/stack_commands.rs @@ -48,61 +48,3 @@ fn stacks_and_flamegraph_commands_use_the_folded_output() { assert!(svg.contains("Path:")); assert!(svg.contains("payload")); } - -#[test] -fn failed_flamegraph_export_preserves_the_previous_report() { - let fixture = tempfile::tempdir().unwrap(); - let input = fixture.path().join("input"); - let empty = fixture.path().join("empty"); - let report = fixture.path().join("usage.svg"); - fs::create_dir(&input).unwrap(); - fs::create_dir(&empty).unwrap(); - fs::write(input.join("payload"), b"data").unwrap(); - - let export = |input: &str, title: &str| { - Command::new(env!("CARGO_BIN_EXE_dua")) - .current_dir(fixture.path()) - .args([ - "flamegraph", - "--output", - "usage.svg", - "--title", - title, - input, - ]) - .output() - .unwrap() - }; - assert!(export("input", "Original report").status.success()); - let original = fs::read(&report).unwrap(); - - let failed = export("empty", "Failed report"); - assert!(!failed.status.success()); - assert!(String::from_utf8_lossy(&failed.stderr).contains("No stack counts found")); - assert_eq!(fs::read(&report).unwrap(), original); - assert_eq!(fs::read_dir(fixture.path()).unwrap().count(), 3); - - // A successful export must still replace an existing report. - assert!(export("input", "Updated report").status.success()); - let updated = fs::read_to_string(&report).unwrap(); - assert!(updated.contains(">Updated report")); - assert!(!updated.contains("Original report")); - assert_eq!(fs::read_dir(fixture.path()).unwrap().count(), 3); -} - -#[test] -fn failed_flamegraph_export_does_not_leave_a_partial_report() { - let fixture = tempfile::tempdir().unwrap(); - fs::create_dir(fixture.path().join("empty")).unwrap(); - - let output = Command::new(env!("CARGO_BIN_EXE_dua")) - .current_dir(fixture.path()) - .args(["flamegraph", "--output", "usage.svg", "empty"]) - .output() - .unwrap(); - - assert!(!output.status.success()); - assert!(String::from_utf8_lossy(&output.stderr).contains("No stack counts found")); - assert!(!fixture.path().join("usage.svg").exists()); - assert_eq!(fs::read_dir(fixture.path()).unwrap().count(), 1); -}