feat: add support for test skip reasons (#8858)

Author: DaniPopes

crates/cheatcodes/assets/cheatcodes.json

@@ -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;

crates/cheatcodes/spec/src/vm.rs

@@ -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())
         }

crates/cheatcodes/src/test.rs

@@ -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.

crates/evm/core/src/constants.rs

@@ -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> {
@@ -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)`
@@ -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
+        })
     }
 }
 
@@ -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(),
         )
     }
 }

crates/evm/core/src/decode.rs

@@ -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},
@@ -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;
@@ -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,
@@ -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()
@@ -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();
@@ -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))
         }

crates/evm/evm/src/executors/fuzz/mod.rs

@@ -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)"))
+            }
         }
     }
 }

crates/evm/evm/src/executors/invariant/error.rs

@@ -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;
@@ -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),
@@ -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())
     }
 }
 
@@ -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)))

crates/evm/evm/src/executors/mod.rs

@@ -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),
 }
 

crates/evm/fuzz/src/error.rs

@@ -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.