remove comment and bad error-surfacing logic

foundry-rs · Oct 24, 2024 · 59efe2d · 59efe2d
1 parent 3d057c1
commit 59efe2d
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 37 deletions.
diff --git a/crates/cheatcodes/src/test/assume.rs b/crates/cheatcodes/src/test/assume.rs
@@ -187,29 +187,14 @@ pub(crate) fn handle_assume_no_revert(
     known_contracts: &Option<ContractsByArtifact>,
     reverter: Option<&Address>,
 ) -> Result<()> {
-    // iterate over acceptable reasons and try to match against any, otherwise, return an Error with
-    // the revert data
-    assume_no_revert.reasons.as_ref().map_or_else(
-        || {
-            // todo: fix this hack to get cheatcode name in error message
-            let retdata_str = retdata.to_string();
-            if retdata_str.contains(ASSUME_REJECT_MAGIC) ||
-                retdata_str.contains(ASSUME_EXPECT_REJECT_MAGIC)
-            {
-                // raise error with retdata as a string, so apply_dispatch will insert the cheatcode
-                // name
-                Err(Error::from(String::from_utf8(retdata.to_vec()).unwrap()))
-            } else {
-                Ok(())
-            }
-        },
-        |reasons| {
-            reasons
-                .iter()
-                .find_map(|reason| {
-                    handle_revert(false, reason, status, retdata, known_contracts, reverter).ok()
-                })
-                .ok_or_else(|| retdata.clone().into())
-        },
-    )
+    // if a generic assumeNoRevert, return Ok(). Otherwise, iterate over acceptable reasons and try
+    // to match against any, otherwise, return an Error with the revert data
+    assume_no_revert.reasons.as_ref().map_or(Ok(()), |reasons| {
+        reasons
+            .iter()
+            .find_map(|reason| {
+                handle_revert(false, reason, status, retdata, known_contracts, reverter).ok()
+            })
+            .ok_or_else(|| retdata.clone().into())
+    })
 }
diff --git a/crates/cheatcodes/src/test/revert.rs b/crates/cheatcodes/src/test/revert.rs
@@ -44,7 +44,6 @@ pub(crate) fn handle_revert(
         bail!("call reverted as expected, but without data");
     }
 
-    // todo: would love to not copy here but types don't seem to work easily
     let mut actual_revert: Vec<u8> = retdata.to_vec();
 
     // Compare only the first 4 bytes if partial match.

diff --git a/crates/forge/tests/cli/test_cmd.rs b/crates/forge/tests/cli/test_cmd.rs
@@ -2556,12 +2556,12 @@ forgetest_init!(assume_no_revert_tests, |prj, cmd| {
             reverter.twoPossibleReverts(2);
         }
  
-        /// @dev Test that `assumeNoRevert` correctly rejects any error selector when no selector is provided
-        function testMultipleAssumes_ThrowOnGenericNoRevert_fails(bytes4 selector) public view {
-            _vm.assumeNoRevert();
-            _vm.assumeNoRevert(selector);
-            reverter.twoPossibleReverts(2);
-        }
+        // /// @dev Test that `assumeNoRevert` correctly rejects any error selector when no selector is provided
+        // function testMultipleAssumes_ThrowOnGenericNoRevert_fails(bytes4 selector) public view {
+        //     _vm.assumeNoRevert();
+        //     _vm.assumeNoRevert(selector);
+        //     reverter.twoPossibleReverts(2);
+        // }
     
         /// @dev Test that `assumeNoRevert` correctly rejects a generic assumeNoRevert call after any specific reason is provided
         function testMultipleAssumes_ThrowOnGenericNoRevert_AfterSpecific_fails(bytes4 selector) public view {
@@ -2629,11 +2629,14 @@ forgetest_init!(assume_no_revert_tests, |prj, cmd| {
         "testMultipleAssumesClearAfterCall_fails",
         "FAIL: MyRevert(); counterexample:",
     );
-    assert_failure_contains(
-        &mut cmd,
-        "testMultipleAssumes_ThrowOnGenericNoRevert_fails",
-        "FAIL: vm.assumeNoRevert: cannot combine a generic assumeNoRevert with specific assumeNoRevert reasons;",
-    );
+    // need a better way to handle cheatcodes reverting; currently their messages get abi-encoded
+    // and treated like normal evm data, which makes them hard (and inefficient) to match in the
+    // handler
+    // assert_failure_contains(
+    //     &mut cmd,
+    //     "testMultipleAssumes_ThrowOnGenericNoRevert_fails",
+    //     "FAIL: vm.assumeNoRevert: cannot combine a generic assumeNoRevert with specific
+    // assumeNoRevert reasons;", );
     assert_failure_contains(
         &mut cmd,
         "testMultipleAssumes_ThrowOnGenericNoRevert_AfterSpecific_fails",