From 824de94ed0b812374eafc8d21a5c283dd39e02e5 Mon Sep 17 00:00:00 2001 From: Abeeujah Date: Mon, 22 Sep 2025 11:49:36 +0100 Subject: [PATCH 1/8] dev: Don't fetch block numbers nor check rpc version for ignored tests --- crates/forge/src/run_tests/resolve_config.rs | 12 ++++++------ crates/forge/src/test_filter.rs | 5 ++++- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/forge/src/run_tests/resolve_config.rs b/crates/forge/src/run_tests/resolve_config.rs index 97dfe5e25b..7d2af8f217 100644 --- a/crates/forge/src/run_tests/resolve_config.rs +++ b/crates/forge/src/run_tests/resolve_config.rs @@ -31,12 +31,12 @@ pub async fn resolve_config( ignored: case.config.ignored || (env_ignore_fork_tests && case.config.fork_config.is_some()), expected_result: case.config.expected_result, - fork_config: resolve_fork_config( - case.config.fork_config, - block_number_map, - fork_targets, - ) - .await?, + fork_config: if case.config.ignored { + None + } else { + resolve_fork_config(case.config.fork_config, block_number_map, fork_targets) + .await? + }, fuzzer_config: case.config.fuzzer_config, disable_predeployed_contracts: case.config.disable_predeployed_contracts, }, diff --git a/crates/forge/src/test_filter.rs b/crates/forge/src/test_filter.rs index 361b843b1a..26fdc2dac5 100644 --- a/crates/forge/src/test_filter.rs +++ b/crates/forge/src/test_filter.rs @@ -123,7 +123,10 @@ impl TestCaseFilter for TestsFilter { match self.ignored_filter { IgnoredFilter::All => true, - IgnoredFilter::Ignored => ignored, + IgnoredFilter::Ignored => { + assert!(ignored); + true + } IgnoredFilter::NotIgnored => !ignored, } } From bb4e267c6b3e893f787392272ca37d06cee8f4c6 Mon Sep 17 00:00:00 2001 From: Abeeujah Date: Wed, 24 Sep 2025 14:00:56 +0100 Subject: [PATCH 2/8] dev: Revert to inexpensive safety check --- crates/forge/src/test_filter.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/forge/src/test_filter.rs b/crates/forge/src/test_filter.rs index 26fdc2dac5..361b843b1a 100644 --- a/crates/forge/src/test_filter.rs +++ b/crates/forge/src/test_filter.rs @@ -123,10 +123,7 @@ impl TestCaseFilter for TestsFilter { match self.ignored_filter { IgnoredFilter::All => true, - IgnoredFilter::Ignored => { - assert!(ignored); - true - } + IgnoredFilter::Ignored => ignored, IgnoredFilter::NotIgnored => !ignored, } } From 8546e3e641f2b74c6fea82ad0f4f6776d2e525a7 Mon Sep 17 00:00:00 2001 From: Abeeujah Date: Wed, 24 Sep 2025 15:54:14 +0100 Subject: [PATCH 3/8] dev: check tests filter before resolving fork config --- crates/forge/src/run_tests/package.rs | 5 ++++- crates/forge/src/run_tests/resolve_config.rs | 16 ++++++++++++---- crates/forge/src/test_filter.rs | 2 +- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/crates/forge/src/run_tests/package.rs b/crates/forge/src/run_tests/package.rs index c41248491f..30504146a9 100644 --- a/crates/forge/src/run_tests/package.rs +++ b/crates/forge/src/run_tests/package.rs @@ -137,6 +137,7 @@ async fn test_package_with_config_resolved( fork_targets: &[ForkTarget], block_number_map: &mut BlockNumberMap, forge_config: &ForgeConfig, + tests_filter: TestsFilter, ) -> Result> { let mut test_targets_with_resolved_config = Vec::with_capacity(test_targets.len()); @@ -146,7 +147,8 @@ async fn test_package_with_config_resolved( &forge_config.test_runner_config.tracked_resource, )?; - let test_target = resolve_config(test_target, fork_targets, block_number_map).await?; + let test_target = + resolve_config(test_target, fork_targets, block_number_map, tests_filter).await?; test_targets_with_resolved_config.push(test_target); } @@ -175,6 +177,7 @@ pub async fn run_for_package( &fork_targets, block_number_map, &forge_config, + tests_filter, ) .await?; let all_tests = sum_test_cases(&test_targets); diff --git a/crates/forge/src/run_tests/resolve_config.rs b/crates/forge/src/run_tests/resolve_config.rs index 7756bf079c..6c5ec81cc4 100644 --- a/crates/forge/src/run_tests/resolve_config.rs +++ b/crates/forge/src/run_tests/resolve_config.rs @@ -1,5 +1,9 @@ use super::maat::env_ignore_fork_tests; -use crate::{block_number_map::BlockNumberMap, scarb::config::ForkTarget}; +use crate::{ + block_number_map::BlockNumberMap, + scarb::config::ForkTarget, + test_filter::{IgnoredFilter, TestsFilter}, +}; use anyhow::{Result, anyhow}; use cheatnet::runtime_extensions::forge_config_extension::config::{ BlockId, InlineForkConfig, OverriddenForkConfig, RawForkConfig, @@ -19,20 +23,24 @@ pub async fn resolve_config( test_target: TestTargetWithConfig, fork_targets: &[ForkTarget], block_number_map: &mut BlockNumberMap, + tests_filter: TestsFilter, ) -> Result { let mut test_cases = Vec::with_capacity(test_target.test_cases.len()); let env_ignore_fork_tests = env_ignore_fork_tests(); for case in test_target.test_cases { + let ignored = + case.config.ignored || (env_ignore_fork_tests && case.config.fork_config.is_some()); test_cases.push(TestCaseWithResolvedConfig { name: case.name, test_details: case.test_details, config: TestCaseResolvedConfig { available_gas: case.config.available_gas, - ignored: case.config.ignored - || (env_ignore_fork_tests && case.config.fork_config.is_some()), + ignored, expected_result: case.config.expected_result, - fork_config: if case.config.ignored { + fork_config: if ignored + && matches!(tests_filter.ignored_filter, IgnoredFilter::NotIgnored) + { None } else { resolve_fork_config(case.config.fork_config, block_number_map, fork_targets) diff --git a/crates/forge/src/test_filter.rs b/crates/forge/src/test_filter.rs index 361b843b1a..577f4fe565 100644 --- a/crates/forge/src/test_filter.rs +++ b/crates/forge/src/test_filter.rs @@ -9,7 +9,7 @@ pub struct TestsFilter { // based on name pub(crate) name_filter: NameFilter, // based on `#[ignore]` attribute - ignored_filter: IgnoredFilter, + pub(crate) ignored_filter: IgnoredFilter, // based on `--rerun_failed` flag last_failed_filter: bool, // based on `--skip` flag From dee6faf12f341bbefce52bc80bb438d7a8e7e7e8 Mon Sep 17 00:00:00 2001 From: Abeeujah Date: Wed, 24 Sep 2025 17:02:46 +0100 Subject: [PATCH 4/8] dev: Fix Visibility and Ownership --- crates/forge/src/run_tests/package.rs | 15 ++++++++++----- crates/forge/src/run_tests/resolve_config.rs | 10 +++------- crates/forge/src/test_filter.rs | 4 ++-- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/crates/forge/src/run_tests/package.rs b/crates/forge/src/run_tests/package.rs index 30504146a9..c8e0712087 100644 --- a/crates/forge/src/run_tests/package.rs +++ b/crates/forge/src/run_tests/package.rs @@ -12,7 +12,7 @@ use crate::{ load_test_artifacts, should_compile_starknet_contract_target, }, shared_cache::FailedTestsCache, - test_filter::{NameFilter, TestsFilter}, + test_filter::{IgnoredFilter, NameFilter, TestsFilter}, warn::{ warn_if_available_gas_used_with_incompatible_scarb_version, warn_if_incompatible_rpc_version, @@ -137,7 +137,7 @@ async fn test_package_with_config_resolved( fork_targets: &[ForkTarget], block_number_map: &mut BlockNumberMap, forge_config: &ForgeConfig, - tests_filter: TestsFilter, + ignored_filter: IgnoredFilter, ) -> Result> { let mut test_targets_with_resolved_config = Vec::with_capacity(test_targets.len()); @@ -147,8 +147,13 @@ async fn test_package_with_config_resolved( &forge_config.test_runner_config.tracked_resource, )?; - let test_target = - resolve_config(test_target, fork_targets, block_number_map, tests_filter).await?; + let test_target = resolve_config( + test_target, + fork_targets, + block_number_map, + ignored_filter.clone(), + ) + .await?; test_targets_with_resolved_config.push(test_target); } @@ -177,7 +182,7 @@ pub async fn run_for_package( &fork_targets, block_number_map, &forge_config, - tests_filter, + tests_filter.ignored_filter.clone(), ) .await?; let all_tests = sum_test_cases(&test_targets); diff --git a/crates/forge/src/run_tests/resolve_config.rs b/crates/forge/src/run_tests/resolve_config.rs index 6c5ec81cc4..0093960abf 100644 --- a/crates/forge/src/run_tests/resolve_config.rs +++ b/crates/forge/src/run_tests/resolve_config.rs @@ -1,8 +1,6 @@ use super::maat::env_ignore_fork_tests; use crate::{ - block_number_map::BlockNumberMap, - scarb::config::ForkTarget, - test_filter::{IgnoredFilter, TestsFilter}, + block_number_map::BlockNumberMap, scarb::config::ForkTarget, test_filter::IgnoredFilter, }; use anyhow::{Result, anyhow}; use cheatnet::runtime_extensions::forge_config_extension::config::{ @@ -23,7 +21,7 @@ pub async fn resolve_config( test_target: TestTargetWithConfig, fork_targets: &[ForkTarget], block_number_map: &mut BlockNumberMap, - tests_filter: TestsFilter, + ignored_filter: IgnoredFilter, ) -> Result { let mut test_cases = Vec::with_capacity(test_target.test_cases.len()); let env_ignore_fork_tests = env_ignore_fork_tests(); @@ -38,9 +36,7 @@ pub async fn resolve_config( available_gas: case.config.available_gas, ignored, expected_result: case.config.expected_result, - fork_config: if ignored - && matches!(tests_filter.ignored_filter, IgnoredFilter::NotIgnored) - { + fork_config: if ignored && matches!(ignored_filter, IgnoredFilter::NotIgnored) { None } else { resolve_fork_config(case.config.fork_config, block_number_map, fork_targets) diff --git a/crates/forge/src/test_filter.rs b/crates/forge/src/test_filter.rs index 577f4fe565..de9a2b6875 100644 --- a/crates/forge/src/test_filter.rs +++ b/crates/forge/src/test_filter.rs @@ -25,8 +25,8 @@ pub(crate) enum NameFilter { ExactMatch(String), } -#[derive(Debug, PartialEq)] -pub(crate) enum IgnoredFilter { +#[derive(Debug, PartialEq, Clone)] +pub enum IgnoredFilter { NotIgnored, Ignored, All, From c0eeffb277b984846a66ca5824778839f7cad757 Mon Sep 17 00:00:00 2001 From: Abeeujah Date: Fri, 26 Sep 2025 17:30:22 +0100 Subject: [PATCH 5/8] dev: Introduce TestCaseIsIgnored trait --- crates/forge-runner/src/lib.rs | 9 ++++- .../src/package_tests/with_config.rs | 11 +++++- .../src/package_tests/with_config_resolved.rs | 8 +++- crates/forge/src/run_tests/package.rs | 15 +++----- crates/forge/src/run_tests/resolve_config.rs | 38 +++++++++++++------ crates/forge/src/test_filter.rs | 14 ++++--- 6 files changed, 65 insertions(+), 30 deletions(-) diff --git a/crates/forge-runner/src/lib.rs b/crates/forge-runner/src/lib.rs index 384c65d302..f6afcb926a 100644 --- a/crates/forge-runner/src/lib.rs +++ b/crates/forge-runner/src/lib.rs @@ -1,5 +1,6 @@ use crate::coverage_api::run_coverage; use crate::forge_config::{ExecutionDataToSave, ForgeConfig}; +use crate::package_tests::TestCase; use crate::running::{run_fuzz_test, run_test}; use crate::test_case_summary::TestCaseSummary; use anyhow::Result; @@ -56,7 +57,13 @@ const BUILTINS: [&str; 11] = [ ]; pub trait TestCaseFilter { - fn should_be_run(&self, test_case: &TestCaseWithResolvedConfig) -> bool; + fn should_be_run(&self, test_case: &TestCase) -> bool + where + T: TestCaseIsIgnored; +} + +pub trait TestCaseIsIgnored { + fn is_ignored(&self) -> bool; } pub fn maybe_save_trace_and_profile( diff --git a/crates/forge-runner/src/package_tests/with_config.rs b/crates/forge-runner/src/package_tests/with_config.rs index 1589a671d0..73481e43ea 100644 --- a/crates/forge-runner/src/package_tests/with_config.rs +++ b/crates/forge-runner/src/package_tests/with_config.rs @@ -1,5 +1,8 @@ use super::{TestCase, TestTarget}; -use crate::expected_result::{ExpectedPanicValue, ExpectedTestResult}; +use crate::{ + TestCaseIsIgnored, + expected_result::{ExpectedPanicValue, ExpectedTestResult}, +}; use cheatnet::runtime_extensions::forge_config_extension::config::{ Expected, RawAvailableResourceBoundsConfig, RawForgeConfig, RawForkConfig, RawFuzzerConfig, RawShouldPanicConfig, @@ -22,6 +25,12 @@ pub struct TestCaseConfig { pub disable_predeployed_contracts: bool, } +impl TestCaseIsIgnored for TestCaseConfig { + fn is_ignored(&self) -> bool { + self.ignored + } +} + impl From for TestCaseConfig { fn from(value: RawForgeConfig) -> Self { Self { diff --git a/crates/forge-runner/src/package_tests/with_config_resolved.rs b/crates/forge-runner/src/package_tests/with_config_resolved.rs index 391d0eb439..7eee9b7d57 100644 --- a/crates/forge-runner/src/package_tests/with_config_resolved.rs +++ b/crates/forge-runner/src/package_tests/with_config_resolved.rs @@ -1,5 +1,5 @@ use super::{TestCase, TestTarget}; -use crate::expected_result::ExpectedTestResult; +use crate::{TestCaseIsIgnored, expected_result::ExpectedTestResult}; use anyhow::Result; use cairo_vm::types::program::Program; use cheatnet::runtime_extensions::forge_config_extension::config::{ @@ -40,3 +40,9 @@ pub struct TestCaseResolvedConfig { pub fuzzer_config: Option, pub disable_predeployed_contracts: bool, } + +impl TestCaseIsIgnored for TestCaseResolvedConfig { + fn is_ignored(&self) -> bool { + self.ignored + } +} diff --git a/crates/forge/src/run_tests/package.rs b/crates/forge/src/run_tests/package.rs index c8e0712087..28b01c369b 100644 --- a/crates/forge/src/run_tests/package.rs +++ b/crates/forge/src/run_tests/package.rs @@ -12,7 +12,7 @@ use crate::{ load_test_artifacts, should_compile_starknet_contract_target, }, shared_cache::FailedTestsCache, - test_filter::{IgnoredFilter, NameFilter, TestsFilter}, + test_filter::{NameFilter, TestsFilter}, warn::{ warn_if_available_gas_used_with_incompatible_scarb_version, warn_if_incompatible_rpc_version, @@ -137,7 +137,7 @@ async fn test_package_with_config_resolved( fork_targets: &[ForkTarget], block_number_map: &mut BlockNumberMap, forge_config: &ForgeConfig, - ignored_filter: IgnoredFilter, + tests_filter: &TestsFilter, ) -> Result> { let mut test_targets_with_resolved_config = Vec::with_capacity(test_targets.len()); @@ -147,13 +147,8 @@ async fn test_package_with_config_resolved( &forge_config.test_runner_config.tracked_resource, )?; - let test_target = resolve_config( - test_target, - fork_targets, - block_number_map, - ignored_filter.clone(), - ) - .await?; + let test_target = + resolve_config(test_target, fork_targets, block_number_map, tests_filter).await?; test_targets_with_resolved_config.push(test_target); } @@ -182,7 +177,7 @@ pub async fn run_for_package( &fork_targets, block_number_map, &forge_config, - tests_filter.ignored_filter.clone(), + &tests_filter, ) .await?; let all_tests = sum_test_cases(&test_targets); diff --git a/crates/forge/src/run_tests/resolve_config.rs b/crates/forge/src/run_tests/resolve_config.rs index 0093960abf..e9dcb61628 100644 --- a/crates/forge/src/run_tests/resolve_config.rs +++ b/crates/forge/src/run_tests/resolve_config.rs @@ -1,17 +1,20 @@ use super::maat::env_ignore_fork_tests; use crate::{ - block_number_map::BlockNumberMap, scarb::config::ForkTarget, test_filter::IgnoredFilter, + block_number_map::BlockNumberMap, scarb::config::ForkTarget, test_filter::TestsFilter, }; use anyhow::{Result, anyhow}; use cheatnet::runtime_extensions::forge_config_extension::config::{ BlockId, InlineForkConfig, OverriddenForkConfig, RawForkConfig, }; use conversions::byte_array::ByteArray; -use forge_runner::package_tests::{ - with_config::TestTargetWithConfig, - with_config_resolved::{ - ResolvedForkConfig, TestCaseResolvedConfig, TestCaseWithResolvedConfig, - TestTargetWithResolvedConfig, +use forge_runner::{ + TestCaseFilter, + package_tests::{ + with_config::TestTargetWithConfig, + with_config_resolved::{ + ResolvedForkConfig, TestCaseResolvedConfig, TestCaseWithResolvedConfig, + TestTargetWithResolvedConfig, + }, }, }; use starknet_api::block::BlockNumber; @@ -21,7 +24,7 @@ pub async fn resolve_config( test_target: TestTargetWithConfig, fork_targets: &[ForkTarget], block_number_map: &mut BlockNumberMap, - ignored_filter: IgnoredFilter, + tests_filter: &TestsFilter, ) -> Result { let mut test_cases = Vec::with_capacity(test_target.test_cases.len()); let env_ignore_fork_tests = env_ignore_fork_tests(); @@ -30,21 +33,21 @@ pub async fn resolve_config( let ignored = case.config.ignored || (env_ignore_fork_tests && case.config.fork_config.is_some()); test_cases.push(TestCaseWithResolvedConfig { - name: case.name, - test_details: case.test_details, config: TestCaseResolvedConfig { available_gas: case.config.available_gas, ignored, - expected_result: case.config.expected_result, - fork_config: if ignored && matches!(ignored_filter, IgnoredFilter::NotIgnored) { + fork_config: if ignored && tests_filter.should_be_run(&case) { None } else { resolve_fork_config(case.config.fork_config, block_number_map, fork_targets) .await? }, + expected_result: case.config.expected_result, fuzzer_config: case.config.fuzzer_config, disable_predeployed_contracts: case.config.disable_predeployed_contracts, }, + name: case.name, + test_details: case.test_details, }); } @@ -130,6 +133,8 @@ fn replace_id_with_params( #[cfg(test)] mod tests { + use crate::shared_cache::FailedTestsCache; + use super::*; use cairo_lang_sierra::program::ProgramArtifact; use cairo_lang_sierra::{ids::GenericTypeId, program::Program}; @@ -198,7 +203,16 @@ mod tests { url: Url::parse("https://not_taken.com").expect("Should be valid url"), block_id: BlockId::BlockNumber(120), }], - &mut BlockNumberMap::default() + &mut BlockNumberMap::default(), + &TestsFilter::from_flags( + None, + false, + Vec::new(), + true, + true, + false, + FailedTestsCache::default(), + ) ) .await .is_err() diff --git a/crates/forge/src/test_filter.rs b/crates/forge/src/test_filter.rs index de9a2b6875..06a3efdb29 100644 --- a/crates/forge/src/test_filter.rs +++ b/crates/forge/src/test_filter.rs @@ -1,7 +1,8 @@ use crate::shared_cache::FailedTestsCache; use anyhow::Result; -use forge_runner::TestCaseFilter; +use forge_runner::package_tests::TestCase; use forge_runner::package_tests::with_config_resolved::TestCaseWithResolvedConfig; +use forge_runner::{TestCaseFilter, TestCaseIsIgnored}; #[derive(Debug, PartialEq)] // Specifies what tests should be included @@ -9,7 +10,7 @@ pub struct TestsFilter { // based on name pub(crate) name_filter: NameFilter, // based on `#[ignore]` attribute - pub(crate) ignored_filter: IgnoredFilter, + ignored_filter: IgnoredFilter, // based on `--rerun_failed` flag last_failed_filter: bool, // based on `--skip` flag @@ -25,7 +26,7 @@ pub(crate) enum NameFilter { ExactMatch(String), } -#[derive(Debug, PartialEq, Clone)] +#[derive(Debug, PartialEq)] pub enum IgnoredFilter { NotIgnored, Ignored, @@ -118,8 +119,11 @@ impl TestsFilter { } impl TestCaseFilter for TestsFilter { - fn should_be_run(&self, test_case: &TestCaseWithResolvedConfig) -> bool { - let ignored = test_case.config.ignored; + fn should_be_run(&self, test_case: &TestCase) -> bool + where + T: TestCaseIsIgnored, + { + let ignored = test_case.config.is_ignored(); match self.ignored_filter { IgnoredFilter::All => true, From 5b466f87d6beb58e091ac1382a2737fd3a0c03a7 Mon Sep 17 00:00:00 2001 From: Abeeujah Date: Fri, 26 Sep 2025 17:45:36 +0100 Subject: [PATCH 6/8] dev: Implement Default Trait for TestsFilter --- crates/forge/src/run_tests/resolve_config.rs | 12 +----------- crates/forge/src/test_filter.rs | 12 ++++++++++++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/crates/forge/src/run_tests/resolve_config.rs b/crates/forge/src/run_tests/resolve_config.rs index e9dcb61628..db1df38fe7 100644 --- a/crates/forge/src/run_tests/resolve_config.rs +++ b/crates/forge/src/run_tests/resolve_config.rs @@ -133,8 +133,6 @@ fn replace_id_with_params( #[cfg(test)] mod tests { - use crate::shared_cache::FailedTestsCache; - use super::*; use cairo_lang_sierra::program::ProgramArtifact; use cairo_lang_sierra::{ids::GenericTypeId, program::Program}; @@ -204,15 +202,7 @@ mod tests { block_id: BlockId::BlockNumber(120), }], &mut BlockNumberMap::default(), - &TestsFilter::from_flags( - None, - false, - Vec::new(), - true, - true, - false, - FailedTestsCache::default(), - ) + &TestsFilter::default(), ) .await .is_err() diff --git a/crates/forge/src/test_filter.rs b/crates/forge/src/test_filter.rs index 06a3efdb29..3b6255d4c7 100644 --- a/crates/forge/src/test_filter.rs +++ b/crates/forge/src/test_filter.rs @@ -118,6 +118,18 @@ impl TestsFilter { } } +impl Default for TestsFilter { + fn default() -> Self { + Self { + name_filter: NameFilter::All, + ignored_filter: IgnoredFilter::All, + last_failed_filter: false, + skip_filter: Vec::new(), + failed_tests_cache: FailedTestsCache::default(), + } + } +} + impl TestCaseFilter for TestsFilter { fn should_be_run(&self, test_case: &TestCase) -> bool where From a6cba94ab822d58f9f9a05672990296a48c8964f Mon Sep 17 00:00:00 2001 From: Abeeujah Date: Tue, 30 Sep 2025 11:53:53 +0100 Subject: [PATCH 7/8] dev: Test Fork Config Resolution --- crates/forge/src/run_tests/resolve_config.rs | 518 +++++++++++++++++-- crates/forge/src/test_filter.rs | 1 + 2 files changed, 484 insertions(+), 35 deletions(-) diff --git a/crates/forge/src/run_tests/resolve_config.rs b/crates/forge/src/run_tests/resolve_config.rs index db1df38fe7..49ec690191 100644 --- a/crates/forge/src/run_tests/resolve_config.rs +++ b/crates/forge/src/run_tests/resolve_config.rs @@ -36,11 +36,11 @@ pub async fn resolve_config( config: TestCaseResolvedConfig { available_gas: case.config.available_gas, ignored, - fork_config: if ignored && tests_filter.should_be_run(&case) { - None - } else { + fork_config: if tests_filter.should_be_run(&case) { resolve_fork_config(case.config.fork_config, block_number_map, fork_targets) .await? + } else { + None }, expected_result: case.config.expected_result, fuzzer_config: case.config.fuzzer_config, @@ -134,6 +134,7 @@ fn replace_id_with_params( #[cfg(test)] mod tests { use super::*; + use crate::shared_cache::FailedTestsCache; use cairo_lang_sierra::program::ProgramArtifact; use cairo_lang_sierra::{ids::GenericTypeId, program::Program}; use forge_runner::package_tests::TestTargetLocation; @@ -155,9 +156,38 @@ mod tests { } } - #[tokio::test] - async fn to_runnable_non_existent_id() { - let mocked_tests = TestTargetWithConfig { + fn create_test_case_with_config( + name: &str, + ignored: bool, + fork_config: Option, + ) -> TestCaseWithConfig { + TestCaseWithConfig { + name: name.to_string(), + config: TestCaseConfig { + available_gas: None, + ignored, + expected_result: ExpectedTestResult::Success, + fork_config, + fuzzer_config: None, + disable_predeployed_contracts: false, + }, + test_details: TestDetails { + sierra_entry_point_statement_idx: 100, + parameter_types: vec![ + (GenericTypeId("RangeCheck".into()), 1), + (GenericTypeId("GasBuiltin".into()), 1), + ], + return_types: vec![ + (GenericTypeId("RangeCheck".into()), 1), + (GenericTypeId("GasBuiltin".into()), 1), + (GenericTypeId("Enum".into()), 3), + ], + }, + } + } + + fn create_test_target_with_cases(test_cases: Vec) -> TestTargetWithConfig { + TestTargetWithConfig { sierra_program: program_for_testing(), sierra_program_path: Arc::default(), casm_program: Arc::new( @@ -167,40 +197,35 @@ mod tests { ) .unwrap(), ), - test_cases: vec![TestCaseWithConfig { - name: "crate1::do_thing".to_string(), - config: TestCaseConfig { - available_gas: None, - ignored: false, - expected_result: ExpectedTestResult::Success, - fork_config: Some(RawForkConfig::Named("non_existent".into())), - fuzzer_config: None, - disable_predeployed_contracts: false, - }, - test_details: TestDetails { - sierra_entry_point_statement_idx: 100, - parameter_types: vec![ - (GenericTypeId("RangeCheck".into()), 1), - (GenericTypeId("GasBuiltin".into()), 1), - ], - return_types: vec![ - (GenericTypeId("RangeCheck".into()), 1), - (GenericTypeId("GasBuiltin".into()), 1), - (GenericTypeId("Enum".into()), 3), - ], - }, - }], + test_cases, tests_location: TestTargetLocation::Lib, - }; + } + } + + fn create_fork_target(name: &str, url: &str, block_id: BlockId) -> ForkTarget { + ForkTarget { + name: name.to_string(), + url: Url::parse(url).expect("Should be valid url"), + block_id, + } + } + + #[tokio::test] + async fn to_runnable_non_existent_id() { + let mocked_tests = create_test_target_with_cases(vec![create_test_case_with_config( + "crate1::do_thing", + false, + Some(RawForkConfig::Named("non_existent".into())), + )]); assert!( resolve_config( mocked_tests, - &[ForkTarget { - name: "definitely_non_existing".to_string(), - url: Url::parse("https://not_taken.com").expect("Should be valid url"), - block_id: BlockId::BlockNumber(120), - }], + &[create_fork_target( + "definitely_non_existing", + "https://not_taken.com", + BlockId::BlockNumber(120) + )], &mut BlockNumberMap::default(), &TestsFilter::default(), ) @@ -208,4 +233,427 @@ mod tests { .is_err() ); } + + #[tokio::test] + async fn test_ignored_filter_skips_fork_config_resolution() { + let ignored_test = create_test_case_with_config( + "ignored_test", + true, + Some(RawForkConfig::Named("non_existent_fork".into())), + ); + + let test_target = create_test_target_with_cases(vec![ignored_test]); + + // Create a filter that excludes ignored tests + let tests_filter = TestsFilter::from_flags( + None, + false, + Vec::new(), + false, + false, + false, + FailedTestsCache::default(), + ); + + let resolved = resolve_config( + test_target, + &[], + &mut BlockNumberMap::default(), + &tests_filter, + ) + .await + .unwrap(); + + assert_eq!(resolved.test_cases.len(), 1); + assert!(resolved.test_cases[0].config.ignored); + assert!(resolved.test_cases[0].config.fork_config.is_none()); + } + + #[tokio::test] + async fn test_non_ignored_filter_resolves_fork_config() { + let test_case = create_test_case_with_config( + "valid_test", + false, + Some(RawForkConfig::Named("valid_fork".into())), + ); + + let test_target = create_test_target_with_cases(vec![test_case]); + + let fork_targets = vec![create_fork_target( + "valid_fork", + "https://example.com", + BlockId::BlockNumber(100), + )]; + + let tests_filter = TestsFilter::default(); + + let resolved = resolve_config( + test_target, + &fork_targets, + &mut BlockNumberMap::default(), + &tests_filter, + ) + .await + .unwrap(); + + assert_eq!(resolved.test_cases.len(), 1); + assert!(!resolved.test_cases[0].config.ignored); + assert!(resolved.test_cases[0].config.fork_config.is_some()); + + let fork_config = resolved.test_cases[0].config.fork_config.as_ref().unwrap(); + assert_eq!(fork_config.url.as_str(), "https://example.com/"); + assert_eq!(fork_config.block_number.0, 100); + } + + #[tokio::test] + async fn test_name_filtered_test_still_resolves_fork_config() { + let test_case = create_test_case_with_config( + "filtered_out_test", + false, + Some(RawForkConfig::Named("valid_fork".into())), + ); + + let test_target = create_test_target_with_cases(vec![test_case]); + + let fork_targets = vec![create_fork_target( + "valid_fork", + "https://example.com", + BlockId::BlockNumber(100), + )]; + + let tests_filter = TestsFilter::from_flags( + Some("different_pattern".to_string()), + false, + Vec::new(), + false, + false, + false, + FailedTestsCache::default(), + ); + + let resolved = resolve_config( + test_target, + &fork_targets, + &mut BlockNumberMap::default(), + &tests_filter, + ) + .await + .unwrap(); + + assert_eq!(resolved.test_cases.len(), 1); + assert!(!resolved.test_cases[0].config.ignored); + assert!(resolved.test_cases[0].config.fork_config.is_some()); + + let fork_config = resolved.test_cases[0].config.fork_config.as_ref().unwrap(); + assert_eq!(fork_config.url.as_str(), "https://example.com/"); + assert_eq!(fork_config.block_number.0, 100); + } + + #[tokio::test] + async fn test_mixed_scenarios_with_ignored_filter() { + let test_cases = vec![ + create_test_case_with_config( + "ignored_with_valid_fork", + true, + Some(RawForkConfig::Named("valid_fork".into())), + ), + create_test_case_with_config( + "matching_test", + false, + Some(RawForkConfig::Named("valid_fork".into())), + ), + create_test_case_with_config( + "non_matching_test", + false, + Some(RawForkConfig::Named("valid_fork".into())), + ), + create_test_case_with_config("no_fork_test", false, None), + ]; + + let test_target = create_test_target_with_cases(test_cases); + let fork_targets = vec![create_fork_target( + "valid_fork", + "https://example.com", + BlockId::BlockNumber(200), + )]; + + let tests_filter = TestsFilter::from_flags( + Some("matching".to_string()), + false, + Vec::new(), + false, + false, + false, + FailedTestsCache::default(), + ); + + let resolved = resolve_config( + test_target, + &fork_targets, + &mut BlockNumberMap::default(), + &tests_filter, + ) + .await + .unwrap(); + + assert_eq!(resolved.test_cases.len(), 4); + + // Check ignored test - should have no fork config resolved + let ignored_test = &resolved.test_cases[0]; + assert_eq!(ignored_test.name, "ignored_with_valid_fork"); + assert!(ignored_test.config.ignored); + assert!(ignored_test.config.fork_config.is_none()); + + // Check matching test - should have fork config resolved + let matching_test = &resolved.test_cases[1]; + assert_eq!(matching_test.name, "matching_test"); + assert!(!matching_test.config.ignored); + assert!(matching_test.config.fork_config.is_some()); + + // Check non-matching test - should still have fork config resolved (name filtering happens later) + let non_matching_test = &resolved.test_cases[2]; + assert_eq!(non_matching_test.name, "non_matching_test"); + assert!(!non_matching_test.config.ignored); + assert!(non_matching_test.config.fork_config.is_some()); + + // Check no-fork test - should work fine + let no_fork_test = &resolved.test_cases[3]; + assert_eq!(no_fork_test.name, "no_fork_test"); + assert!(!no_fork_test.config.ignored); + assert!(no_fork_test.config.fork_config.is_none()); + } + + #[tokio::test] + async fn test_only_ignored_filter_skips_non_ignored_fork_resolution() { + let test_cases = vec![ + create_test_case_with_config( + "ignored_test", + true, + Some(RawForkConfig::Named("valid_fork".into())), + ), + create_test_case_with_config( + "non_ignored_test", + false, + Some(RawForkConfig::Named("valid_fork".into())), + ), + ]; + + let test_target = create_test_target_with_cases(test_cases); + let fork_targets = vec![create_fork_target( + "valid_fork", + "https://example.com", + BlockId::BlockNumber(400), + )]; + + let tests_filter = TestsFilter::from_flags( + None, + false, + Vec::new(), + true, + false, + false, + FailedTestsCache::default(), + ); + + let resolved = resolve_config( + test_target, + &fork_targets, + &mut BlockNumberMap::default(), + &tests_filter, + ) + .await + .unwrap(); + + assert_eq!(resolved.test_cases.len(), 2); + + // Ignored test should have fork config resolved since it should be run + let ignored_test = &resolved.test_cases[0]; + assert_eq!(ignored_test.name, "ignored_test"); + assert!(ignored_test.config.ignored); + assert!(ignored_test.config.fork_config.is_some()); + + // Non-ignored test should not have fork config resolved since it won't be run + let non_ignored_test = &resolved.test_cases[1]; + assert_eq!(non_ignored_test.name, "non_ignored_test"); + assert!(!non_ignored_test.config.ignored); + assert!(non_ignored_test.config.fork_config.is_none()); + } + + #[tokio::test] + async fn test_include_ignored_filter_resolves_all_fork_configs() { + let test_cases = vec![ + create_test_case_with_config( + "ignored_test", + true, + Some(RawForkConfig::Named("valid_fork".into())), + ), + create_test_case_with_config( + "non_ignored_test", + false, + Some(RawForkConfig::Named("valid_fork".into())), + ), + ]; + + let test_target = create_test_target_with_cases(test_cases); + + let fork_targets = vec![create_fork_target( + "valid_fork", + "https://example.com", + BlockId::BlockNumber(500), + )]; + + let tests_filter = TestsFilter::from_flags( + None, + false, + Vec::new(), + false, + true, + false, + FailedTestsCache::default(), + ); + + let resolved = resolve_config( + test_target, + &fork_targets, + &mut BlockNumberMap::default(), + &tests_filter, + ) + .await + .unwrap(); + + assert_eq!(resolved.test_cases.len(), 2); + + for test_case in &resolved.test_cases { + assert!(test_case.config.fork_config.is_some()); + let fork_config = test_case.config.fork_config.as_ref().unwrap(); + assert_eq!(fork_config.url.as_str(), "https://example.com/"); + assert_eq!(fork_config.block_number.0, 500); + } + + let ignored_test = &resolved.test_cases[0]; + assert_eq!(ignored_test.name, "ignored_test"); + assert!(ignored_test.config.ignored); + + let non_ignored_test = &resolved.test_cases[1]; + assert_eq!(non_ignored_test.name, "non_ignored_test"); + assert!(!non_ignored_test.config.ignored); + } + + #[tokio::test] + async fn test_fork_config_resolution_with_inline_config() { + let test_case = create_test_case_with_config( + "test_with_inline_fork", + false, + Some(RawForkConfig::Inline(InlineForkConfig { + url: "https://inline-fork.com".parse().unwrap(), + block: BlockId::BlockNumber(123), + })), + ); + + let test_target = create_test_target_with_cases(vec![test_case]); + let tests_filter = TestsFilter::default(); + + let resolved = resolve_config( + test_target, + &[], + &mut BlockNumberMap::default(), + &tests_filter, + ) + .await + .unwrap(); + + assert_eq!(resolved.test_cases.len(), 1); + + let test_case = &resolved.test_cases[0]; + assert!(!test_case.config.ignored); + assert!(test_case.config.fork_config.is_some()); + + let fork_config = test_case.config.fork_config.as_ref().unwrap(); + assert_eq!(fork_config.url.as_str(), "https://inline-fork.com/"); + assert_eq!(fork_config.block_number.0, 123); + } + + #[tokio::test] + async fn test_overridden_fork_config_resolution() { + let test_case = create_test_case_with_config( + "test_with_overridden_fork", + false, + Some(RawForkConfig::Overridden(OverriddenForkConfig { + name: "base_fork".into(), + block: BlockId::BlockNumber(999), + })), + ); + + let test_target = create_test_target_with_cases(vec![test_case]); + let fork_targets = vec![create_fork_target( + "base_fork", + "https://base-fork.com", + BlockId::BlockNumber(100), + )]; + + let tests_filter = TestsFilter::default(); + + let resolved = resolve_config( + test_target, + &fork_targets, + &mut BlockNumberMap::default(), + &tests_filter, + ) + .await + .unwrap(); + + assert_eq!(resolved.test_cases.len(), 1); + let test_case = &resolved.test_cases[0]; + assert!(test_case.config.fork_config.is_some()); + + let fork_config = test_case.config.fork_config.as_ref().unwrap(); + assert_eq!(fork_config.url.as_str(), "https://base-fork.com/"); + assert_eq!(fork_config.block_number.0, 999); + } + + #[tokio::test] + async fn test_skip_filter_does_not_affect_fork_resolution() { + let test_case = create_test_case_with_config( + "test_to_be_skipped", + false, + Some(RawForkConfig::Named("valid_fork".into())), + ); + + let test_target = create_test_target_with_cases(vec![test_case]); + + let fork_targets = vec![create_fork_target( + "valid_fork", + "https://example.com", + BlockId::BlockNumber(600), + )]; + + let tests_filter = TestsFilter::from_flags( + None, + false, + vec!["skipped".to_string()], + false, + false, + false, + FailedTestsCache::default(), + ); + + let resolved = resolve_config( + test_target, + &fork_targets, + &mut BlockNumberMap::default(), + &tests_filter, + ) + .await + .unwrap(); + + assert_eq!(resolved.test_cases.len(), 1); + + let test_case = &resolved.test_cases[0]; + assert!(!test_case.config.ignored); + assert!(test_case.config.fork_config.is_some()); + + let fork_config = test_case.config.fork_config.as_ref().unwrap(); + assert_eq!(fork_config.url.as_str(), "https://example.com/"); + assert_eq!(fork_config.block_number.0, 600); + } } diff --git a/crates/forge/src/test_filter.rs b/crates/forge/src/test_filter.rs index 3b6255d4c7..f37f0de6f3 100644 --- a/crates/forge/src/test_filter.rs +++ b/crates/forge/src/test_filter.rs @@ -118,6 +118,7 @@ impl TestsFilter { } } +#[cfg(test)] impl Default for TestsFilter { fn default() -> Self { Self { From 0da75a2aed4fd71d7a02a3040db888f628e3f50d Mon Sep 17 00:00:00 2001 From: Abeeujah Date: Wed, 1 Oct 2025 10:12:27 +0100 Subject: [PATCH 8/8] dev: Remove Default impl for TestsFilter --- crates/forge/src/run_tests/resolve_config.rs | 47 +++++++++++++++++--- crates/forge/src/test_filter.rs | 13 ------ 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/crates/forge/src/run_tests/resolve_config.rs b/crates/forge/src/run_tests/resolve_config.rs index 49ec690191..8918f632be 100644 --- a/crates/forge/src/run_tests/resolve_config.rs +++ b/crates/forge/src/run_tests/resolve_config.rs @@ -30,12 +30,11 @@ pub async fn resolve_config( let env_ignore_fork_tests = env_ignore_fork_tests(); for case in test_target.test_cases { - let ignored = - case.config.ignored || (env_ignore_fork_tests && case.config.fork_config.is_some()); test_cases.push(TestCaseWithResolvedConfig { config: TestCaseResolvedConfig { available_gas: case.config.available_gas, - ignored, + ignored: case.config.ignored + || (env_ignore_fork_tests && case.config.fork_config.is_some()), fork_config: if tests_filter.should_be_run(&case) { resolve_fork_config(case.config.fork_config, block_number_map, fork_targets) .await? @@ -218,6 +217,16 @@ mod tests { Some(RawForkConfig::Named("non_existent".into())), )]); + let tests_filter = TestsFilter::from_flags( + None, + false, + Vec::new(), + false, + true, + false, + FailedTestsCache::default(), + ); + assert!( resolve_config( mocked_tests, @@ -227,7 +236,7 @@ mod tests { BlockId::BlockNumber(120) )], &mut BlockNumberMap::default(), - &TestsFilter::default(), + &tests_filter, ) .await .is_err() @@ -285,7 +294,15 @@ mod tests { BlockId::BlockNumber(100), )]; - let tests_filter = TestsFilter::default(); + let tests_filter = TestsFilter::from_flags( + None, + false, + Vec::new(), + false, + true, + false, + FailedTestsCache::default(), + ); let resolved = resolve_config( test_target, @@ -551,7 +568,15 @@ mod tests { ); let test_target = create_test_target_with_cases(vec![test_case]); - let tests_filter = TestsFilter::default(); + let tests_filter = TestsFilter::from_flags( + None, + false, + Vec::new(), + false, + true, + false, + FailedTestsCache::default(), + ); let resolved = resolve_config( test_target, @@ -591,7 +616,15 @@ mod tests { BlockId::BlockNumber(100), )]; - let tests_filter = TestsFilter::default(); + let tests_filter = TestsFilter::from_flags( + None, + false, + Vec::new(), + false, + true, + false, + FailedTestsCache::default(), + ); let resolved = resolve_config( test_target, diff --git a/crates/forge/src/test_filter.rs b/crates/forge/src/test_filter.rs index f37f0de6f3..06a3efdb29 100644 --- a/crates/forge/src/test_filter.rs +++ b/crates/forge/src/test_filter.rs @@ -118,19 +118,6 @@ impl TestsFilter { } } -#[cfg(test)] -impl Default for TestsFilter { - fn default() -> Self { - Self { - name_filter: NameFilter::All, - ignored_filter: IgnoredFilter::All, - last_failed_filter: false, - skip_filter: Vec::new(), - failed_tests_cache: FailedTestsCache::default(), - } - } -} - impl TestCaseFilter for TestsFilter { fn should_be_run(&self, test_case: &TestCase) -> bool where