Skip to content

Commit 082fc7e

Browse files
silvekkkonbjerg
andauthored
feat(forge): enhance gas snapshot diff with configurable sorting and improved output (#11974)
* feat(forge): enhance gas snapshot diff with configurable sorting and improved output * fix fmt * fix ci --------- Co-authored-by: onbjerg <[email protected]>
1 parent f5322d9 commit 082fc7e

File tree

1 file changed

+104
-7
lines changed

1 file changed

+104
-7
lines changed

crates/forge/src/cmd/snapshot.rs

Lines changed: 104 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ pub struct GasSnapshotArgs {
7474
)]
7575
tolerance: Option<u32>,
7676

77+
/// How to sort diff results.
78+
#[arg(long, value_name = "ORDER")]
79+
diff_sort: Option<DiffSortOrder>,
80+
7781
/// All test arguments are supported
7882
#[command(flatten)]
7983
pub(crate) test: test::TestArgs,
@@ -105,7 +109,7 @@ impl GasSnapshotArgs {
105109
if let Some(path) = self.diff {
106110
let snap = path.as_ref().unwrap_or(&self.snap);
107111
let snaps = read_gas_snapshot(snap)?;
108-
diff(tests, snaps)?;
112+
diff(tests, snaps, self.diff_sort.unwrap_or_default())?;
109113
} else if let Some(path) = self.check {
110114
let snap = path.as_ref().unwrap_or(&self.snap);
111115
let snaps = read_gas_snapshot(snap)?;
@@ -162,6 +166,20 @@ struct GasSnapshotConfig {
162166
max: Option<u64>,
163167
}
164168

169+
/// Sort order for diff output
170+
#[derive(Clone, Debug, Default, clap::ValueEnum)]
171+
enum DiffSortOrder {
172+
/// Sort by percentage change (smallest to largest) - default behavior
173+
#[default]
174+
Percentage,
175+
/// Sort by percentage change (largest to smallest)
176+
PercentageDesc,
177+
/// Sort by absolute gas change (smallest to largest)
178+
Absolute,
179+
/// Sort by absolute gas change (largest to smallest)
180+
AbsoluteDesc,
181+
}
182+
165183
impl GasSnapshotConfig {
166184
fn is_in_gas_range(&self, gas_used: u64) -> bool {
167185
if let Some(min) = self.min
@@ -387,42 +405,121 @@ fn check(
387405
}
388406

389407
/// Compare the set of tests with an existing gas snapshot.
390-
fn diff(tests: Vec<SuiteTestResult>, snaps: Vec<GasSnapshotEntry>) -> Result<()> {
408+
fn diff(
409+
tests: Vec<SuiteTestResult>,
410+
snaps: Vec<GasSnapshotEntry>,
411+
sort_order: DiffSortOrder,
412+
) -> Result<()> {
391413
let snaps = snaps
392414
.into_iter()
393415
.map(|s| ((s.contract_name, s.signature), s.gas_used))
394416
.collect::<HashMap<_, _>>();
395417
let mut diffs = Vec::with_capacity(tests.len());
418+
let mut new_tests = Vec::new();
419+
396420
for test in tests.into_iter() {
397421
if let Some(target_gas_used) =
398422
snaps.get(&(test.contract_name().to_string(), test.signature.clone())).cloned()
399423
{
400424
diffs.push(GasSnapshotDiff {
401425
source_gas_used: test.result.kind.report(),
402-
signature: test.signature,
426+
signature: format!("{}::{}", test.contract_name(), test.signature),
403427
target_gas_used,
404428
});
429+
} else {
430+
// Track new tests
431+
new_tests.push(format!("{}::{}", test.contract_name(), test.signature));
405432
}
406433
}
434+
435+
let mut increased = 0;
436+
let mut decreased = 0;
437+
let mut unchanged = 0;
407438
let mut overall_gas_change = 0i128;
408439
let mut overall_gas_used = 0i128;
409440

410-
diffs.sort_by(|a, b| a.gas_diff().abs().total_cmp(&b.gas_diff().abs()));
441+
// Sort based on user preference
442+
match sort_order {
443+
DiffSortOrder::Percentage => {
444+
// Default: sort by percentage change (smallest to largest)
445+
diffs.sort_by(|a, b| a.gas_diff().abs().total_cmp(&b.gas_diff().abs()));
446+
}
447+
DiffSortOrder::PercentageDesc => {
448+
// Sort by percentage change (largest to smallest)
449+
diffs.sort_by(|a, b| b.gas_diff().abs().total_cmp(&a.gas_diff().abs()));
450+
}
451+
DiffSortOrder::Absolute => {
452+
// Sort by absolute gas change (smallest to largest)
453+
diffs.sort_by_key(|d| d.gas_change().abs());
454+
}
455+
DiffSortOrder::AbsoluteDesc => {
456+
// Sort by absolute gas change (largest to smallest)
457+
diffs.sort_by_key(|d| std::cmp::Reverse(d.gas_change().abs()));
458+
}
459+
}
411460

412-
for diff in diffs {
461+
for diff in &diffs {
413462
let gas_change = diff.gas_change();
414463
overall_gas_change += gas_change;
415464
overall_gas_used += diff.target_gas_used.gas() as i128;
416465
let gas_diff = diff.gas_diff();
466+
467+
// Classify changes
468+
if gas_change > 0 {
469+
increased += 1;
470+
} else if gas_change < 0 {
471+
decreased += 1;
472+
} else {
473+
unchanged += 1;
474+
}
475+
476+
// Display with icon and before/after values
477+
let icon = if gas_change > 0 {
478+
"↑".red().to_string()
479+
} else if gas_change < 0 {
480+
"↓".green().to_string()
481+
} else {
482+
"━".to_string()
483+
};
484+
417485
sh_println!(
418-
"{} (gas: {} ({})) ",
486+
"{} {} (gas: {} → {} | {} {})",
487+
icon,
419488
diff.signature,
489+
diff.target_gas_used.gas(),
490+
diff.source_gas_used.gas(),
420491
fmt_change(gas_change),
421492
fmt_pct_change(gas_diff)
422493
)?;
423494
}
424495

425-
let overall_gas_diff = overall_gas_change as f64 / overall_gas_used as f64;
496+
// Display new tests if any
497+
if !new_tests.is_empty() {
498+
sh_println!("\n{}", "New tests:".yellow())?;
499+
for test in new_tests {
500+
sh_println!(" {} {}", "+".green(), test)?;
501+
}
502+
}
503+
504+
// Summary separator
505+
sh_println!("\n{}", "-".repeat(80))?;
506+
507+
let overall_gas_diff = if overall_gas_used > 0 {
508+
overall_gas_change as f64 / overall_gas_used as f64
509+
} else {
510+
0.0
511+
};
512+
513+
sh_println!(
514+
"Total tests: {}, {} {}, {} {}, {} {}",
515+
diffs.len(),
516+
"↑".red().to_string(),
517+
increased,
518+
"↓".green().to_string(),
519+
decreased,
520+
"━",
521+
unchanged
522+
)?;
426523
sh_println!(
427524
"Overall gas change: {} ({})",
428525
fmt_change(overall_gas_change),

0 commit comments

Comments
 (0)