Implement should_revert for in test runner

Author: cburgdorf

crates/codegen/src/lib.rs

@@ -1,6 +1,6 @@
 mod yul;
 
 pub use yul::{
-    EmitModuleError, TestMetadata, TestModuleOutput, YulError, emit_module_yul,
+    EmitModuleError, ExpectedRevert, TestMetadata, TestModuleOutput, YulError, emit_module_yul,
     emit_module_yul_with_layout, emit_test_module_yul, emit_test_module_yul_with_layout,
 };

crates/codegen/src/yul/emitter/mod.rs

@@ -3,7 +3,7 @@ use std::fmt;
 use crate::yul::errors::YulError;
 
 pub use module::{
-    TestMetadata, TestModuleOutput, emit_module_yul, emit_module_yul_with_layout,
+    ExpectedRevert, TestMetadata, TestModuleOutput, emit_module_yul, emit_module_yul_with_layout,
     emit_test_module_yul, emit_test_module_yul_with_layout,
 };
 

crates/codegen/src/yul/emitter/module.rs

@@ -36,6 +36,17 @@ pub struct TestMetadata {
     pub yul: String,
     pub value_param_count: usize,
     pub effect_param_count: usize,
+    pub expected_revert: Option<ExpectedRevert>,
+}
+
+/// Describes the expected revert behavior for a test.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum ExpectedRevert {
+    /// Test should revert with any data.
+    Any,
+    // Future phases:
+    // ExactData(Vec<u8>),
+    // Selector([u8; 4]),
 }
 
 /// Output returned by `emit_test_module_yul`.
@@ -333,6 +344,7 @@ pub fn emit_test_module_yul_with_layout(
             yul,
             value_param_count: test.value_param_count,
             effect_param_count: test.effect_param_count,
+            expected_revert: test.expected_revert,
         });
     }
 
@@ -427,6 +439,7 @@ struct TestInfo {
     object_name: String,
     value_param_count: usize,
     effect_param_count: usize,
+    expected_revert: Option<ExpectedRevert>,
 }
 
 /// Dependency set required to emit a single test object.
@@ -448,12 +461,15 @@ fn collect_test_infos(db: &dyn HirDb, functions: &[MirFunction<'_>]) -> Vec<Test
             let MirFunctionOrigin::Hir(hir_func) = mir_func.origin else {
                 return None;
             };
-            if !ItemKind::from(hir_func)
-                .attrs(db)
-                .is_some_and(|attrs| attrs.has_attr(db, "test"))
-            {
-                return None;
-            }
+            let attrs = ItemKind::from(hir_func).attrs(db)?;
+            let test_attr = attrs.get_attr(db, "test")?;
+
+            // Check for #[test(should_revert)]
+            let expected_revert = if test_attr.has_arg(db, "should_revert") {
+                Some(ExpectedRevert::Any)
+            } else {
+                None
+            };
 
             let hir_name = hir_func
                 .name(db)
@@ -473,6 +489,7 @@ fn collect_test_infos(db: &dyn HirDb, functions: &[MirFunction<'_>]) -> Vec<Test
                 object_name: String::new(),
                 value_param_count,
                 effect_param_count,
+                expected_revert,
             })
         })
         .collect()
@@ -1149,6 +1166,7 @@ mod tests {
                 object_name: String::new(),
                 value_param_count: 0,
                 effect_param_count: 0,
+                expected_revert: None,
             },
             TestInfo {
                 hir_name: "foo_bar".to_string(),
@@ -1157,6 +1175,7 @@ mod tests {
                 object_name: String::new(),
                 value_param_count: 0,
                 effect_param_count: 0,
+                expected_revert: None,
             },
         ];
 

crates/codegen/src/yul/mod.rs

@@ -4,7 +4,7 @@ mod errors;
 mod state;
 
 pub use emitter::{
-    EmitModuleError, TestMetadata, TestModuleOutput, emit_module_yul, emit_module_yul_with_layout,
-    emit_test_module_yul, emit_test_module_yul_with_layout,
+    EmitModuleError, ExpectedRevert, TestMetadata, TestModuleOutput, emit_module_yul,
+    emit_module_yul_with_layout, emit_test_module_yul, emit_test_module_yul_with_layout,
 };
 pub use errors::YulError;

crates/fe/src/test.rs

@@ -4,7 +4,7 @@
 //! executes them using revm.
 
 use camino::Utf8PathBuf;
-use codegen::{TestMetadata, emit_test_module_yul};
+use codegen::{ExpectedRevert, TestMetadata, emit_test_module_yul};
 use colored::Colorize;
 use common::InputDb;
 use contract_harness::{ExecutionOptions, RuntimeInstance};
@@ -298,20 +298,32 @@ fn compile_and_run_test(case: &TestMetadata, show_logs: bool) -> TestOutcome {
     };
 
     // Execute the test bytecode in revm
-    let (result, logs) = execute_test(&case.display_name, &bytecode, show_logs);
+    let (result, logs) = execute_test(
+        &case.display_name,
+        &bytecode,
+        show_logs,
+        case.expected_revert.as_ref(),
+    );
     TestOutcome { result, logs }
 }
 
 /// Deploys and executes compiled test bytecode in revm.
 ///
 /// The test passes if the function returns normally, fails if it reverts.
+/// When `expected_revert` is set, the logic is inverted: the test passes if it reverts.
 ///
 /// * `name` - Display name used for reporting.
 /// * `bytecode_hex` - Hex-encoded init bytecode for the test object.
 /// * `show_logs` - Whether to execute with log collection enabled.
+/// * `expected_revert` - If set, the test is expected to revert.
 ///
 /// Returns the test result and any emitted logs.
-fn execute_test(name: &str, bytecode_hex: &str, show_logs: bool) -> (TestResult, Vec<String>) {
+fn execute_test(
+    name: &str,
+    bytecode_hex: &str,
+    show_logs: bool,
+    expected_revert: Option<&ExpectedRevert>,
+) -> (TestResult, Vec<String>) {
     // Deploy the test contract
     let mut instance = match RuntimeInstance::deploy(bytecode_hex) {
         Ok(instance) => instance,
@@ -337,23 +349,45 @@ fn execute_test(name: &str, bytecode_hex: &str, show_logs: bool) -> (TestResult,
         instance.call_raw(&[], options).map(|_| Vec::new())
     };
 
-    match call_result {
-        Ok(logs) => (
+    match (call_result, expected_revert) {
+        // Normal test: execution succeeded
+        (Ok(logs), None) => (
             TestResult {
                 name: name.to_string(),
                 passed: true,
                 error_message: None,
             },
             logs,
         ),
-        Err(err) => (
+        // Normal test: execution reverted (failure)
+        (Err(err), None) => (
             TestResult {
                 name: name.to_string(),
                 passed: false,
                 error_message: Some(format_harness_error(err)),
             },
             Vec::new(),
         ),
+        // Expected revert: execution succeeded (failure - should have reverted)
+        (Ok(_), Some(_)) => (
+            TestResult {
+                name: name.to_string(),
+                passed: false,
+                error_message: Some("Expected test to revert, but it succeeded".to_string()),
+            },
+            Vec::new(),
+        ),
+        // Expected revert: execution reverted (success)
+        (Err(_err), Some(ExpectedRevert::Any)) => (
+            TestResult {
+                name: name.to_string(),
+                passed: true,
+                error_message: None,
+            },
+            Vec::new(),
+        ),
+        // Future: match specific revert data
+        // (Err(HarnessError::Revert(data)), Some(ExpectedRevert::ExactData(expected))) => { ... }
     }
 }
 

crates/fe/tests/fixtures/fe_test/checked_arithmetic_methods.fe

@@ -28,3 +28,53 @@ fn test_wrapping_and_saturating_u8() {
 }
 
 fn as_u8(_ x: u8) -> u8 { x }
+
+// Tests that verify overflow/underflow causes reverts
+
+#[test(should_revert)]
+fn test_add_overflow_u8() {
+    let x: u8 = 255
+    let y: u8 = x + 1
+}
+
+#[test(should_revert)]
+fn test_aug_assign_overflow_u8() {
+    let mut x: u8 = 255
+    x += 1
+}
+
+#[test(should_revert)]
+fn test_div_by_zero_u8() {
+    let x: u8 = 1
+    let y: u8 = x / 0
+}
+
+#[test(should_revert)]
+fn test_rem_by_zero_u8() {
+    let x: u8 = 1
+    let y: u8 = x % 0
+}
+
+#[test(should_revert)]
+fn test_pow_overflow_u8() {
+    let x: u8 = 2
+    let y: u8 = x ** 8
+}
+
+#[test(should_revert)]
+fn test_neg_overflow_i8() {
+    let x: i8 = -128
+    let y: i8 = -x
+}
+
+#[test(should_revert)]
+fn test_div_overflow_i8() {
+    let x: i8 = -128
+    let y: i8 = x / -1
+}
+
+#[test(should_revert)]
+fn test_pow_negative_exp_i8() {
+    let x: i8 = 2
+    let y: i8 = x ** -1
+}

crates/fe/tests/fixtures/fe_test_runner/checked_arithmetic_reverts.fe

@@ -25,6 +25,37 @@ impl<'db> AttrListId<'db> {
             }
         })
     }
+
+    /// Returns the attribute with the given name, if present.
+    pub fn get_attr(self, db: &'db dyn HirDb, name: &str) -> Option<&'db NormalAttr<'db>> {
+        self.data(db).iter().find_map(|attr| {
+            if let Attr::Normal(normal_attr) = attr
+                && let Some(path) = normal_attr.path.to_opt()
+                && let Some(ident) = path.as_ident(db)
+                && ident.data(db) == name
+            {
+                Some(normal_attr)
+            } else {
+                None
+            }
+        })
+    }
+}
+
+impl<'db> NormalAttr<'db> {
+    /// Returns true if this attribute has an argument with the given key (no value).
+    ///
+    /// For example, `#[test(should_revert)]` has the argument `should_revert`.
+    pub fn has_arg(&self, db: &'db dyn HirDb, key: &str) -> bool {
+        self.args.iter().any(|arg| {
+            arg.value.is_none()
+                && arg
+                    .key
+                    .to_opt()
+                    .and_then(|p| p.as_ident(db))
+                    .is_some_and(|ident| ident.data(db) == key)
+        })
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, derive_more::From)]