Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for test skip reasons #8858

Merged
merged 2 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions crates/cheatcodes/assets/cheatcodes.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion crates/cheatcodes/spec/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -843,10 +843,14 @@ interface Vm {
#[cheatcode(group = Testing, safety = Unsafe)]
function expectSafeMemoryCall(uint64 min, uint64 max) external;

/// Marks a test as skipped. Must be called at the top of the test.
/// Marks a test as skipped. Must be called at the top level of a test.
#[cheatcode(group = Testing, safety = Unsafe)]
function skip(bool skipTest) external;

/// Marks a test as skipped with a reason. Must be called at the top level of a test.
#[cheatcode(group = Testing, safety = Unsafe)]
function skip(bool skipTest, string calldata reason) external;

/// Asserts that the given condition is true.
#[cheatcode(group = Testing, safety = Safe)]
function assertTrue(bool condition) external pure;
Expand Down
13 changes: 10 additions & 3 deletions crates/cheatcodes/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,21 @@ impl Cheatcode for sleepCall {
}
}

impl Cheatcode for skipCall {
impl Cheatcode for skip_0Call {
fn apply_stateful<DB: DatabaseExt>(&self, ccx: &mut CheatsCtxt<DB>) -> Result {
let Self { skipTest } = *self;
if skipTest {
skip_1Call { skipTest, reason: String::new() }.apply_stateful(ccx)
}
}

impl Cheatcode for skip_1Call {
fn apply_stateful<DB: DatabaseExt>(&self, ccx: &mut CheatsCtxt<DB>) -> Result {
let Self { skipTest, reason } = self;
if *skipTest {
// Skip should not work if called deeper than at test level.
// Since we're not returning the magic skip bytes, this will cause a test failure.
ensure!(ccx.ecx.journaled_state.depth() <= 1, "`skip` can only be used at test level");
Err(MAGIC_SKIP.into())
Err([MAGIC_SKIP, reason.as_bytes()].concat().into())
} else {
Ok(Default::default())
}
Expand Down
2 changes: 1 addition & 1 deletion crates/evm/core/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub const TEST_CONTRACT_ADDRESS: Address = address!("b4c79daB8f259C7Aee6E5b2Aa72
/// Magic return value returned by the `assume` cheatcode.
pub const MAGIC_ASSUME: &[u8] = b"FOUNDRY::ASSUME";

/// Magic return value returned by the `skip` cheatcode.
/// Magic return value returned by the `skip` cheatcode. Optionally appended with a reason.
pub const MAGIC_SKIP: &[u8] = b"FOUNDRY::SKIP";

/// The address that deploys the default CREATE2 deployer contract.
Expand Down
57 changes: 47 additions & 10 deletions crates/evm/core/src/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,39 @@ use foundry_common::SELECTOR_LEN;
use itertools::Itertools;
use revm::interpreter::InstructionResult;
use rustc_hash::FxHashMap;
use std::sync::OnceLock;
use std::{fmt, sync::OnceLock};

/// A skip reason.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct SkipReason(pub Option<String>);

impl SkipReason {
/// Decodes a skip reason, if any.
pub fn decode(raw_result: &[u8]) -> Option<Self> {
raw_result.strip_prefix(crate::constants::MAGIC_SKIP).map(|reason| {
let reason = String::from_utf8_lossy(reason).into_owned();
Self((!reason.is_empty()).then_some(reason))
})
}

/// Decodes a skip reason from a string that was obtained by formatting `Self`.
///
/// This is a hack to support re-decoding a skip reason in proptest.
pub fn decode_self(s: &str) -> Option<Self> {
s.strip_prefix("skipped").map(|rest| Self(rest.strip_prefix(": ").map(ToString::to_string)))
}
}

impl fmt::Display for SkipReason {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str("skipped")?;
if let Some(reason) = &self.0 {
f.write_str(": ")?;
f.write_str(reason)?;
}
Ok(())
}
}

/// Decode a set of logs, only returning logs from DSTest logging events and Hardhat's `console.log`
pub fn decode_console_logs(logs: &[Log]) -> Vec<String> {
Expand Down Expand Up @@ -120,9 +152,8 @@ impl RevertDecoder {
};
}

if err == crate::constants::MAGIC_SKIP {
// Also used in forge fuzz runner
return Some("SKIPPED".to_string());
if let Some(reason) = SkipReason::decode(err) {
return Some(reason.to_string());
}

// Solidity's `Error(string)` or `Panic(uint256)`
Expand Down Expand Up @@ -177,11 +208,17 @@ impl RevertDecoder {
}

// Generic custom error.
Some(format!(
"custom error {}:{}",
hex::encode(selector),
std::str::from_utf8(data).map_or_else(|_| trimmed_hex(data), String::from)
))
Some({
let mut s = format!("custom error {}", hex::encode_prefixed(selector));
if !data.is_empty() {
s.push_str(": ");
match std::str::from_utf8(data) {
Ok(data) => s.push_str(data),
Err(_) => s.push_str(&trimmed_hex(data)),
}
}
s
})
}
}

Expand All @@ -194,7 +231,7 @@ fn trimmed_hex(s: &[u8]) -> String {
"{}…{} ({} bytes)",
&hex::encode(&s[..n / 2]),
&hex::encode(&s[s.len() - n / 2..]),
s.len()
s.len(),
)
}
}
Expand Down
45 changes: 28 additions & 17 deletions crates/evm/evm/src/executors/fuzz/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ use alloy_primitives::{Address, Bytes, Log, U256};
use eyre::Result;
use foundry_common::evm::Breakpoints;
use foundry_config::FuzzConfig;
use foundry_evm_core::{constants::MAGIC_ASSUME, decode::RevertDecoder};
use foundry_evm_core::{
constants::MAGIC_ASSUME,
decode::{RevertDecoder, SkipReason},
};
use foundry_evm_coverage::HitMaps;
use foundry_evm_fuzz::{
strategies::{fuzz_calldata, fuzz_calldata_from_state, EvmFuzzState},
Expand Down Expand Up @@ -131,9 +134,8 @@ impl FuzzedExecutor {
}) => {
// We cannot use the calldata returned by the test runner in `TestError::Fail`,
// since that input represents the last run case, which may not correspond with
// our failure - when a fuzz case fails, proptest will try
// to run at least one more case to find a minimal failure
// case.
// our failure - when a fuzz case fails, proptest will try to run at least one
// more case to find a minimal failure case.
let reason = rd.maybe_decode(&outcome.1.result, Some(status));
execution_data.borrow_mut().logs.extend(outcome.1.logs.clone());
execution_data.borrow_mut().counterexample = outcome;
Expand All @@ -157,6 +159,7 @@ impl FuzzedExecutor {
first_case: fuzz_result.first_case.unwrap_or_default(),
gas_by_case: fuzz_result.gas_by_case,
success: run_result.is_ok(),
skipped: false,
reason: None,
counterexample: None,
logs: fuzz_result.logs,
Expand All @@ -168,20 +171,22 @@ impl FuzzedExecutor {
};

match run_result {
// Currently the only operation that can trigger proptest global rejects is the
// `vm.assume` cheatcode, thus we surface this info to the user when the fuzz test
// aborts due to too many global rejects, making the error message more actionable.
Err(TestError::Abort(reason)) if reason.message() == "Too many global rejects" => {
result.reason = Some(
FuzzError::TooManyRejects(self.runner.config().max_global_rejects).to_string(),
);
}
Ok(()) => {}
Err(TestError::Abort(reason)) => {
result.reason = Some(reason.to_string());
let msg = reason.message();
// Currently the only operation that can trigger proptest global rejects is the
// `vm.assume` cheatcode, thus we surface this info to the user when the fuzz test
// aborts due to too many global rejects, making the error message more actionable.
result.reason = if msg == "Too many global rejects" {
let error = FuzzError::TooManyRejects(self.runner.config().max_global_rejects);
Some(error.to_string())
} else {
Some(msg.to_string())
};
}
Err(TestError::Fail(reason, _)) => {
let reason = reason.to_string();
result.reason = if reason.is_empty() { None } else { Some(reason) };
result.reason = (!reason.is_empty()).then_some(reason);

let args = if let Some(data) = calldata.get(4..) {
func.abi_decode_input(data, false).unwrap_or_default()
Expand All @@ -193,7 +198,13 @@ impl FuzzedExecutor {
BaseCounterExample::from_fuzz_call(calldata, args, call.traces),
));
}
_ => {}
}

if let Some(reason) = &result.reason {
if let Some(reason) = SkipReason::decode_self(reason) {
result.skipped = true;
result.reason = reason.0;
}
}

state.log_stats();
Expand All @@ -212,9 +223,9 @@ impl FuzzedExecutor {
let mut call = self
.executor
.call_raw(self.sender, address, calldata.clone(), U256::ZERO)
.map_err(|_| TestCaseError::fail(FuzzError::FailedContractCall))?;
.map_err(|e| TestCaseError::fail(e.to_string()))?;

// When the `assume` cheatcode is called it returns a special string
// Handle `vm.assume`.
if call.result.as_ref() == MAGIC_ASSUME {
return Err(TestCaseError::reject(FuzzError::AssumeReject))
}
Expand Down
6 changes: 3 additions & 3 deletions crates/evm/evm/src/executors/invariant/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ impl InvariantFuzzError {
Self::BrokenInvariant(case_data) | Self::Revert(case_data) => {
(!case_data.revert_reason.is_empty()).then(|| case_data.revert_reason.clone())
}
Self::MaxAssumeRejects(allowed) => Some(format!(
"The `vm.assume` cheatcode rejected too many inputs ({allowed} allowed)"
)),
Self::MaxAssumeRejects(allowed) => {
Some(format!("`vm.assume` rejected too many inputs ({allowed} allowed)"))
}
}
}
}
Expand Down
20 changes: 10 additions & 10 deletions crates/evm/evm/src/executors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use foundry_evm_core::{
CALLER, CHEATCODE_ADDRESS, CHEATCODE_CONTRACT_HASH, DEFAULT_CREATE2_DEPLOYER,
DEFAULT_CREATE2_DEPLOYER_CODE, DEFAULT_CREATE2_DEPLOYER_DEPLOYER,
},
decode::RevertDecoder,
decode::{RevertDecoder, SkipReason},
utils::StateChangeset,
};
use foundry_evm_coverage::HitMaps;
Expand Down Expand Up @@ -649,15 +649,15 @@ impl std::ops::DerefMut for ExecutionErr {

#[derive(Debug, thiserror::Error)]
pub enum EvmError {
/// Error which occurred during execution of a transaction
/// Error which occurred during execution of a transaction.
#[error(transparent)]
Execution(#[from] Box<ExecutionErr>),
/// Error which occurred during ABI encoding/decoding
/// Error which occurred during ABI encoding/decoding.
#[error(transparent)]
AbiError(#[from] alloy_dyn_abi::Error),
/// Error caused which occurred due to calling the skip() cheatcode.
#[error("Skipped")]
SkipError,
Abi(#[from] alloy_dyn_abi::Error),
/// Error caused which occurred due to calling the `skip` cheatcode.
#[error("{_0}")]
Skip(SkipReason),
/// Any other error.
#[error(transparent)]
Eyre(#[from] eyre::Error),
Expand All @@ -671,7 +671,7 @@ impl From<ExecutionErr> for EvmError {

impl From<alloy_sol_types::Error> for EvmError {
fn from(err: alloy_sol_types::Error) -> Self {
Self::AbiError(err.into())
Self::Abi(err.into())
}
}

Expand Down Expand Up @@ -769,8 +769,8 @@ impl Default for RawCallResult {
impl RawCallResult {
/// Converts the result of the call into an `EvmError`.
pub fn into_evm_error(self, rd: Option<&RevertDecoder>) -> EvmError {
if self.result[..] == crate::constants::MAGIC_SKIP[..] {
return EvmError::SkipError;
if let Some(reason) = SkipReason::decode(&self.result) {
return EvmError::Skip(reason);
}
let reason = rd.unwrap_or_default().decode(&self.result, Some(self.exit_reason));
EvmError::Execution(Box::new(self.into_execution_error(reason)))
Expand Down
9 changes: 3 additions & 6 deletions crates/evm/fuzz/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
//! errors related to fuzz tests
//! Errors related to fuzz tests.

use proptest::test_runner::Reason;

/// Possible errors when running fuzz tests
#[derive(Debug, thiserror::Error)]
pub enum FuzzError {
#[error("Couldn't call unknown contract")]
UnknownContract,
#[error("Failed contract call")]
FailedContractCall,
#[error("`vm.assume` reject")]
AssumeReject,
#[error("The `vm.assume` cheatcode rejected too many inputs ({0} allowed)")]
#[error("`vm.assume` rejected too many inputs ({0} allowed)")]
TooManyRejects(u32),
}

Expand Down
2 changes: 2 additions & 0 deletions crates/evm/fuzz/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ pub struct FuzzTestResult {
/// properly, or that there was a revert and that the test was expected to fail
/// (prefixed with `testFail`)
pub success: bool,
/// Whether the test case was skipped. `reason` will contain the skip reason, if any.
pub skipped: bool,

/// If there was a revert, this field will be populated. Note that the test can
/// still be successful (i.e self.success == true) when it's expected to fail.
Expand Down
Loading
Loading