diff --git a/README.md b/README.md index 131f0a7..337d159 100644 --- a/README.md +++ b/README.md @@ -157,6 +157,7 @@ they are included in their respective sets. | `--no-attrs` | Skip metadata comparison (type, size, direction, attributes) | | `--set1 ` | File(s) for set 1; may be specified multiple times | | `--set2 ` | File(s) for set 2; may be specified multiple times | +| `-f, --filter ` | Filter signals by glob pattern; may be repeated or space-separated | ### Exit codes @@ -226,6 +227,14 @@ Skip metadata comparison (only compare signal values): wavediff --no-attrs golden.fst test.fst ``` +Restrict the comparison to a subset of signals with a glob filter: + +``` +wavediff --filter '*.clk' golden.fst test.fst +wavediff --filter '*.clk *.cyc' golden.fst test.fst +wavediff --filter '*.clk' --filter '*.cyc' golden.fst test.fst +``` + ## Supported formats ### FST diff --git a/src/bin/wavecat.rs b/src/bin/wavecat.rs index 81cd94f..e3aef9a 100644 --- a/src/bin/wavecat.rs +++ b/src/bin/wavecat.rs @@ -7,10 +7,9 @@ //------------------------------------------------------------------------------ use clap::Parser; -use glob::Pattern; use wavetools::{ - names_only, open_wave_files, write_attrs, write_names, write_signals_wave_multi, NameOptions, - SignalOutputOptions, WaveFormat, WaveHierarchy, + apply_filter, names_only, open_wave_files, parse_filter_patterns, write_attrs, write_names, + write_signals_wave_multi, NameOptions, SignalOutputOptions, WaveFormat, }; use std::path::PathBuf; use std::process; @@ -87,28 +86,6 @@ fn parse_format(s: &str) -> Result { } } -fn parse_filter_patterns(filter: &[String]) -> Result, String> { - filter - .iter() - .flat_map(|s| s.split_whitespace()) - .map(|p| Pattern::new(p).map_err(|e| format!("Invalid glob pattern '{}': {}", p, e))) - .collect() -} - -fn apply_filters(hier: &mut WaveHierarchy, patterns: &[Pattern]) { - if patterns.is_empty() { - return; - } - let tree = &hier.names; - hier.signal_map.retain(|_, info| { - info.vars.retain(|v| { - let name = tree.format_path(v.name); - patterns.iter().any(|p| p.matches(&name)) - }); - !info.vars.is_empty() - }); -} - fn main() { let args = Args::parse(); @@ -133,7 +110,7 @@ fn process_wave_file(args: &Args) -> Result<(), String> { let paths: Vec<&std::path::Path> = args.file.iter().map(std::path::PathBuf::as_path).collect(); let (readers, mut hierarchy, offsets) = open_wave_files(&paths, &name_options, args.format)?; - apply_filters(&mut hierarchy, &patterns); + apply_filter(&mut hierarchy, &patterns); if args.attrs { let mut stdout = std::io::stdout(); diff --git a/src/bin/wavediff.rs b/src/bin/wavediff.rs index 41af608..eeaacde 100644 --- a/src/bin/wavediff.rs +++ b/src/bin/wavediff.rs @@ -10,8 +10,8 @@ use clap::Parser; use std::io::Write; use std::path::PathBuf; use wavetools::{ - compare_signal_meta, compare_signal_names, diff_wave_sets, open_and_read_wave_sets, - DiffOptions, NameOptions, WaveHierarchy, + apply_filter, compare_signal_meta, compare_signal_names, diff_wave_sets, + open_and_read_wave_sets, parse_filter_patterns, DiffOptions, NameOptions, WaveHierarchy, }; const VERSION: &str = concat!( @@ -71,6 +71,11 @@ struct Args { /// File(s) for set 2; may be specified multiple times #[arg(long, action = clap::ArgAction::Append)] set2: Vec, + + /// Filter signals by glob pattern(s); may be specified multiple times or as a + /// space-separated list (e.g. --filter "*.foo *.bar" or --filter "*.foo" --filter "*.bar") + #[arg(short, long, action = clap::ArgAction::Append)] + filter: Vec, } fn main() { @@ -171,10 +176,13 @@ fn run(args: Args) -> Result { set2_paths.extend(args.set2.iter().cloned()); let name_options = NameOptions::default(); + let patterns = parse_filter_patterns(&args.filter)?; let paths1: Vec<&std::path::Path> = set1_paths.iter().map(PathBuf::as_path).collect(); let paths2: Vec<&std::path::Path> = set2_paths.iter().map(PathBuf::as_path).collect(); - let sets = open_and_read_wave_sets(&paths1, &paths2, &name_options)?; + let mut sets = open_and_read_wave_sets(&paths1, &paths2, &name_options)?; + apply_filter(&mut sets.hier1, &patterns); + apply_filter(&mut sets.hier2, &patterns); // For name-mismatch reporting, use FILE1/FILE2 if given, else first --set file let label1 = args.file1.as_deref().unwrap_or(set1_paths[0].as_path()); diff --git a/src/diff.rs b/src/diff.rs index 3953183..1d6e837 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -372,9 +372,14 @@ fn compare_signal_channels( } // Anything still in the buffer was in file2 but never matched by file1. + // Drop entries whose handles were filtered out of hier2 -- the streaming + // reader still emits them but they're not part of the comparison set. // Sort rows before printing so diff output does not depend on HashMap // iteration order. - let owned: Vec<((u64, usize), OwnedSignalValue)> = buffered2.drain().collect(); + let owned: Vec<((u64, usize), OwnedSignalValue)> = buffered2 + .drain() + .filter(|((_t, h), _)| hier2.signal_map.contains_key(h)) + .collect(); if !owned.is_empty() { has_differences = true; } @@ -390,10 +395,15 @@ fn compare_signal_channels( } // Drain any remaining file2 batches we never read from the channel. + // Only changes for handles still in hier2 count -- filtered-out handles + // are streamed by the reader but excluded from the comparison set. if !source2_ended { for batch2 in &rx2 { - has_differences = true; for change2 in &batch2.changes { + if !hier2.signal_map.contains_key(&change2.handle) { + continue; + } + has_differences = true; for name in format_handle_names(change2.handle, &hier2.signal_map, &hier2.names) { writeln!( writer, diff --git a/src/lib.rs b/src/lib.rs index e7bc977..b2232d8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -307,6 +307,32 @@ pub fn names_only(map: &SignalMap, tree: &NameTree) -> SignalNames { .collect() } +/// Parse a list of filter strings (each may contain whitespace-separated +/// glob patterns) into compiled `glob::Pattern`s. +pub fn parse_filter_patterns(filter: &[String]) -> Result, String> { + filter + .iter() + .flat_map(|s| s.split_whitespace()) + .map(|p| glob::Pattern::new(p).map_err(|e| format!("Invalid glob pattern '{}': {}", p, e))) + .collect() +} + +/// Drop entries from `hier.signal_map` whose fully-qualified names do not match +/// any of the provided glob patterns. No-op when `patterns` is empty. +pub fn apply_filter(hier: &mut WaveHierarchy, patterns: &[glob::Pattern]) { + if patterns.is_empty() { + return; + } + let tree = &hier.names; + hier.signal_map.retain(|_, info| { + info.vars.retain(|v| { + let name = tree.format_path(v.name); + patterns.iter().any(|p| p.matches(&name)) + }); + !info.vars.is_empty() + }); +} + /// An FST reader over a buffered file pub(crate) type FstFileReader = FstReader>; diff --git a/tests/diff_test.rs b/tests/diff_test.rs index ef60e88..fc45c4f 100644 --- a/tests/diff_test.rs +++ b/tests/diff_test.rs @@ -901,3 +901,273 @@ fn test_non_qualified_duplicate_enums_no_conflict() { result.err() ); } + +// -- --filter tests ----------------------------------------------------------- + +fn run_wave_diff_test_with_filter( + file1: &str, + file2: &str, + filter: &[&str], +) -> (bool, String) { + let name_options = NameOptions::default(); + let (reader1, mut hier1, reader2, mut hier2) = + wavetools::open_and_read_waves(file1, file2, &name_options) + .expect("Failed to open wave files"); + + let filter_strings: Vec = filter.iter().map(|&s| s.to_string()).collect(); + let patterns = wavetools::parse_filter_patterns(&filter_strings) + .expect("Failed to parse filter patterns"); + wavetools::apply_filter(&mut hier1, &patterns); + wavetools::apply_filter(&mut hier2, &patterns); + + let mut output = Vec::new(); + let options = DiffOptions { start: 0, end: None, real_epsilon: None }; + let has_differences = wavetools::diff_waves( + &mut output, + reader1, + hier1, + reader2, + hier2, + &options, + ) + .expect("Failed to diff files"); + + let output_str = String::from_utf8(output).expect("Invalid UTF-8"); + (has_differences, output_str) +} + +#[test] +fn test_filter_excludes_differing_signal() { + // counter.value.diff.fst differs only at t.the_sub.cyc_plus_one. + // Filtering it out leaves only matching signals -- no diff. + let (has_diff, output) = run_wave_diff_test_with_filter( + "tests/data/counter.fst", + "tests/data/counter.value.diff.fst", + &["*.clk"], + ); + assert!(!has_diff, "Filter excluding the differing signal should report no diff. Output:\n{}", output); + assert_eq!(output, "", "No diff output expected"); +} + +#[test] +fn test_filter_includes_differing_signal() { + // Same case but the filter targets the differing signal. + let (has_diff, output) = run_wave_diff_test_with_filter( + "tests/data/counter.fst", + "tests/data/counter.value.diff.fst", + &["*.cyc_plus_one"], + ); + assert!(has_diff, "Filter including the differing signal should report a diff"); + let expected = "\ +10 t.the_sub.cyc_plus_one 00000000000000000000000000000010 != 00000000000000000000000000000100 +"; + assert_eq!(output, expected, "Expected diff output for the targeted signal"); +} + +#[test] +fn test_filter_space_separated_patterns() { + // A single --filter value with multiple whitespace-separated globs should + // be split into individual patterns. + let (has_diff, output) = run_wave_diff_test_with_filter( + "tests/data/counter.fst", + "tests/data/counter.value.diff.fst", + &["*.clk *.cyc"], + ); + assert!(!has_diff, "Two safe-signal globs should still report no diff. Output:\n{}", output); +} + +#[test] +fn test_filter_multiple_args_are_unioned() { + // Several --filter values should union: signals matching any pattern are kept. + let (has_diff, output) = run_wave_diff_test_with_filter( + "tests/data/counter.fst", + "tests/data/counter.value.diff.fst", + &["*.clk", "*.cyc"], + ); + assert!(!has_diff, "Unioned safe-signal filters should report no diff. Output:\n{}", output); +} + +#[test] +fn test_filter_matches_nothing_yields_no_diff() { + // A filter that matches no signals in either file leaves both hierarchies + // empty -- no name diffs, no value diffs, nothing to report. + let (has_diff, output) = run_wave_diff_test_with_filter( + "tests/data/counter.fst", + "tests/data/counter.value.diff.fst", + &["*.nonexistent"], + ); + assert!(!has_diff, "Filter matching nothing should not synthesize a diff. Output:\n{}", output); + assert_eq!(output, "", "Empty filtered set should produce no output"); +} + +#[test] +fn test_filter_preserves_unrelated_diff() { + // Filter includes the differing signal AND others. Diff should still fire, + // and only the differing signal's line should appear. + let (has_diff, output) = run_wave_diff_test_with_filter( + "tests/data/counter.fst", + "tests/data/counter.value.diff.fst", + &["*.clk", "*.cyc_plus_one"], + ); + assert!(has_diff, "Filter including the differing signal should still diff"); + let expected = "\ +10 t.the_sub.cyc_plus_one 00000000000000000000000000000010 != 00000000000000000000000000000100 +"; + assert_eq!(output, expected, "Only the targeted differing signal should be reported"); +} + +// -- --filter CLI tests ------------------------------------------------------- + +#[test] +fn test_cli_filter_excludes_diff_exits_zero() { + let output = run_wavediff_cli(&[ + "--filter", + "*.clk", + "tests/data/counter.fst", + "tests/data/counter.value.diff.fst", + ]); + assert_eq!( + output.status.code(), + Some(0), + "Filtering out the diff should exit 0. stderr: {} stdout: {}", + String::from_utf8_lossy(&output.stderr), + String::from_utf8_lossy(&output.stdout), + ); +} + +#[test] +fn test_cli_filter_short_flag_includes_diff_exits_one() { + let output = run_wavediff_cli(&[ + "-f", + "*.cyc_plus_one", + "tests/data/counter.fst", + "tests/data/counter.value.diff.fst", + ]); + assert_eq!(output.status.code(), Some(1), "Targeting the differing signal should exit 1"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("cyc_plus_one"), + "stdout should contain the diff: {}", + stdout + ); +} + +#[test] +fn test_cli_filter_repeated_flag() { + // Repeated --filter flags should union, leaving only safe signals. + let output = run_wavediff_cli(&[ + "--filter", + "*.clk", + "--filter", + "*.cyc", + "tests/data/counter.fst", + "tests/data/counter.value.diff.fst", + ]); + assert_eq!( + output.status.code(), + Some(0), + "Repeated filters covering only safe signals should exit 0. stderr: {}", + String::from_utf8_lossy(&output.stderr), + ); +} + +fn filter_name_mismatch( + file1: &str, + file2: &str, + filter: &[&str], +) -> (std::collections::HashSet, std::collections::HashSet) { + let name_options = NameOptions::default(); + let (_r1, mut hier1, _r2, mut hier2) = + wavetools::open_and_read_waves(file1, file2, &name_options) + .expect("Failed to open wave files"); + let filter_strings: Vec = filter.iter().map(|&s| s.to_string()).collect(); + let patterns = wavetools::parse_filter_patterns(&filter_strings) + .expect("Failed to parse filter patterns"); + wavetools::apply_filter(&mut hier1, &patterns); + wavetools::apply_filter(&mut hier2, &patterns); + compare_signal_names(&hier1, &hier2) +} + +#[test] +fn test_filter_matches_only_file1_reports_name_mismatch() { + // counter.sig_name.diff.fst renames cyc_plus_one -> blargh, so the filter + // hits a signal in file1 only. This asymmetry must surface as a diff so + // the user knows their filter scope didn't line up across the two files. + let (only_in_1, only_in_2) = filter_name_mismatch( + "tests/data/counter.fst", + "tests/data/counter.sig_name.diff.fst", + &["*.cyc_plus_one"], + ); + assert!(only_in_1.contains("t.the_sub.cyc_plus_one"), + "Should report cyc_plus_one only in file1, got: {:?}", only_in_1); + assert!(only_in_2.is_empty(), + "Nothing should be only in file2, got: {:?}", only_in_2); +} + +#[test] +fn test_filter_matches_only_file2_reports_name_mismatch() { + // Symmetric case: *.blargh hits the renamed signal in file2 but nothing + // in file1. + let (only_in_1, only_in_2) = filter_name_mismatch( + "tests/data/counter.fst", + "tests/data/counter.sig_name.diff.fst", + &["*.blargh"], + ); + assert!(only_in_2.contains("t.the_sub.blargh"), + "Should report blargh only in file2, got: {:?}", only_in_2); + assert!(only_in_1.is_empty(), + "Nothing should be only in file1, got: {:?}", only_in_1); +} + +#[test] +fn test_cli_filter_matches_only_file1_exits_one() { + let output = run_wavediff_cli(&[ + "--filter", + "*.cyc_plus_one", + "tests/data/counter.fst", + "tests/data/counter.sig_name.diff.fst", + ]); + assert_eq!(output.status.code(), Some(1), + "Filter matching only file1 should exit 1. stderr: {}", + String::from_utf8_lossy(&output.stderr)); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("t.the_sub.cyc_plus_one"), + "stderr should name the asymmetric signal: {}", stderr); +} + +#[test] +fn test_cli_filter_matches_only_file2_exits_one() { + let output = run_wavediff_cli(&[ + "--filter", + "*.blargh", + "tests/data/counter.fst", + "tests/data/counter.sig_name.diff.fst", + ]); + assert_eq!(output.status.code(), Some(1), + "Filter matching only file2 should exit 1. stderr: {}", + String::from_utf8_lossy(&output.stderr)); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("t.the_sub.blargh"), + "stderr should name the asymmetric signal: {}", stderr); +} + +#[test] +fn test_cli_filter_invalid_glob_exits_with_error() { + let output = run_wavediff_cli(&[ + "--filter", + "[", + "tests/data/counter.fst", + "tests/data/counter.fst", + ]); + assert_eq!( + output.status.code(), + Some(2), + "Invalid glob pattern should exit 2 with error" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("Invalid glob pattern"), + "stderr should mention the invalid pattern: {}", + stderr + ); +}