From c5c585f1ec52602c2a4c750ff670e22fbf71ccb6 Mon Sep 17 00:00:00 2001 From: Sandeep Narendranath Karjala Date: Tue, 15 Jul 2025 17:07:46 -0700 Subject: [PATCH 1/2] Add fixtures for multi-rank tests --- src/cli.rs | 3 - .../some_other_file.txt | 0 tests/integration_test.rs | 148 +++++++++++++++++- 3 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 tests/inputs/multi_rank_messy_input/some_other_file.txt diff --git a/src/cli.rs b/src/cli.rs index a1eccc9..33cefb9 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -82,9 +82,6 @@ fn main() -> anyhow::Result<()> { if cli.latest { bail!("--latest cannot be used with --all-ranks-html"); } - if cli.no_browser { - bail!("--no-browser not yet implemented with --all-ranks-html"); - } } let config = ParseConfig { diff --git a/tests/inputs/multi_rank_messy_input/some_other_file.txt b/tests/inputs/multi_rank_messy_input/some_other_file.txt new file mode 100644 index 0000000..e69de29 diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 2e659b6..80fee71 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -1,6 +1,5 @@ use std::collections::HashMap; use std::path::Path; -use std::path::PathBuf; use tlparse; fn prefix_exists(map: &HashMap, prefix: &str) -> bool { @@ -419,3 +418,150 @@ fn test_provenance_tracking() { ); } } + +use std::fs::{self, create_dir_all, remove_dir_all}; +use std::path::PathBuf; +use std::process::Command; + +#[test] +fn test_all_ranks_basic() { + let pid = std::process::id(); + let input_dir = PathBuf::from(format!("test_input_basic_{pid}")); + create_dir_all(&input_dir).unwrap(); + let out_dir = PathBuf::from(format!("test_out_basic_{pid}")); + + let log_content = fs::read_to_string("tests/inputs/simple.log").unwrap(); + fs::write( + input_dir.join("dedicated_log_torch_trace_rank_0.log"), + &log_content, + ) + .unwrap(); + fs::write( + input_dir.join("dedicated_log_torch_trace_rank_1.log"), + &log_content, + ) + .unwrap(); + + let output = Command::new(env!("CARGO_BIN_EXE_tlparse")) + .arg(&input_dir) + .arg("--all-ranks-html") + .arg("--overwrite") + .arg("-o") + .arg(&out_dir) + .arg("--no-browser") + .output() + .expect("failed to run tlparse"); + + assert!( + output.status.success(), + "tlparse command failed. stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let rank0_index = out_dir.join("rank_0/index.html"); + let rank1_index = out_dir.join("rank_1/index.html"); + let landing_page = out_dir.join("index.html"); + + assert!(rank0_index.exists(), "rank 0 index.html should exist"); + assert!(rank1_index.exists(), "rank 1 index.html should exist"); + assert!(landing_page.exists(), "toplevel index.html should exist"); + + let landing_content = fs::read_to_string(landing_page).unwrap(); + assert!( + landing_content.contains(r#""#), + "Landing page should contain a link to rank 0" + ); + assert!( + landing_content.contains(r#""#), + "Landing page should contain a link to rank 1" + ); + + remove_dir_all(&input_dir).unwrap(); + remove_dir_all(&out_dir).unwrap(); +} + +#[test] +fn test_all_ranks_no_logs() { + let pid = std::process::id(); + let input_dir = PathBuf::from(format!("test_input_nologs_{pid}")); + create_dir_all(&input_dir).unwrap(); + let out_dir = PathBuf::from(format!("test_out_nologs_{pid}")); + + let output = Command::new(env!("CARGO_BIN_EXE_tlparse")) + .arg(&input_dir) + .arg("--all-ranks-html") + .arg("--overwrite") + .arg("-o") + .arg(&out_dir) + .arg("--no-browser") + .output() + .expect("failed to run tlparse"); + + assert!(!output.status.success(), "tlparse should fail on empty dir"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("No rank log files found"), + "stderr should complain about missing log files" + ); + + remove_dir_all(&input_dir).unwrap(); + let _ = remove_dir_all(&out_dir); +} + +#[test] +fn test_all_ranks_overwrite() { + let pid = std::process::id(); + let input_dir = PathBuf::from(format!("test_input_overwrite_{pid}")); + create_dir_all(&input_dir).unwrap(); + let out_dir = PathBuf::from(format!("test_out_overwrite_{pid}")); + create_dir_all(&out_dir).unwrap(); + + let log_content = fs::read_to_string("tests/inputs/simple.log").unwrap(); + fs::write( + input_dir.join("dedicated_log_torch_trace_rank_0.log"), + &log_content, + ) + .unwrap(); + + // Run without --overwrite, should fail + let output_fail = Command::new(env!("CARGO_BIN_EXE_tlparse")) + .arg(&input_dir) + .arg("--all-ranks-html") + .arg("-o") + .arg(&out_dir) + .arg("--no-browser") + .output() + .expect("failed to run tlparse"); + + assert!( + !output_fail.status.success(), + "tlparse should fail without --overwrite on existing dir" + ); + let stderr = String::from_utf8_lossy(&output_fail.stderr); + assert!( + stderr.contains("already exists"), + "stderr should complain about existing directory" + ); + + // Run with --overwrite, should succeed + let output_success = Command::new(env!("CARGO_BIN_EXE_tlparse")) + .arg(&input_dir) + .arg("--all-ranks-html") + .arg("--overwrite") + .arg("-o") + .arg(&out_dir) + .arg("--no-browser") + .output() + .expect("failed to run tlparse"); + + assert!( + output_success.status.success(), + "tlparse should succeed with --overwrite. stderr: {}", + String::from_utf8_lossy(&output_success.stderr) + ); + assert!(out_dir.join("rank_0/index.html").exists()); + assert!(out_dir.join("index.html").exists()); + + remove_dir_all(&input_dir).unwrap(); + remove_dir_all(&out_dir).unwrap(); +} From 616b0157fa77f0095c17949bee4bed9fcac2dd9e Mon Sep 17 00:00:00 2001 From: Sandeep Narendranath Karjala Date: Tue, 15 Jul 2025 17:11:56 -0700 Subject: [PATCH 2/2] Generate html landing page and all-ranks-html tests --- Cargo.toml | 3 + src/cli.rs | 32 +++++-- src/lib.rs | 26 ++++++ tests/integration_test.rs | 177 ++++++++++++++++++++++---------------- 4 files changed, 156 insertions(+), 82 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 32ccf79..7b1172b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,3 +32,6 @@ regex = "1.9.2" serde = { version = "1.0.185", features = ["serde_derive"] } serde_json = "1.0.100" tinytemplate = "1.1.0" + +[dev-dependencies] +tempfile = "3" diff --git a/src/cli.rs b/src/cli.rs index 33cefb9..ce852dd 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -4,6 +4,7 @@ use anyhow::{bail, Context}; use std::fs; use std::path::PathBuf; +use tlparse::generate_multi_rank_html; use tlparse::{parse_path, ParseConfig}; #[derive(Parser)] @@ -54,6 +55,12 @@ pub struct Cli { fn main() -> anyhow::Result<()> { let cli = Cli::parse(); + + // Early validation of incompatible flags + if cli.all_ranks_html && cli.latest { + bail!("--latest cannot be used with --all-ranks-html"); + } + let path = if cli.latest { let input_path = cli.path; // Path should be a directory @@ -78,12 +85,6 @@ fn main() -> anyhow::Result<()> { cli.path }; - if cli.all_ranks_html { - if cli.latest { - bail!("--latest cannot be used with --all-ranks-html"); - } - } - let config = ParseConfig { strict: cli.strict, strict_compile_id: cli.strict_compile_id, @@ -96,7 +97,7 @@ fn main() -> anyhow::Result<()> { }; if cli.all_ranks_html { - handle_all_ranks(&config, path, cli.out, cli.overwrite)?; + handle_all_ranks(&config, path, cli.out, cli.overwrite, !cli.no_browser)?; } else { handle_one_rank( &config, @@ -183,6 +184,7 @@ fn handle_all_ranks( path: PathBuf, out_path: PathBuf, overwrite: bool, + open_browser: bool, ) -> anyhow::Result<()> { let input_dir = path; if !input_dir.is_dir() { @@ -221,6 +223,14 @@ fn handle_all_ranks( ); } + let mut sorted_ranks: Vec = + rank_logs.iter().map(|(_, rank)| rank.to_string()).collect(); + sorted_ranks.sort_by(|a, b| { + a.parse::() + .unwrap_or(0) + .cmp(&b.parse::().unwrap_or(0)) + }); + for (log_path, rank_num) in rank_logs { let subdir = out_path.join(format!("rank_{rank_num}")); println!("Processing rank {rank_num} → {}", subdir.display()); @@ -232,6 +242,12 @@ fn handle_all_ranks( "Multi-rank report generated under {}\nIndividual pages: rank_*/index.html", out_path.display() ); - // TODO: generate and open a landing page + + let (landing_page_path, landing_html) = generate_multi_rank_html(&out_path, sorted_ranks, cfg)?; + fs::write(&landing_page_path, landing_html)?; + if open_browser { + opener::open(&landing_page_path)?; + } + Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index 3665746..349bb41 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,6 +22,8 @@ pub mod parsers; mod templates; mod types; +pub use crate::templates::{CSS, TEMPLATE_MULTI_RANK_INDEX, TEMPLATE_QUERY_PARAM_SCRIPT}; + #[derive(Debug)] enum ParserResult { NoPayload, @@ -1145,3 +1147,27 @@ pub fn parse_path(path: &PathBuf, config: &ParseConfig) -> anyhow::Result, + cfg: &ParseConfig, +) -> anyhow::Result<(PathBuf, String)> { + // Create the TinyTemplate instance for rendering the landing page. + let mut tt = TinyTemplate::new(); + tt.add_formatter("format_unescaped", tinytemplate::format_unescaped); + tt.add_template("multi_rank_index.html", TEMPLATE_MULTI_RANK_INDEX)?; + + let ctx = MultiRankContext { + css: CSS, + custom_header_html: &cfg.custom_header_html, + num_ranks: sorted_ranks.len(), + ranks: sorted_ranks, + qps: TEMPLATE_QUERY_PARAM_SCRIPT, + }; + + let html = tt.render("multi_rank_index.html", &ctx)?; + let landing_page_path = out_path.join("index.html"); + + Ok((landing_page_path, html)) +} diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 80fee71..0e0c0a1 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -1,5 +1,9 @@ use std::collections::HashMap; +use std::fs; use std::path::Path; +use std::path::PathBuf; +use std::process::Command; +use tempfile::tempdir; use tlparse; fn prefix_exists(map: &HashMap, prefix: &str) -> bool { @@ -419,28 +423,11 @@ fn test_provenance_tracking() { } } -use std::fs::{self, create_dir_all, remove_dir_all}; -use std::path::PathBuf; -use std::process::Command; - #[test] -fn test_all_ranks_basic() { - let pid = std::process::id(); - let input_dir = PathBuf::from(format!("test_input_basic_{pid}")); - create_dir_all(&input_dir).unwrap(); - let out_dir = PathBuf::from(format!("test_out_basic_{pid}")); - - let log_content = fs::read_to_string("tests/inputs/simple.log").unwrap(); - fs::write( - input_dir.join("dedicated_log_torch_trace_rank_0.log"), - &log_content, - ) - .unwrap(); - fs::write( - input_dir.join("dedicated_log_torch_trace_rank_1.log"), - &log_content, - ) - .unwrap(); +fn test_all_ranks_basic() -> Result<(), Box> { + let input_dir = PathBuf::from("tests/inputs/multi_rank_logs"); + let temp_dir = tempdir().unwrap(); + let out_dir = temp_dir.path().join("out"); let output = Command::new(env!("CARGO_BIN_EXE_tlparse")) .arg(&input_dir) @@ -449,8 +436,7 @@ fn test_all_ranks_basic() { .arg("-o") .arg(&out_dir) .arg("--no-browser") - .output() - .expect("failed to run tlparse"); + .output()?; assert!( output.status.success(), @@ -475,17 +461,14 @@ fn test_all_ranks_basic() { landing_content.contains(r#""#), "Landing page should contain a link to rank 1" ); - - remove_dir_all(&input_dir).unwrap(); - remove_dir_all(&out_dir).unwrap(); + Ok(()) } #[test] -fn test_all_ranks_no_logs() { - let pid = std::process::id(); - let input_dir = PathBuf::from(format!("test_input_nologs_{pid}")); - create_dir_all(&input_dir).unwrap(); - let out_dir = PathBuf::from(format!("test_out_nologs_{pid}")); +fn test_all_ranks_messy_input() -> Result<(), Box> { + let input_dir = PathBuf::from("tests/inputs/multi_rank_messy_input"); + let temp_dir = tempdir().unwrap(); + let out_dir = temp_dir.path().join("out"); let output = Command::new(env!("CARGO_BIN_EXE_tlparse")) .arg(&input_dir) @@ -494,74 +477,120 @@ fn test_all_ranks_no_logs() { .arg("-o") .arg(&out_dir) .arg("--no-browser") - .output() - .expect("failed to run tlparse"); + .output()?; - assert!(!output.status.success(), "tlparse should fail on empty dir"); - let stderr = String::from_utf8_lossy(&output.stderr); assert!( - stderr.contains("No rank log files found"), - "stderr should complain about missing log files" + output.status.success(), + "tlparse command failed on messy input. stderr: {}", + String::from_utf8_lossy(&output.stderr) ); - remove_dir_all(&input_dir).unwrap(); - let _ = remove_dir_all(&out_dir); + let rank0_index = out_dir.join("rank_0/index.html"); + let rank1_index = out_dir.join("rank_1/index.html"); + let landing_page = out_dir.join("index.html"); + + assert!( + rank0_index.exists(), + "rank 0 index.html should exist in messy input test" + ); + assert!( + rank1_index.exists(), + "rank 1 index.html should exist in messy input test" + ); + assert!( + landing_page.exists(), + "toplevel index.html should exist in messy input test" + ); + + let landing_content = fs::read_to_string(landing_page).unwrap(); + assert!( + landing_content.contains(r#""#), + "Landing page should contain a link to rank 0 in messy input test" + ); + assert!( + landing_content.contains(r#""#), + "Landing page should contain a link to rank 1 in messy input test" + ); + Ok(()) } #[test] -fn test_all_ranks_overwrite() { - let pid = std::process::id(); - let input_dir = PathBuf::from(format!("test_input_overwrite_{pid}")); - create_dir_all(&input_dir).unwrap(); - let out_dir = PathBuf::from(format!("test_out_overwrite_{pid}")); - create_dir_all(&out_dir).unwrap(); - - let log_content = fs::read_to_string("tests/inputs/simple.log").unwrap(); - fs::write( - input_dir.join("dedicated_log_torch_trace_rank_0.log"), - &log_content, - ) - .unwrap(); - - // Run without --overwrite, should fail - let output_fail = Command::new(env!("CARGO_BIN_EXE_tlparse")) +fn test_all_ranks_no_browser() -> Result<(), Box> { + let input_dir = PathBuf::from("tests/inputs/multi_rank_logs"); + let temp_dir = tempdir().unwrap(); + let out_dir = temp_dir.path().join("out"); + + let output = Command::new(env!("CARGO_BIN_EXE_tlparse")) .arg(&input_dir) .arg("--all-ranks-html") + .arg("--overwrite") .arg("-o") .arg(&out_dir) .arg("--no-browser") - .output() - .expect("failed to run tlparse"); + .output()?; assert!( - !output_fail.status.success(), - "tlparse should fail without --overwrite on existing dir" + output.status.success(), + "tlparse command failed. stderr: {}", + String::from_utf8_lossy(&output.stderr) ); - let stderr = String::from_utf8_lossy(&output_fail.stderr); + + let rank0_index = out_dir.join("rank_0/index.html"); + let rank1_index = out_dir.join("rank_1/index.html"); + let landing_page = out_dir.join("index.html"); + + assert!(rank0_index.exists(), "rank 0 index.html should exist"); + assert!(rank1_index.exists(), "rank 1 index.html should exist"); + assert!(landing_page.exists(), "toplevel index.html should exist"); + Ok(()) +} + +#[test] +fn test_all_ranks_with_latest_fails() -> Result<(), Box> { + let input_dir = PathBuf::from("tests/inputs/multi_rank_logs"); + let temp_root = tempdir()?; // only used for output cleanup + let out_dir = temp_root.path().join("out"); + + let output = Command::new(env!("CARGO_BIN_EXE_tlparse")) + .arg(&input_dir) + .arg("--all-ranks-html") + .arg("--latest") + .arg("-o") + .arg(&out_dir) + .output()?; + assert!( - stderr.contains("already exists"), - "stderr should complain about existing directory" + !output.status.success(), + "tlparse should fail when --all-ranks-html and --latest are used together" ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("--latest cannot be used with --all-ranks-html"), + "stderr should complain about using --latest with --all-ranks-html" + ); + Ok(()) +} - // Run with --overwrite, should succeed - let output_success = Command::new(env!("CARGO_BIN_EXE_tlparse")) +#[test] +fn test_all_ranks_no_logs() -> Result<(), Box> { + let temp_root = tempdir()?; + let input_dir = temp_root.path().to_path_buf(); + let out_dir = temp_root.path().join("out"); + + let output = Command::new(env!("CARGO_BIN_EXE_tlparse")) .arg(&input_dir) .arg("--all-ranks-html") .arg("--overwrite") .arg("-o") .arg(&out_dir) .arg("--no-browser") - .output() - .expect("failed to run tlparse"); + .output()?; + assert!(!output.status.success(), "tlparse should fail on empty dir"); + let stderr = String::from_utf8_lossy(&output.stderr); assert!( - output_success.status.success(), - "tlparse should succeed with --overwrite. stderr: {}", - String::from_utf8_lossy(&output_success.stderr) + stderr.contains("No rank log files found"), + "stderr should complain about missing log files" ); - assert!(out_dir.join("rank_0/index.html").exists()); - assert!(out_dir.join("index.html").exists()); - - remove_dir_all(&input_dir).unwrap(); - remove_dir_all(&out_dir).unwrap(); + Ok(()) }