From 22e46e0762e17400116667307173a10645ace183 Mon Sep 17 00:00:00 2001 From: Omar Abdulla Date: Mon, 11 Aug 2025 14:03:20 +0300 Subject: [PATCH] Refactor tests to use steps --- .../files_with_extension_iterator.rs | 10 ++- crates/core/src/driver/mod.rs | 77 +++++++++++++------ crates/core/src/main.rs | 8 +- crates/format/src/case.rs | 24 +++--- crates/format/src/input.rs | 11 +++ 5 files changed, 90 insertions(+), 40 deletions(-) diff --git a/crates/common/src/iterators/files_with_extension_iterator.rs b/crates/common/src/iterators/files_with_extension_iterator.rs index 1892c5f..ac81103 100644 --- a/crates/common/src/iterators/files_with_extension_iterator.rs +++ b/crates/common/src/iterators/files_with_extension_iterator.rs @@ -1,4 +1,8 @@ -use std::{borrow::Cow, collections::HashSet, path::PathBuf}; +use std::{ + borrow::Cow, + collections::HashSet, + path::{Path, PathBuf}, +}; /// An iterator that finds files of a certain extension in the provided directory. You can think of /// this a glob pattern similar to: `${path}/**/*.md` @@ -18,10 +22,10 @@ pub struct FilesWithExtensionIterator { } impl FilesWithExtensionIterator { - pub fn new(root_directory: PathBuf) -> Self { + pub fn new(root_directory: impl AsRef) -> Self { Self { allowed_extensions: Default::default(), - directories_to_search: vec![root_directory], + directories_to_search: vec![root_directory.as_ref().to_path_buf()], files_matching_allowed_extensions: Default::default(), } } diff --git a/crates/core/src/driver/mod.rs b/crates/core/src/driver/mod.rs index c070d38..e0b6bae 100644 --- a/crates/core/src/driver/mod.rs +++ b/crates/core/src/driver/mod.rs @@ -26,9 +26,9 @@ use revive_dt_format::traits::{ResolutionContext, ResolverApi}; use semver::Version; use revive_dt_format::case::{Case, CaseIdx}; -use revive_dt_format::input::{Calldata, EtherValue, Expected, ExpectedOutput, Method}; +use revive_dt_format::input::{Calldata, EtherValue, Expected, ExpectedOutput, Input, Method}; use revive_dt_format::metadata::{ContractInstance, ContractPathAndIdent}; -use revive_dt_format::{input::Input, metadata::Metadata}; +use revive_dt_format::{input::Step, metadata::Metadata}; use revive_dt_node::Node; use revive_dt_node_interaction::EthereumNode; use tracing::Instrument; @@ -70,6 +70,22 @@ where } } + pub async fn handle_step( + &mut self, + metadata: &Metadata, + case_idx: CaseIdx, + step: &Step, + node: &T::Blockchain, + ) -> anyhow::Result { + match step { + Step::FunctionCall(input) => { + let (receipt, geth_trace, diff_mode) = + self.handle_input(metadata, case_idx, input, node).await?; + Ok(StepOutput::FunctionCall(receipt, geth_trace, diff_mode)) + } + } + } + pub async fn handle_input( &mut self, metadata: &Metadata, @@ -78,7 +94,7 @@ where node: &T::Blockchain, ) -> anyhow::Result<(TransactionReceipt, GethTrace, DiffMode)> { let deployment_receipts = self - .handle_contract_deployment(metadata, case_idx, input, node) + .handle_input_contract_deployment(metadata, case_idx, input, node) .await?; let execution_receipt = self .handle_input_execution(input, deployment_receipts, node) @@ -94,7 +110,7 @@ where } /// Handles the contract deployment for a given input performing it if it needs to be performed. - async fn handle_contract_deployment( + async fn handle_input_contract_deployment( &mut self, metadata: &Metadata, case_idx: CaseIdx, @@ -651,38 +667,49 @@ where return Ok(0); } - let mut inputs_executed = 0; - for (input_idx, input) in self.case.inputs_iterator().enumerate() { - let tracing_span = tracing::info_span!("Handling input", input_idx); + let mut steps_executed = 0; + for (step_idx, step) in self.case.steps_iterator().enumerate() { + let tracing_span = tracing::info_span!("Handling input", step_idx); - let (leader_receipt, _, leader_diff) = self + let leader_step_output = self .leader_state - .handle_input(self.metadata, self.case_idx, &input, self.leader_node) + .handle_step(self.metadata, self.case_idx, &step, self.leader_node) .instrument(tracing_span.clone()) .await?; - let (follower_receipt, _, follower_diff) = self + let follower_step_output = self .follower_state - .handle_input(self.metadata, self.case_idx, &input, self.follower_node) + .handle_step(self.metadata, self.case_idx, &step, self.follower_node) .instrument(tracing_span) .await?; + match (leader_step_output, follower_step_output) { + ( + StepOutput::FunctionCall(leader_receipt, _, leader_diff), + StepOutput::FunctionCall(follower_receipt, _, follower_diff), + ) => { + if leader_diff == follower_diff { + tracing::debug!("State diffs match between leader and follower."); + } else { + tracing::debug!("State diffs mismatch between leader and follower."); + Self::trace_diff_mode("Leader", &leader_diff); + Self::trace_diff_mode("Follower", &follower_diff); + } - if leader_diff == follower_diff { - tracing::debug!("State diffs match between leader and follower."); - } else { - tracing::debug!("State diffs mismatch between leader and follower."); - Self::trace_diff_mode("Leader", &leader_diff); - Self::trace_diff_mode("Follower", &follower_diff); + if leader_receipt.logs() != follower_receipt.logs() { + tracing::debug!("Log/event mismatch between leader and follower."); + tracing::trace!("Leader logs: {:?}", leader_receipt.logs()); + tracing::trace!("Follower logs: {:?}", follower_receipt.logs()); + } + } } - if leader_receipt.logs() != follower_receipt.logs() { - tracing::debug!("Log/event mismatch between leader and follower."); - tracing::trace!("Leader logs: {:?}", leader_receipt.logs()); - tracing::trace!("Follower logs: {:?}", follower_receipt.logs()); - } - - inputs_executed += 1; + steps_executed += 1; } - Ok(inputs_executed) + Ok(steps_executed) } } + +#[derive(Clone, Debug)] +pub enum StepOutput { + FunctionCall(TransactionReceipt, GethTrace, DiffMode), +} diff --git a/crates/core/src/main.rs b/crates/core/src/main.rs index 4221b8d..1a87cbc 100644 --- a/crates/core/src/main.rs +++ b/crates/core/src/main.rs @@ -32,7 +32,7 @@ use revive_dt_core::{ use revive_dt_format::{ case::{Case, CaseIdx}, corpus::Corpus, - input::Input, + input::{Input, Step}, metadata::{ContractInstance, ContractPathAndIdent, Metadata, MetadataFile}, mode::SolcMode, }; @@ -446,9 +446,11 @@ where // doing the deployments from different accounts and therefore we're not slowed down by // the nonce. let deployer_address = case - .inputs + .steps .iter() - .map(|input| input.caller) + .filter_map(|step| match step { + Step::FunctionCall(input) => Some(input.caller), + }) .next() .unwrap_or(Input::default_caller()); let leader_tx = TransactionBuilder::::with_deploy_code( diff --git a/crates/format/src/case.rs b/crates/format/src/case.rs index 59572f0..26e803e 100644 --- a/crates/format/src/case.rs +++ b/crates/format/src/case.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; use revive_dt_common::macros::define_wrapper_type; use crate::{ - input::{Expected, Input}, + input::{Expected, Step}, mode::Mode, }; @@ -12,21 +12,27 @@ pub struct Case { pub name: Option, pub comment: Option, pub modes: Option>, - pub inputs: Vec, + #[serde(rename = "inputs")] + pub steps: Vec, pub group: Option, pub expected: Option, pub ignore: Option, } impl Case { - pub fn inputs_iterator(&self) -> impl Iterator { - let inputs_len = self.inputs.len(); - self.inputs + #[allow(irrefutable_let_patterns)] + pub fn steps_iterator(&self) -> impl Iterator { + let steps_len = self.steps.len(); + self.steps .clone() .into_iter() .enumerate() - .map(move |(idx, mut input)| { - if idx + 1 == inputs_len { + .map(move |(idx, mut step)| { + let Step::FunctionCall(ref mut input) = step else { + return step; + }; + + if idx + 1 == steps_len { if input.expected.is_none() { input.expected = self.expected.clone(); } @@ -36,9 +42,9 @@ impl Case { // the case? What are we supposed to do with that final expected field on the // case? - input + step } else { - input + step } }) } diff --git a/crates/format/src/input.rs b/crates/format/src/input.rs index 810ec31..5f95d07 100644 --- a/crates/format/src/input.rs +++ b/crates/format/src/input.rs @@ -17,6 +17,17 @@ use revive_dt_common::macros::define_wrapper_type; use crate::traits::ResolverApi; use crate::{metadata::ContractInstance, traits::ResolutionContext}; +/// A test step. +/// +/// A test step can be anything. It could be an invocation to a function, an assertion, or any other +/// action that needs to be run or executed on the nodes used in the tests. +#[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)] +#[serde(untagged)] +pub enum Step { + /// A function call or an invocation to some function on some smart contract. + FunctionCall(Input), +} + #[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)] pub struct Input { #[serde(default = "Input::default_caller")]