Skip to content

Commit 660b5d2

Browse files
DaniPopesDaniPopes
committed
fix: only identify traces if they should be included
1 parent a6f7af3 commit 660b5d2

File tree

2 files changed

+116
-120
lines changed

2 files changed

+116
-120
lines changed

‎crates/chisel/src/dispatcher.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -958,8 +958,9 @@ impl ChiselDispatcher {
958958
session_config.evm_opts.get_remote_chain_id(),
959959
)?;
960960

961-
let mut decoder =
962-
CallTraceDecoderBuilder::new().with_labels(result.labeled_addresses.clone()).build();
961+
let mut decoder = CallTraceDecoderBuilder::new()
962+
.with_labels(result.labeled_addresses.iter().map(|(a, s)| (*a, s.clone())))
963+
.build();
963964

964965
decoder.add_signature_identifier(SignaturesIdentifier::new(
965966
Config::foundry_cache_dir(),

‎crates/forge/bin/cmd/test/mod.rs‎

Lines changed: 113 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -537,152 +537,147 @@ async fn test(
537537
if json {
538538
let results = runner.test(filter, None, test_options).await;
539539
println!("{}", serde_json::to_string(&results)?);
540-
Ok(TestOutcome::new(results, allow_failure))
541-
} else {
542-
// Set up identifiers
543-
let known_contracts = runner.known_contracts.clone();
544-
let mut local_identifier = LocalTraceIdentifier::new(&known_contracts);
545-
let remote_chain_id = runner.evm_opts.get_remote_chain_id();
546-
// Do not re-query etherscan for contracts that you've already queried today.
547-
let mut etherscan_identifier = EtherscanIdentifier::new(&config, remote_chain_id)?;
540+
return Ok(TestOutcome::new(results, allow_failure))
541+
}
548542

549-
// Set up test reporter channel
550-
let (tx, rx) = channel::<(String, SuiteResult)>();
543+
// Set up identifiers
544+
let known_contracts = runner.known_contracts.clone();
545+
let mut local_identifier = LocalTraceIdentifier::new(&known_contracts);
546+
let remote_chain_id = runner.evm_opts.get_remote_chain_id();
547+
// Do not re-query etherscan for contracts that you've already queried today.
548+
let mut etherscan_identifier = EtherscanIdentifier::new(&config, remote_chain_id)?;
551549

552-
// Run tests
553-
let handle =
554-
tokio::task::spawn(async move { runner.test(filter, Some(tx), test_options).await });
550+
// Set up test reporter channel
551+
let (tx, rx) = channel::<(String, SuiteResult)>();
555552

556-
let mut results: BTreeMap<String, SuiteResult> = BTreeMap::new();
557-
let mut gas_report = GasReport::new(config.gas_reports, config.gas_reports_ignore);
558-
let sig_identifier =
559-
SignaturesIdentifier::new(Config::foundry_cache_dir(), config.offline)?;
553+
// Run tests
554+
let handle =
555+
tokio::task::spawn(async move { runner.test(filter, Some(tx), test_options).await });
560556

561-
let mut total_passed = 0;
562-
let mut total_failed = 0;
563-
let mut total_skipped = 0;
557+
let mut results: BTreeMap<String, SuiteResult> = BTreeMap::new();
558+
let mut gas_report = GasReport::new(config.gas_reports, config.gas_reports_ignore);
559+
let sig_identifier = SignaturesIdentifier::new(Config::foundry_cache_dir(), config.offline)?;
564560

565-
'outer: for (contract_name, suite_result) in rx {
566-
results.insert(contract_name.clone(), suite_result.clone());
561+
let mut total_passed = 0;
562+
let mut total_failed = 0;
563+
let mut total_skipped = 0;
567564

568-
let mut tests = suite_result.test_results.clone();
569-
println!();
570-
for warning in suite_result.warnings.iter() {
571-
eprintln!("{} {warning}", Paint::yellow("Warning:").bold());
572-
}
573-
if !tests.is_empty() {
574-
let term = if tests.len() > 1 { "tests" } else { "test" };
575-
println!("Running {} {term} for {contract_name}", tests.len());
565+
'outer: for (contract_name, suite_result) in rx {
566+
results.insert(contract_name.clone(), suite_result.clone());
567+
568+
let mut tests = suite_result.test_results.clone();
569+
println!();
570+
for warning in suite_result.warnings.iter() {
571+
eprintln!("{} {warning}", Paint::yellow("Warning:").bold());
572+
}
573+
if !tests.is_empty() {
574+
let term = if tests.len() > 1 { "tests" } else { "test" };
575+
println!("Running {} {term} for {contract_name}", tests.len());
576+
}
577+
for (name, result) in &mut tests {
578+
short_test_result(name, result);
579+
580+
// If the test failed, we want to stop processing the rest of the tests
581+
if fail_fast && result.status == TestStatus::Failure {
582+
break 'outer
576583
}
577-
for (name, result) in &mut tests {
578-
short_test_result(name, result);
579584

580-
// If the test failed, we want to stop processing the rest of the tests
581-
if fail_fast && result.status == TestStatus::Failure {
582-
break 'outer
585+
// We only display logs at level 2 and above
586+
if verbosity >= 2 {
587+
// We only decode logs from Hardhat and DS-style console events
588+
let console_logs = decode_console_logs(&result.logs);
589+
if !console_logs.is_empty() {
590+
println!("Logs:");
591+
for log in console_logs {
592+
println!(" {log}");
593+
}
594+
println!();
583595
}
596+
}
584597

585-
// We only display logs at level 2 and above
586-
if verbosity >= 2 {
587-
// We only decode logs from Hardhat and DS-style console events
588-
let console_logs = decode_console_logs(&result.logs);
589-
if !console_logs.is_empty() {
590-
println!("Logs:");
591-
for log in console_logs {
592-
println!(" {log}");
593-
}
594-
println!();
595-
}
598+
if !result.traces.is_empty() {
599+
// Identify addresses in each trace
600+
let mut decoder = CallTraceDecoderBuilder::new()
601+
.with_labels(result.labeled_addresses.iter().map(|(a, s)| (*a, s.clone())))
602+
.with_events(local_identifier.events().cloned())
603+
.with_verbosity(verbosity)
604+
.build();
605+
606+
// Signatures are of no value for gas reports
607+
if !gas_reporting {
608+
decoder.add_signature_identifier(sig_identifier.clone());
596609
}
597610

598-
if !result.traces.is_empty() {
599-
// Identify addresses in each trace
600-
let mut decoder = CallTraceDecoderBuilder::new()
601-
.with_labels(result.labeled_addresses.clone())
602-
.with_events(local_identifier.events().cloned())
603-
.with_verbosity(verbosity)
604-
.build();
605-
606-
// Signatures are of no value for gas reports
607-
if !gas_reporting {
608-
decoder.add_signature_identifier(sig_identifier.clone());
609-
}
611+
// Decode the traces
612+
let mut decoded_traces = Vec::with_capacity(result.traces.len());
613+
for (kind, trace) in &mut result.traces {
614+
let should_include = match kind {
615+
// At verbosity level 3, we only display traces for failed tests
616+
// At verbosity level 4, we also display the setup trace for failed
617+
// tests At verbosity level 5, we display
618+
// all traces for all tests
619+
TraceKind::Setup => {
620+
(verbosity >= 5) ||
621+
(verbosity == 4 && result.status == TestStatus::Failure)
622+
}
623+
TraceKind::Execution => {
624+
verbosity > 3 ||
625+
(verbosity == 3 && result.status == TestStatus::Failure)
626+
}
627+
_ => false,
628+
};
610629

611-
// Decode the traces
612-
let mut decoded_traces = Vec::new();
613-
for (kind, trace) in &mut result.traces {
630+
if should_include {
614631
decoder.identify(trace, &mut local_identifier);
615632
decoder.identify(trace, &mut etherscan_identifier);
616-
617-
let should_include = match kind {
618-
// At verbosity level 3, we only display traces for failed tests
619-
// At verbosity level 4, we also display the setup trace for failed
620-
// tests At verbosity level 5, we display
621-
// all traces for all tests
622-
TraceKind::Setup => {
623-
(verbosity >= 5) ||
624-
(verbosity == 4 && result.status == TestStatus::Failure)
625-
}
626-
TraceKind::Execution => {
627-
verbosity > 3 ||
628-
(verbosity == 3 && result.status == TestStatus::Failure)
629-
}
630-
_ => false,
631-
};
632-
633-
// We decode the trace if we either need to build a gas report or we need
634-
// to print it
635-
if should_include || gas_reporting {
636-
decoder.decode(trace).await;
637-
}
638-
639-
if should_include {
640-
decoded_traces.push(trace.to_string());
641-
}
642633
}
643634

644-
if !decoded_traces.is_empty() {
645-
println!("Traces:");
646-
decoded_traces.into_iter().for_each(|trace| println!("{trace}"));
635+
// We decode the trace if we either need to build a gas report or we need
636+
// to print it
637+
if should_include || gas_reporting {
638+
decoder.decode(trace).await;
647639
}
648640

649-
if gas_reporting {
650-
gas_report.analyze(&result.traces);
641+
if should_include {
642+
decoded_traces.push(trace.to_string());
651643
}
652644
}
653-
}
654-
let block_outcome =
655-
TestOutcome::new([(contract_name, suite_result)].into(), allow_failure);
656645

657-
total_passed += block_outcome.successes().count();
658-
total_failed += block_outcome.failures().count();
659-
total_skipped += block_outcome.skips().count();
646+
if !decoded_traces.is_empty() {
647+
println!("Traces:");
648+
decoded_traces.into_iter().for_each(|trace| println!("{trace}"));
649+
}
660650

661-
println!("{}", block_outcome.summary());
651+
if gas_reporting {
652+
gas_report.analyze(&result.traces);
653+
}
654+
}
662655
}
656+
let block_outcome = TestOutcome::new([(contract_name, suite_result)].into(), allow_failure);
663657

664-
if gas_reporting {
665-
println!("{}", gas_report.finalize());
666-
}
658+
total_passed += block_outcome.successes().count();
659+
total_failed += block_outcome.failures().count();
660+
total_skipped += block_outcome.skips().count();
667661

668-
let num_test_suites = results.len();
662+
println!("{}", block_outcome.summary());
663+
}
669664

670-
if num_test_suites > 0 {
671-
println!(
672-
"{}",
673-
format_aggregated_summary(
674-
num_test_suites,
675-
total_passed,
676-
total_failed,
677-
total_skipped
678-
)
679-
);
680-
}
665+
if gas_reporting {
666+
println!("{}", gas_report.finalize());
667+
}
681668

682-
// reattach the thread
683-
let _results = handle.await?;
669+
let num_test_suites = results.len();
684670

685-
trace!(target: "forge::test", "received {} results", results.len());
686-
Ok(TestOutcome::new(results, allow_failure))
671+
if num_test_suites > 0 {
672+
println!(
673+
"{}",
674+
format_aggregated_summary(num_test_suites, total_passed, total_failed, total_skipped)
675+
);
687676
}
677+
678+
// reattach the thread
679+
let _results = handle.await?;
680+
681+
trace!(target: "forge::test", "received {} results", results.len());
682+
Ok(TestOutcome::new(results, allow_failure))
688683
}

0 commit comments

Comments
 (0)