diff --git a/package-lock.json b/package-lock.json index bda4b249..7c3bd780 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6477,6 +6477,7 @@ "integrity": "sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw==", "dev": true, "hasInstallScript": true, + "license": "MIT", "optional": true, "os": [ "darwin" diff --git a/src/data/common/utils.rs b/src/data/common/utils.rs index 1e55b550..52e2ecbe 100644 --- a/src/data/common/utils.rs +++ b/src/data/common/utils.rs @@ -1,5 +1,6 @@ -use anyhow::{Error, Result}; +use anyhow::{bail, Error, Result}; use log::error; +use regex::Regex; use std::collections::{HashMap, HashSet, VecDeque}; use std::fs; use std::fs::File; @@ -119,6 +120,33 @@ pub fn copy_graph_and_update_graph_data( } } +/// Returns the name of the first file in dir whose name matches the pattern regex but does +/// not match the optional exclude regex. +pub fn find_file(dir: &PathBuf, pattern: &str, exclude_pattern: Option<&str>) -> Result { + let regex = Regex::new(pattern)?; + let exclude_regex = exclude_pattern.map(Regex::new).transpose()?; + for entry in fs::read_dir(dir)? { + let filename = entry?.file_name().into_string().unwrap(); + if regex.is_match(&filename) + && !exclude_regex + .as_ref() + .is_some_and(|ex| ex.is_match(&filename)) + { + return Ok(filename); + } + } + match exclude_pattern { + Some(exclude_pattern) => bail!( + "Could not find any file matching /{pattern}/ (excluding /{exclude_pattern}/) in {}", + dir.display() + ), + None => bail!( + "Could not find any file matching /{pattern}/ in {}", + dir.display() + ), + } +} + /// Collects the paths of all files in a dir and returns a map from file names to file paths, /// if the file system read was successful pub fn collect_file_paths_in_dir(dir: &PathBuf) -> Result> { @@ -230,7 +258,98 @@ pub fn combine_value_ranges(value_ranges: Vec<(u64, u64)>) -> (u64, u64) { #[cfg(test)] mod utils_test { - use super::{combine_value_ranges, topological_sort}; + use super::{combine_value_ranges, find_file, topological_sort}; + use std::fs; + use std::path::PathBuf; + use tempfile::TempDir; + + #[test] + fn test_find_file_prefix_match() { + let dir = TempDir::new().unwrap(); + for f in &[ + "cpu_utilization.bin", + "other_cpu_utilization.bin", + "noise.txt", + ] { + fs::File::create(dir.path().join(f)).unwrap(); + } + let path = PathBuf::from(dir.path()); + // Anchored at the start with `^`. + assert_eq!( + find_file(&path, "^cpu_utilization", None).unwrap(), + "cpu_utilization.bin", + ); + // No match returns Err. + assert!(find_file(&path, "^missing", None).is_err()); + } + + #[test] + fn test_find_file_suffix_match() { + let dir = TempDir::new().unwrap(); + for f in &["data.bin", "data.bin.bak", "noise.txt"] { + fs::File::create(dir.path().join(f)).unwrap(); + } + let path = PathBuf::from(dir.path()); + // Anchored at the end with `$` (".bin" mid-name in "data.bin.bak" doesn't match). + assert_eq!(find_file(&path, r"\.bin$", None).unwrap(), "data.bin"); + // No match returns Err. + assert!(find_file(&path, r"\.missing$", None).is_err()); + } + + #[test] + fn test_find_file_excludes_substring_collision() { + // Regression test: the forward flamegraph lookup must not pick up + // `reverse-flamegraph.svg`, whose name also ends in `flamegraph.svg`. Create the files + // in both orders to defeat any reliance on directory read ordering. + for order in [ + ["flamegraph.svg", "reverse-flamegraph.svg"], + ["reverse-flamegraph.svg", "flamegraph.svg"], + ] { + let dir = TempDir::new().unwrap(); + for f in order { + fs::File::create(dir.path().join(f)).unwrap(); + } + let path = PathBuf::from(dir.path()); + // Forward: match `flamegraph.svg` but exclude the reverse variant. + assert_eq!( + find_file( + &path, + r"flamegraph\.svg$", + Some(r"reverse-flamegraph\.svg$") + ) + .unwrap(), + "flamegraph.svg", + ); + // Reverse: matches only the reverse variant. + assert_eq!( + find_file(&path, r"reverse-flamegraph\.svg$", None).unwrap(), + "reverse-flamegraph.svg", + ); + } + } + + #[test] + fn test_find_file_excludes_legacy_run_prefixed_names() { + // The same disambiguation must hold for the legacy `-flamegraph.svg` naming. + let dir = TempDir::new().unwrap(); + for f in &["myrun-flamegraph.svg", "myrun-reverse-flamegraph.svg"] { + fs::File::create(dir.path().join(f)).unwrap(); + } + let path = PathBuf::from(dir.path()); + assert_eq!( + find_file( + &path, + r"flamegraph\.svg$", + Some(r"reverse-flamegraph\.svg$") + ) + .unwrap(), + "myrun-flamegraph.svg", + ); + assert_eq!( + find_file(&path, r"reverse-flamegraph\.svg$", None).unwrap(), + "myrun-reverse-flamegraph.svg", + ); + } #[test] fn test_topological_sort_fixed_result() { diff --git a/src/data/java_profile.rs b/src/data/java_profile.rs index 5eba153e..64c5cacf 100644 --- a/src/data/java_profile.rs +++ b/src/data/java_profile.rs @@ -1,9 +1,7 @@ use crate::data::common::data_formats::{ AperfData, GraphData, GraphGroup, Profiler, ProfilingData, }; -use crate::data::common::utils::copy_graph_and_update_graph_data; -#[cfg(target_os = "linux")] -use crate::data::common::utils::get_data_name_from_type; +use crate::data::common::utils::{copy_graph_and_update_graph_data, find_file}; use crate::data::{Data, ProcessData}; use crate::profiling::{jfr, Profile}; use crate::visualizer::ReportParams; @@ -15,6 +13,7 @@ use std::collections::HashMap; use std::fs; #[cfg(target_os = "linux")] use { + crate::data::common::utils::get_data_name_from_type, crate::data::{CollectData, CollectorParams}, crate::PDError, log::debug, @@ -32,6 +31,10 @@ lazy_static! { pub static ref ASPROF_CHILDREN: Mutex> = Mutex::new(Vec::new()); } +fn java_profiler_data_filename(pid: &str) -> String { + format!("java_profiler_data_{}.json", pid) +} + #[derive(Serialize, Deserialize, Debug, Clone)] pub struct JavaProfileRaw { process_map: HashMap>, @@ -300,10 +303,7 @@ impl CollectData for JavaProfileRaw { // Generate heatmaps for each profiling type for metric in PROFILE_METRICS { - let html_path = data_dir.join(format!( - "{}-java-profile-{}-{}.html", - params.run_name, key, metric - )); + let html_path = data_dir.join(format!("java-profile-{}-{}.html", key, metric)); match Command::new("jfrconv") .args([ @@ -340,12 +340,6 @@ impl CollectData for JavaProfileRaw { } } - // Generate Profiler from JFR - let profiler_data_path = params.data_dir.join(format!( - "{}-java-profile-{}-profiler-data.json", - params.run_name, key - )); - let event_out_path_buf = params.data_dir.join(format!("parsed_jfr_events_{key}.out")); let events_out_path = if params.save_profile_events { @@ -358,11 +352,16 @@ impl CollectData for JavaProfileRaw { // so that the new flow is only executed in tests. Remove the guardrail // after the feature is ready to launch. if params.save_profile_events { + // Generate Profiler from JFR match jfr::build_java_profiler_data(&jfr_path, events_out_path) { Ok(mut profiler) => { profiler.metadata = jfr::parse_jfr_metadata(&metadata_json); if let Ok(json) = serde_json::to_string(&profiler) { - fs::write(&profiler_data_path, json).ok(); + fs::write( + params.data_dir.join(java_profiler_data_filename(key)), + json, + ) + .ok(); } } Err(e) => { @@ -371,17 +370,12 @@ impl CollectData for JavaProfileRaw { } } - let jfr_dest = - data_dir.join(format!("{}-java-profile-{}.jfr", params.run_name, key)); + let jfr_dest = data_dir.join(format!("java-profile-{}.jfr", key)); fs::copy(&jfr_path, jfr_dest).ok(); } } - let mut jps_map = File::create( - data_dir - .clone() - .join(format!("{}-jps-map.json", params.run_name)), - )?; + let mut jps_map = File::create(data_dir.clone().join("jps-map.json"))?; write!(jps_map, "{}", serde_json::to_string(&self.process_map)?)?; Ok(()) @@ -411,11 +405,17 @@ impl ProcessData for JavaProfile { // For backward compatibility let mut graph_data = GraphData::default(); - let processes_loc = params - .data_dir - .join(format!("{}-jps-map.json", params.run_name)); - let processes_json = - fs::read_to_string(processes_loc.to_str().unwrap()).unwrap_or_default(); + // Look up the jps map by suffix to support both the current `jps-map.json` and the + // legacy `-jps-map.json` naming. + let processes_json = match find_file(¶ms.data_dir, r"jps-map\.json$", None) { + Ok(processes_json_path) => { + fs::read_to_string(params.data_dir.join(processes_json_path)).unwrap_or_default() + } + Err(e) => { + error!("{e}"); + String::default() + } + }; let process_map: HashMap> = serde_json::from_str(&processes_json).unwrap_or_default(); @@ -446,34 +446,32 @@ impl ProcessData for JavaProfile { // Copy jfrconv-generated HTML graphs for this process to the report data dir // and build the GraphData (backward compatibility). for &metric in &profile_metrics { - let filename = if metric == "legacy" { + let filename_suffix = if metric == "legacy" { // backward compatibility - previous versions generated a single flamegraph - format!("{}-java-flamegraph-{}.html", params.run_name, process) + format!("java-flamegraph-{}.html", process) } else { - format!( - "{}-java-profile-{}-{}.html", - params.run_name, process, metric - ) + format!("java-profile-{}-{}.html", process, metric) }; - copy_graph_and_update_graph_data( - ¶ms.data_dir, - ¶ms.report_dir, - &filename, - metric, - &deduped_name, - format!("({}) JVM: {}", metric, deduped_name), - &mut graph_data, - ); + let filename_pattern = format!("{}$", regex::escape(&filename_suffix)); + if let Ok(filename) = find_file(¶ms.data_dir, &filename_pattern, None) { + copy_graph_and_update_graph_data( + ¶ms.data_dir, + ¶ms.report_dir, + &filename, + metric, + &deduped_name, + format!("({}) JVM: {}", metric, deduped_name), + &mut graph_data, + ); + } } // Deserialize the ProfilerData generated at the end of recording. - let profiler_data_path = params.data_dir.join(format!( - "{}-java-profile-{}-profiler-data.json", - params.run_name, process - )); - let mut profiler = match fs::read_to_string(&profiler_data_path) - .ok() - .and_then(|json| serde_json::from_str::(&json).ok()) + let mut profiler = match fs::read_to_string( + params.data_dir.join(java_profiler_data_filename(process)), + ) + .ok() + .and_then(|json| serde_json::from_str::(&json).ok()) { Some(profiler) => profiler, None => continue, diff --git a/src/data/perf_profile.rs b/src/data/perf_profile.rs index 7f3d6028..840391d1 100644 --- a/src/data/perf_profile.rs +++ b/src/data/perf_profile.rs @@ -1,7 +1,7 @@ use crate::data::common::data_formats::{ AperfData, GraphData, GraphGroup, Profiler, ProfilingData, }; -use crate::data::common::utils::copy_graph_and_update_graph_data; +use crate::data::common::utils::{copy_graph_and_update_graph_data, find_file}; use crate::data::{Data, ProcessData}; use crate::visualizer::ReportParams; use anyhow::Result; @@ -51,6 +51,10 @@ lazy_static! { pub static ref PROFILE_START_TIME_MS: Mutex = Mutex::new(0); } +fn perf_profiler_data_filename() -> String { + "perf_profiler_data.json".to_string() +} + #[derive(Serialize, Deserialize, Debug, Clone)] pub struct PerfProfileRaw { pub data: String, @@ -175,16 +179,8 @@ impl CollectData for PerfProfileRaw { ]) .status(); - let fg_out = File::create( - params - .data_dir - .join(format!("{}-flamegraph.svg", params.run_name)), - )?; - let reverse_fg_out = File::create( - params - .data_dir - .join(format!("{}-reverse-flamegraph.svg", params.run_name)), - )?; + let fg_out = File::create(params.data_dir.join("flamegraph.svg"))?; + let reverse_fg_out = File::create(params.data_dir.join("reverse-flamegraph.svg"))?; match out_jit { Err(e) => { @@ -257,11 +253,8 @@ impl CollectData for PerfProfileRaw { *PROFILE_START_TIME_MS.lock().unwrap(), events_out_path, ); - let perf_profiler_data_path = params - .data_dir - .join(format!("{}-perf-profiler-data.json", params.run_name)); if let Ok(json) = serde_json::to_string(&perf_profiler_data) { - fs::write(&perf_profiler_data_path, json)?; + fs::write(params.data_dir.join(perf_profiler_data_filename()), json)?; } } @@ -304,35 +297,40 @@ impl ProcessData for PerfProfile { graph_data.graph_groups.push(GraphGroup::new("default")); graph_data.graph_groups.push(GraphGroup::new("reverse")); [false, true].iter().for_each(|&is_reverse| { + // Match both the current (`flamegraph.svg`) and legacy (`-flamegraph.svg`) naming. let filename = if is_reverse { - format!("{}-reverse-flamegraph.svg", params.run_name) + find_file(¶ms.data_dir, r"reverse-flamegraph\.svg$", None) } else { - format!("{}-flamegraph.svg", params.run_name) + find_file( + ¶ms.data_dir, + r"flamegraph\.svg$", + Some(r"reverse-flamegraph\.svg$"), + ) }; - copy_graph_and_update_graph_data( - ¶ms.data_dir, - ¶ms.report_dir, - &filename, - if is_reverse { "reverse" } else { "default" }, - "cpu", - "Perf CPU Profile".to_string(), - &mut graph_data, - ); + if let Ok(filename) = filename { + copy_graph_and_update_graph_data( + ¶ms.data_dir, + ¶ms.report_dir, + &filename, + if is_reverse { "reverse" } else { "default" }, + "cpu", + "Perf CPU Profile".to_string(), + &mut graph_data, + ); + } }); // Deserialize the ProfilerData generated at the end of record. - let perf_profiler_data_path = params - .data_dir - .join(format!("{}-perf-profiler-data.json", params.run_name)); - let perf_profiler_data = match fs::read_to_string(&perf_profiler_data_path) - .ok() - .and_then(|json| serde_json::from_str::(&json).ok()) - { - Some(perf_profiler_data) => perf_profiler_data, - // If ProfilerData could not be read, chaces are this run was created before the - // introduction of ProfilingData, so fall back to the old GraphData. - None => return Ok(AperfData::Graph(graph_data)), - }; + let perf_profiler_data = + match fs::read_to_string(params.data_dir.join(perf_profiler_data_filename())) + .ok() + .and_then(|json| serde_json::from_str::(&json).ok()) + { + Some(perf_profiler_data) => perf_profiler_data, + // If ProfilerData could not be read, chaces are this run was created before the + // introduction of ProfilingData, so fall back to the old GraphData. + None => return Ok(AperfData::Graph(graph_data)), + }; let mut profiling_data = ProfilingData::default(); profiling_data diff --git a/src/lib.rs b/src/lib.rs index a983ce7b..459ebdfa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -109,6 +109,9 @@ pub enum PDError { #[error("The run {0:?} does not exist.")] RunNotFound(PathBuf), + #[error("The run {0:?} was specified more than once.")] + DuplicateRunPath(PathBuf), + #[error("The report {0} already exists in current directory.")] ReportExists(String), @@ -415,32 +418,6 @@ impl Default for PerformanceData { } } -pub fn get_file(dir: &PathBuf, name: String) -> Result<(PathBuf, fs::File)> { - for path in fs::read_dir(dir.clone())? { - let file_name = path?.file_name().into_string().unwrap(); - if file_name.starts_with(&name) { - let file_path = dir.join(file_name.clone()); - let file = fs::OpenOptions::new() - .read(true) - .open(file_path.clone()) - .expect("Could not open file"); - // file_name = file_path.to_str().unwrap().to_string(); - return Ok((file_path, file)); - } - } - Err(PDError::VisualizerFileNotFound(name).into()) -} - -pub fn get_file_name(dir: String, name: String) -> Result { - for path in fs::read_dir(dir.clone())? { - let file_name = path?.file_name().into_string().unwrap(); - if file_name.contains(&name) { - return Ok(file_name); - } - } - Err(PDError::VisualizerFileNotFound(name).into()) -} - #[derive(Default)] pub struct VisualizationData { pub visualizers: HashMap, diff --git a/src/report.rs b/src/report.rs index 4c3bd25a..320e8805 100644 --- a/src/report.rs +++ b/src/report.rs @@ -9,7 +9,7 @@ use clap::Args; use flate2::{read::GzDecoder, write::GzEncoder, Compression}; use log::{info, warn}; use serde::{Deserialize, Serialize}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::fs::File; use std::hash::{DefaultHasher, Hash, Hasher}; use std::io::Write; @@ -240,6 +240,7 @@ pub fn report(report: &Report, tmp_dir: &PathBuf) -> Result<()> { } let mut runs_info = RunsInfo::new(); + let mut seen_run_paths: HashSet = HashSet::new(); for run in &report.run { let run_path = PathBuf::from(run); @@ -248,6 +249,12 @@ pub fn report(report: &Report, tmp_dir: &PathBuf) -> Result<()> { return Err(PDError::RunNotFound(run_path).into()); } + if let Ok(canonical_path) = fs::canonicalize(&run_path) { + if !seen_run_paths.insert(canonical_path) { + return Err(PDError::DuplicateRunPath(run_path).into()); + } + } + let is_run_path_dir = run_path.is_dir(); // Extract the data if the input run path is an archive let extracted_dir_path = if is_run_path_dir { diff --git a/src/visualizer.rs b/src/visualizer.rs index 12037c7c..82f8fcb2 100644 --- a/src/visualizer.rs +++ b/src/visualizer.rs @@ -1,9 +1,10 @@ use crate::data::common::data_formats::{AperfData, DataFormat, ProcessedData}; -use crate::data::common::utils::{combine_value_ranges, topological_sort}; +use crate::data::common::utils::{combine_value_ranges, find_file, topological_sort}; use crate::data::TimeEnum; -use crate::{data::Data, data::ReportData, get_file}; +use crate::{data::Data, data::ReportData}; use anyhow::Result; use log::{debug, error}; +use std::fs; use std::io::{Seek, SeekFrom}; use std::path::{Path, PathBuf}; use std::{collections::HashMap, fs::File}; @@ -64,25 +65,32 @@ impl DataVisualizer { ) -> Result<()> { self.report_params.run_name = run_name.clone(); self.report_params.collection_start = collection_start; - let (file_path, file) = - get_file(&run_data_dir, self.data_name.to_string()).or_else(|e| { - // Backward compatibility: if file is not found using the data's name, - // see if files with compatible names exist - for compatible_name in self.data.compatible_filenames() { - match get_file(&run_data_dir, String::from(compatible_name)) { - Ok(compatible_file) => { - debug!( - "Data file {} not found, use compatible file name {}", - self.data_name, compatible_name - ); - return Ok(compatible_file); - } - Err(_) => {} - } + let file_path = find_file( + &run_data_dir, + &format!("^{}", regex::escape(self.data_name)), + None, + ) + .map(|filename| run_data_dir.join(filename)) + .or_else(|e| { + // Backward compatibility: if file is not found using the data's name, + // see if files with compatible names exist + for compatible_name in self.data.compatible_filenames() { + if let Ok(filename) = find_file( + &run_data_dir, + &format!("^{}", regex::escape(compatible_name)), + None, + ) { + debug!( + "Data file {} not found, use compatible file name {}", + self.data_name, compatible_name + ); + return Ok(run_data_dir.join(filename)); } - self.data_available.insert(run_name.clone(), false); - Err(e) - })?; + } + self.data_available.insert(run_name.clone(), false); + Err(e) + })?; + let file = fs::OpenOptions::new().read(true).open(&file_path)?; self.report_params.data_dir = run_data_dir; self.report_params.tmp_dir = tmp_dir.to_path_buf(); self.report_params.report_dir = report_dir.to_path_buf(); diff --git a/tests/test_aperf.rs b/tests/test_aperf.rs index 3f52729d..88cd2c8d 100644 --- a/tests/test_aperf.rs +++ b/tests/test_aperf.rs @@ -423,6 +423,98 @@ fn test_report_renamed_run() { }) } +/// Backward compatibility: a run recorded by a much older APerf release (v0.1.15-alpha) with +/// `--profile --profile-java` must still produce a valid report with the current build. +#[test] +fn test_report_backward_compatibility() { + run_test(|work_dir, tmp_dir| { + let run_name = String::from("aperf_v0_1_15_alpha_run"); + let run_path = get_test_data_path(format!("{}.tar.gz", run_name)); + + let report_name = String::from("legacy_v0_1_15_report"); + let rep = Report { + run: vec![run_path.into_os_string().into_string().unwrap()], + name: Some( + work_dir + .join(&report_name) + .into_os_string() + .into_string() + .unwrap(), + ), + time_range: vec![], + }; + assert!(report(&rep, &tmp_dir).is_ok()); + + verify_report_structure(&work_dir, &report_name, vec![run_name.clone()]); + + // Verify the backward-compatibility data-name mappings produced data under the CURRENT + // names, even though the legacy run stored them under old names. + let report_js_dir = work_dir.join(&report_name).join("data").join("js"); + for current_name in [ + "diskstats", // legacy: disk_stats + "systeminfo", // legacy: system_info + "aperf_stats", // legacy: aperf_run_stats + "cpu_utilization", + "meminfo", + "vmstat", + "netstat", + "interrupts", + "perf_stat", + "processes", + "sysctl", + "kernel_config", + ] { + let js_path = report_js_dir.join(format!("{current_name}.js")); + assert!( + js_path.exists(), + "Expected processed data file {current_name}.js to exist for the legacy run", + ); + let contents = fs::read_to_string(&js_path).unwrap(); + assert!( + contents.contains(&run_name), + "{current_name}.js should contain data keyed by the legacy run name", + ); + } + + // Legacy profiling back-compat: the old run carries a single forward flamegraph SVG + // (`-flamegraph.svg`). The report must copy that SVG into data/js and wire it into + // the perf_profile "default" graph group. (v0.1.15-alpha produced no reverse graph.) + let legacy_flamegraph = report_js_dir.join(format!("{run_name}-flamegraph.svg")); + assert!( + legacy_flamegraph.exists(), + "legacy flamegraph SVG should be copied into the report's data/js dir", + ); + let perf_profile = fs::read_to_string(report_js_dir.join("perf_profile.js")).unwrap(); + assert!( + perf_profile.contains(&format!("{run_name}-flamegraph.svg")), + "perf_profile.js should reference the legacy flamegraph SVG in a graph group", + ); + + // Legacy Java profiling back-compat: the old run carries a per-JVM flamegraph HTML + // (`-java-flamegraph-.html`) resolved via the `-jps-map.json` PID->name + // map. The report must copy that HTML into data/js and wire it into the java_profile + // "legacy" graph group, keyed by the JVM name ("Busy"). + let java_flamegraph = report_js_dir.join(format!("{run_name}-java-flamegraph-26848.html")); + assert!( + java_flamegraph.exists(), + "legacy Java flamegraph HTML should be copied into the report's data/js dir", + ); + let java_profile = fs::read_to_string(report_js_dir.join("java_profile.js")).unwrap(); + assert!( + java_profile.contains(&format!("{run_name}-java-flamegraph-26848.html")), + "java_profile.js should reference the legacy Java flamegraph HTML in a graph group", + ); + assert!( + java_profile.contains("Busy"), + "java_profile.js should resolve the JVM name from the legacy jps-map", + ); + + clean_dir_and_archive(&work_dir, &report_name); + + Ok(()) + }) +} + #[test] fn test_duplicate_run_names() { run_test(|work_dir, tmp_dir| { @@ -655,6 +747,73 @@ fn test_run_data_not_exists() { }) } +#[test] +fn test_report_duplicate_run_path() { + run_test(|work_dir, tmp_dir| { + let run_name = String::from("test_run_1"); + let run_path = get_test_data_path(format!("{}.tar.gz", run_name)); + let run_path_str = run_path.clone().into_os_string().into_string().unwrap(); + + // Case 1: exact same string passed twice. + let report_name = String::from("dup_path_report"); + let report_dir_path = work_dir.join(&report_name); + let report_archive_path = work_dir.join(format!("{}.tar.gz", report_name)); + let rep = Report { + run: vec![run_path_str.clone(), run_path_str.clone()], + name: Some( + report_dir_path + .clone() + .into_os_string() + .into_string() + .unwrap(), + ), + time_range: vec![], + }; + let error = report(&rep, &tmp_dir).unwrap_err(); + assert_eq!( + error.to_string(), + format!("The run {:?} was specified more than once.", run_path), + ); + // Report should not have been created. + assert!(!report_dir_path.exists()); + assert!(!report_archive_path.exists()); + + // Case 2: same path through a different surface form. `./` prefix + // resolves to the same canonical path, which canonicalize() should + // collapse so the duplicate check still fires. + let alt_run_path_str = format!( + "./{}", + run_path.clone().into_os_string().into_string().unwrap() + ); + let report_name_2 = String::from("dup_path_report_2"); + let report_dir_path_2 = work_dir.join(&report_name_2); + let report_archive_path_2 = work_dir.join(format!("{}.tar.gz", report_name_2)); + let rep_alt = Report { + run: vec![run_path_str.clone(), alt_run_path_str], + name: Some( + report_dir_path_2 + .clone() + .into_os_string() + .into_string() + .unwrap(), + ), + time_range: vec![], + }; + let error_alt = report(&rep_alt, &tmp_dir).unwrap_err(); + assert!( + error_alt + .to_string() + .contains("was specified more than once"), + "Error should report a duplicate run path, got: {}", + error_alt, + ); + assert!(!report_dir_path_2.exists()); + assert!(!report_archive_path_2.exists()); + + Ok(()) + }) +} + #[test] fn test_report_with_time_range_invalid_run_name() { run_test(|work_dir, tmp_dir| { diff --git a/tests/test_data/aperf_v0_1_15_alpha_run.tar.gz b/tests/test_data/aperf_v0_1_15_alpha_run.tar.gz new file mode 100644 index 00000000..069437d1 Binary files /dev/null and b/tests/test_data/aperf_v0_1_15_alpha_run.tar.gz differ