Skip to content

Commit

Permalink
rm should_fail from runner and TestFunctionKind
Browse files Browse the repository at this point in the history
  • Loading branch information
yash-atreya committed Jan 7, 2025
1 parent 097decd commit 60e0887
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 53 deletions.
34 changes: 10 additions & 24 deletions crates/common/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,9 @@ pub trait TestFunctionExt {
self.test_function_kind().is_any_test()
}

/// Returns `true` if this function is a test that should fail.
fn is_any_test_fail(&self) -> bool {
self.test_function_kind().is_any_test_fail()
}

/// Returns `true` if this function is a unit test.
fn is_unit_test(&self) -> bool {
matches!(self.test_function_kind(), TestFunctionKind::UnitTest { .. })
matches!(self.test_function_kind(), TestFunctionKind::UnitTest)
}

/// Returns `true` if this function is a `beforeTestSetup` function.
Expand Down Expand Up @@ -111,9 +106,9 @@ pub enum TestFunctionKind {
/// `setUp`.
Setup,
/// `test*`. `should_fail` is `true` for `testFail*`.
UnitTest { should_fail: bool },
UnitTest,
/// `test*`, with arguments. `should_fail` is `true` for `testFail*`.
FuzzTest { should_fail: bool },
FuzzTest,
/// `invariant*` or `statefulFuzz*`.
InvariantTest,
/// `afterInvariant`.
Expand All @@ -130,11 +125,10 @@ impl TestFunctionKind {
pub fn classify(name: &str, has_inputs: bool) -> Self {
match () {
_ if name.starts_with("test") => {
let should_fail = name.starts_with("testFail");
if has_inputs {
Self::FuzzTest { should_fail }
Self::FuzzTest
} else {
Self::UnitTest { should_fail }
Self::UnitTest
}
}
_ if name.starts_with("invariant") || name.starts_with("statefulFuzz") => {
Expand All @@ -151,10 +145,8 @@ impl TestFunctionKind {
pub const fn name(&self) -> &'static str {
match self {
Self::Setup => "setUp",
Self::UnitTest { should_fail: false } => "test",
Self::UnitTest { should_fail: true } => "testFail",
Self::FuzzTest { should_fail: false } => "fuzz",
Self::FuzzTest { should_fail: true } => "fuzz fail",
Self::UnitTest => "test",
Self::FuzzTest => "fuzz",
Self::InvariantTest => "invariant",
Self::AfterInvariant => "afterInvariant",
Self::Fixture => "fixture",
Expand All @@ -171,25 +163,19 @@ impl TestFunctionKind {
/// Returns `true` if this function is a unit, fuzz, or invariant test.
#[inline]
pub const fn is_any_test(&self) -> bool {
matches!(self, Self::UnitTest { .. } | Self::FuzzTest { .. } | Self::InvariantTest)
}

/// Returns `true` if this function is a test that should fail.
#[inline]
pub const fn is_any_test_fail(&self) -> bool {
matches!(self, Self::UnitTest { should_fail: true } | Self::FuzzTest { should_fail: true })
matches!(self, Self::UnitTest | Self::FuzzTest | Self::InvariantTest)
}

/// Returns `true` if this function is a unit test.
#[inline]
pub fn is_unit_test(&self) -> bool {
matches!(self, Self::UnitTest { .. })
matches!(self, Self::UnitTest)
}

/// Returns `true` if this function is a fuzz test.
#[inline]
pub const fn is_fuzz_test(&self) -> bool {
matches!(self, Self::FuzzTest { .. })
matches!(self, Self::FuzzTest)
}

/// Returns `true` if this function is an invariant test.
Expand Down
6 changes: 2 additions & 4 deletions crates/evm/evm/src/executors/fuzz/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ impl FuzzedExecutor {
fuzz_fixtures: &FuzzFixtures,
deployed_libs: &[Address],
address: Address,
should_fail: bool,
rd: &RevertDecoder,
progress: Option<&ProgressBar>,
) -> FuzzTestResult {
Expand All @@ -109,7 +108,7 @@ impl FuzzedExecutor {
return Err(TestCaseError::fail(TEST_TIMEOUT));
}

let fuzz_res = self.single_fuzz(address, should_fail, calldata)?;
let fuzz_res = self.single_fuzz(address, calldata)?;

// If running with progress then increment current run.
if let Some(progress) = progress {
Expand Down Expand Up @@ -238,7 +237,6 @@ impl FuzzedExecutor {
pub fn single_fuzz(
&self,
address: Address,
should_fail: bool,
calldata: alloy_primitives::Bytes,
) -> Result<FuzzOutcome, TestCaseError> {
let mut call = self
Expand All @@ -256,7 +254,7 @@ impl FuzzedExecutor {
(cheats.breakpoints.clone(), cheats.deprecated.clone())
});

let success = self.executor.is_raw_call_mut_success(address, &mut call, should_fail);
let success = self.executor.is_raw_call_mut_success(address, &mut call, false);
if success {
Ok(FuzzOutcome::Case(CaseOutcome {
case: FuzzCase { calldata, gas: call.gas_used, stipend: call.stipend },
Expand Down
30 changes: 7 additions & 23 deletions crates/forge/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,16 +369,12 @@ impl<'a> ContractRunner<'a> {

let test_fail_instances = functions
.iter()
.filter_map(|func| {
TestFunctionKind::classify(&func.name, !func.inputs.is_empty())
.is_any_test_fail()
.then_some(func.name.clone())
})
.filter_map(|func| (func.name.starts_with("testFail")).then_some(func.name.clone()))
.collect::<Vec<_>>()
.join(", ");

if !test_fail_instances.is_empty() {
return SuiteResult::new(start.elapsed(),[("`testFail*` has been deprecated".to_string(), TestResult::fail(format!("Found {test_fail_instances}. Consider changing to test_Revert[If|When]_Condition and expecting a revert.")))].into(), warnings)
return SuiteResult::new(start.elapsed(),[(format!("Found: {test_fail_instances}"), TestResult::fail(format!("`testFail*` has been deprecated. Consider changing to test_Revert[If|When]_Condition and expecting a revert")))].into(), warnings)
}

let test_results = functions
Expand Down Expand Up @@ -487,20 +483,9 @@ impl<'a> FunctionRunner<'a> {
return self.result;
}

let test_fail_deprecation = |should_fail: bool| {
if should_fail {
return Some(TestResult::fail(format!("`testFail*` has been deprecated. Consider changing {} to something along the lines of `test_Revert[If|When]_Condition` and expecting a revert.", func.name)));
}
None
};

match kind {
TestFunctionKind::UnitTest { should_fail } => {
test_fail_deprecation(should_fail).unwrap_or(self.run_unit_test(func, should_fail))
}
TestFunctionKind::FuzzTest { should_fail } => {
test_fail_deprecation(should_fail).unwrap_or(self.run_fuzz_test(func, should_fail))
}
TestFunctionKind::UnitTest => self.run_unit_test(func),
TestFunctionKind::FuzzTest => self.run_fuzz_test(func),
TestFunctionKind::InvariantTest => {
self.run_invariant_test(func, call_after_invariant, identified_contracts.unwrap())
}
Expand All @@ -516,7 +501,7 @@ impl<'a> FunctionRunner<'a> {
/// (therefore the unit test call will be made on modified state).
/// State modifications of before test txes and unit test function call are discarded after
/// test ends, similar to `eth_call`.
fn run_unit_test(mut self, func: &Function, should_fail: bool) -> TestResult {
fn run_unit_test(mut self, func: &Function) -> TestResult {
// Prepare unit test execution.
if self.prepare_test(func).is_err() {
return self.result;
Expand Down Expand Up @@ -544,7 +529,7 @@ impl<'a> FunctionRunner<'a> {
};

let success =
self.executor.is_raw_call_mut_success(self.address, &mut raw_call_result, should_fail);
self.executor.is_raw_call_mut_success(self.address, &mut raw_call_result, false);
self.result.single_result(success, reason, raw_call_result);
self.result
}
Expand Down Expand Up @@ -743,7 +728,7 @@ impl<'a> FunctionRunner<'a> {
/// (therefore the fuzz test will use the modified state).
/// State modifications of before test txes and fuzz test are discarded after test ends,
/// similar to `eth_call`.
fn run_fuzz_test(mut self, func: &Function, should_fail: bool) -> TestResult {
fn run_fuzz_test(mut self, func: &Function) -> TestResult {
// Prepare fuzz test execution.
if self.prepare_test(func).is_err() {
return self.result;
Expand All @@ -763,7 +748,6 @@ impl<'a> FunctionRunner<'a> {
&self.setup.fuzz_fixtures,
&self.setup.deployed_libs,
self.address,
should_fail,
&self.cr.mcr.revert_decoder,
progress.as_ref(),
);
Expand Down
4 changes: 2 additions & 2 deletions crates/forge/tests/cli/failure_assertions.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Tests in which we want to assert failures.

forgetest!(test_fail_deprecation_warning, |prj, cmd| {
forgetest!(test_fail_deprecation, |prj, cmd| {
prj.insert_ds_test();

prj.add_source(
Expand All @@ -24,7 +24,7 @@ forgetest!(test_fail_deprecation_warning, |prj, cmd| {
r#"[COMPILING_FILES] with [SOLC_VERSION]
[SOLC_VERSION] [ELAPSED]
...
[FAIL: Found testFail_deprecated, testFail_deprecated2. Consider changing to test_Revert[If|When]_Condition and expecting a revert.] `testFail*` has been deprecated ([GAS])
[FAIL: `testFail*` has been deprecated. Consider changing to test_Revert[If|When]_Condition and expecting a revert] Found: testFail_deprecated, testFail_deprecated2 ([GAS])
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; [ELAPSED]
...
"#,
Expand Down

0 comments on commit 60e0887

Please sign in to comment.