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 6d8b9b2b15..11597441ff 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, package_tests::TestDetails}; +use crate::{TestCaseIsIgnored, expected_result::ExpectedTestResult, package_tests::TestDetails}; use anyhow::Result; use cairo_vm::types::program::Program; use cheatnet::runtime_extensions::forge_config_extension::config::{ @@ -55,3 +55,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 c41248491f..28b01c369b 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 f872e9579c..8918f632be 100644 --- a/crates/forge/src/run_tests/resolve_config.rs +++ b/crates/forge/src/run_tests/resolve_config.rs @@ -1,15 +1,20 @@ 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::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; @@ -19,30 +24,30 @@ 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 config = TestCaseResolvedConfig { - available_gas: case.config.available_gas, - 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?, - fuzzer_config: case.config.fuzzer_config, - disable_predeployed_contracts: case.config.disable_predeployed_contracts, - }; - test_cases.push(TestCaseWithResolvedConfig::new( - &case.name, - case.test_details, - config, - )); + test_cases.push(TestCaseWithResolvedConfig { + config: TestCaseResolvedConfig { + available_gas: case.config.available_gas, + 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? + } else { + None + }, + 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, + }); } Ok(TestTargetWithResolvedConfig { @@ -128,6 +133,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; @@ -149,9 +155,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( @@ -161,44 +196,497 @@ 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())), + )]); + + let tests_filter = TestsFilter::from_flags( + None, + false, + Vec::new(), + false, + true, + false, + FailedTestsCache::default(), + ); 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), - }], - &mut BlockNumberMap::default() + &[create_fork_target( + "definitely_non_existing", + "https://not_taken.com", + BlockId::BlockNumber(120) + )], + &mut BlockNumberMap::default(), + &tests_filter, ) .await .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::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(), 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::from_flags( + None, + false, + Vec::new(), + false, + true, + false, + FailedTestsCache::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::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(), 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 361b843b1a..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 @@ -26,7 +27,7 @@ pub(crate) enum NameFilter { } #[derive(Debug, PartialEq)] -pub(crate) enum IgnoredFilter { +pub enum IgnoredFilter { NotIgnored, Ignored, All, @@ -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,