Get features from all targets (#2570)

<!-- Reference any GitHub issues resolved by this PR -->

Closes #2568 
Closes #2567

## Introduced changes

<!-- A brief description of the changes -->

- Fixed features support for optimized compilation
- Changed the starknet artifacts loading logic so they work with custom
test targets.
- Artifacts are now loaded from all test targets
- Artifacts are now compiled to casm concurrently
- Refactored the scarb-api crate

## Checklist

<!-- Make sure all of these are complete -->

- [x] Linked relevant issue
- [x] Updated relevant documentation
- [x] Added relevant tests
- [x] Performed self-review of the code
- [x] Added changes to `CHANGELOG.md`

---------

Co-authored-by: Piotr Magiera <56825108+piotmag769@users.noreply.github.com>

.github/workflows/ci.yml

@@ -44,7 +44,7 @@ jobs:
     needs: build-test-forge-e2e-nextest-archive
     strategy:
       matrix:
-        partition: [1, 2, 3, 4, 5, 6, 7, 8]
+        partition: [ 1, 2, 3, 4, 5, 6, 7, 8 ]
     steps:
       - name: Extract branch name
         if: github.event_name != 'pull_request'
@@ -119,6 +119,9 @@ jobs:
       - uses: software-mansion/setup-universal-sierra-compiler@v1
       - run: |
           cargo test --package forge --features scarb_2_8_3 e2e::contract_artifacts
+      - run: |
+          cargo test --package forge --features scarb_2_8_3 e2e::features
+      - run: |
           cargo test --package scarb-api --features scarb_2_8_3 get_starknet_artifacts_path
 
   test-forge-runner:

CHANGELOG.md

@@ -15,6 +15,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   `snforge test --build-profile -- --show-inlined-functions`
 - You can't use now `--coverage` and `--build-profile` flags at the same time. If you want to use both, you need to run
   `snforge test` twice with different flags.
+- Contract artifacts are compiled to CASM concurrently.
+- Starknet artifacts are now loaded from all tests targets
+- Cairo Edition in `snforge init` template set to `2024_07`
+
+#### Fixed
+
+- Scarb features work with optimized compilation
+- Custom test targets are now supported with optimized compilation
 
 ## [0.32.0] - 2024-10-16
 
@@ -40,12 +48,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - When using test name filter with `--exact` flag, forge will try to compile only the selected test.
 
-### Forge
-
-#### Changed
-
-- Cairo Edition in `snforge init` template set to `2024_07`
-
 ## [0.31.0] - 2024-09-26
 
 ### Cast

crates/cheatnet/tests/common/mod.rs

@@ -26,7 +26,9 @@ use conversions::string::TryFromHexStr;
 use conversions::IntoConv;
 use runtime::starknet::context::build_context;
 use scarb_api::metadata::MetadataCommandExt;
-use scarb_api::{get_contracts_artifacts_and_source_sierra_paths, ScarbCommand};
+use scarb_api::{
+    get_contracts_artifacts_and_source_sierra_paths, target_dir_for_workspace, ScarbCommand,
+};
 use starknet::core::utils::get_selector_from_name;
 use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector};
 use starknet_api::deprecated_contract_class::EntryPointType;
@@ -72,12 +74,12 @@ pub fn get_contracts() -> ContractsData {
         .manifest_path("tests/contracts/Scarb.toml")
         .run()
         .unwrap();
+    let target_dir = target_dir_for_workspace(&scarb_metadata).join("dev");
 
     let package = scarb_metadata.packages.first().unwrap();
 
     let contracts =
-        get_contracts_artifacts_and_source_sierra_paths(&scarb_metadata, &package.id, None, false)
-            .unwrap();
+        get_contracts_artifacts_and_source_sierra_paths(&target_dir, package, false).unwrap();
     ContractsData::try_from(contracts).unwrap()
 }
 

crates/forge/src/run_tests/package.rs

@@ -47,15 +47,14 @@ impl RunForPackageArgs {
         scarb_metadata: &Metadata,
         args: &TestArgs,
         cache_dir: &Utf8PathBuf,
-        snforge_target_dir_path: &Utf8Path,
+        artifacts_dir: &Utf8Path,
         versioned_programs_dir: Utf8PathBuf,
     ) -> Result<RunForPackageArgs> {
-        let raw_test_targets = load_test_artifacts(snforge_target_dir_path, &package)?;
+        let raw_test_targets = load_test_artifacts(artifacts_dir, &package)?;
 
         let contracts = get_contracts_artifacts_and_source_sierra_paths(
-            scarb_metadata,
-            &package.id,
-            None,
+            artifacts_dir,
+            &package,
             !should_compile_starknet_contract_target(
                 &scarb_metadata.app_version_info.version,
                 args.no_optimization,

crates/forge/src/run_tests/workspace.rs

@@ -35,7 +35,7 @@ pub async fn run_for_workspace(args: TestArgs) -> Result<ExitStatus> {
 
     warn_if_snforge_std_not_compatible(&scarb_metadata)?;
 
-    let snforge_target_dir_path =
+    let artifacts_dir_path =
         target_dir_for_workspace(&scarb_metadata).join(&scarb_metadata.current_profile);
 
     let packages: Vec<PackageMetadata> = args
@@ -76,7 +76,7 @@ pub async fn run_for_workspace(args: TestArgs) -> Result<ExitStatus> {
             &scarb_metadata,
             &args,
             &cache_dir,
-            &snforge_target_dir_path,
+            &artifacts_dir_path,
             versioned_programs_dir.clone(),
         )?;
 

crates/forge/src/scarb.rs

@@ -5,11 +5,10 @@ use camino::Utf8Path;
 use configuration::PackageConfig;
 use forge_runner::package_tests::raw::TestTargetRaw;
 use forge_runner::package_tests::TestTargetLocation;
-use scarb_api::ScarbCommand;
-use scarb_metadata::{PackageMetadata, TargetMetadata};
+use scarb_api::{test_targets_by_name, ScarbCommand};
+use scarb_metadata::PackageMetadata;
 use scarb_ui::args::{FeaturesSpec, PackagesFilter};
 use semver::Version;
-use std::collections::HashMap;
 use std::fs::read_to_string;
 use std::io::ErrorKind;
 
@@ -76,27 +75,6 @@ fn build_test_artifacts_with_scarb(filter: PackagesFilter, features: FeaturesSpe
     Ok(())
 }
 
-/// collecting by name allow us to dedup targets
-/// we do it because they use same sierra and we display them without distinction anyway
-fn test_targets_by_name(package: &PackageMetadata) -> HashMap<String, &TargetMetadata> {
-    fn test_target_name(target: &TargetMetadata) -> String {
-        // this is logic copied from scarb: https://github.com/software-mansion/scarb/blob/90ab01cb6deee48210affc2ec1dc94d540ab4aea/extensions/scarb-cairo-test/src/main.rs#L115
-        target
-            .params
-            .get("group-id") // by unit tests grouping
-            .and_then(|v| v.as_str())
-            .map(ToString::to_string)
-            .unwrap_or(target.name.clone()) // else by integration test name
-    }
-
-    package
-        .targets
-        .iter()
-        .filter(|target| target.kind == "test")
-        .map(|target| (test_target_name(target), target))
-        .collect()
-}
-
 pub fn load_test_artifacts(
     target_dir: &Utf8Path,
     package: &PackageMetadata,

crates/forge/test_utils/src/runner.rs

@@ -13,8 +13,8 @@ use forge_runner::{
 };
 use indoc::formatdoc;
 use scarb_api::{
-    get_contracts_artifacts_and_source_sierra_paths, metadata::MetadataCommandExt, ScarbCommand,
-    StarknetContractArtifacts,
+    get_contracts_artifacts_and_source_sierra_paths, metadata::MetadataCommandExt,
+    target_dir_for_workspace, ScarbCommand, StarknetContractArtifacts,
 };
 use shared::command::CommandExt;
 use std::{
@@ -96,17 +96,14 @@ impl Contract {
             .iter()
             .find(|package| package.name == "contract")
             .unwrap();
-
-        let contract = get_contracts_artifacts_and_source_sierra_paths(
-            &scarb_metadata,
-            &package.id,
-            None,
-            false,
-        )
-        .unwrap()
-        .remove(&self.name)
-        .ok_or(anyhow!("there is no contract with name {}", self.name))?
-        .0;
+        let artifacts_dir = target_dir_for_workspace(&scarb_metadata).join("dev");
+
+        let contract =
+            get_contracts_artifacts_and_source_sierra_paths(&artifacts_dir, package, false)
+                .unwrap()
+                .remove(&self.name)
+                .ok_or(anyhow!("there is no contract with name {}", self.name))?
+                .0;
 
         Ok((contract.sierra, contract.casm))
     }

crates/forge/tests/data/contracts/benchmarks/declare_deploy_interact.cairo

@@ -12,7 +12,7 @@ mod HelloStarknet {
         balance: felt252,
     }
 
-    // Increases the balance by the given amount.
+    // Increases the balance by the given amount
     #[external(v0)]
     fn increase_balance(ref self: ContractState, amount: felt252) {
         self.balance.write(self.balance.read() + amount);

crates/forge/tests/data/contracts/hello_starknet.cairo

@@ -18,12 +18,12 @@ mod HelloStarknet {
 
     #[abi(embed_v0)]
     impl IHelloStarknetImpl of super::IHelloStarknet<ContractState> {
-        // Increases the balance by the given amount.
+        // Increases the balance by the given amount
         fn increase_balance(ref self: ContractState, amount: felt252) {
             self.balance.write(self.balance.read() + amount);
         }
 
-        // Returns the current balance.
+        // Returns the current balance
         fn get_balance(self: @ContractState) -> felt252 {
             self.balance.read()
         }

crates/forge/tests/data/contracts/l1_handler_execute_checker.cairo

@@ -29,7 +29,7 @@ mod l1_handler_executor {
 
     #[abi(embed_v0)]
     impl IBalanceTokenImpl of super::IBalanceToken<ContractState> {
-        // Returns the current balance.
+        // Returns the current balance
         fn get_balance(self: @ContractState) -> felt252 {
             self.balance.read()
         }

crates/forge/tests/data/contracts/response_with_2_felts.cairo

@@ -19,7 +19,7 @@ mod ResponseWith2Felts {
 
     #[abi(embed_v0)]
     impl IResponseWith2FeltsImpl of super::IResponseWith2Felts<ContractState> {
-        // Increases the balance by the given amount.
+        // Increases the balance by the given amount
         fn get(self: @ContractState) -> Response {
             Response { a: 8, b: 43 }
         }

crates/forge/tests/data/hello_workspaces/Scarb.toml

@@ -30,5 +30,8 @@ starknet.workspace = true
 fibonacci = { path = "crates/fibonacci" }
 addition = { path = "crates/addition" }
 
+[dev-dependencies]
+snforge_std.workspace = true
+
 [[target.starknet-contract]]
 sierra = true

crates/forge/tests/data/simple_package/src/hello_starknet.cairo

@@ -17,12 +17,12 @@ mod HelloStarknet {
 
     #[abi(embed_v0)]
     impl IHelloStarknetImpl of super::IHelloStarknet<ContractState> {
-        // Increases the balance by the given amount.
+        // Increases the balance by the given amount
         fn increase_balance(ref self: ContractState, amount: felt252) {
             self.balance.write(self.balance.read() + amount);
         }
 
-        // Returns the current balance.
+        // Returns the current balance
         fn get_balance(self: @ContractState) -> felt252 {
             self.balance.read()
         }

crates/forge/tests/data/targets/custom_target/Scarb.toml

@@ -0,0 +1,25 @@
+[package]
+name = "custom_target"
+version = "0.1.0"
+
+[dependencies]
+starknet = "2.7.0"
+
+[dev-dependencies]
+snforge_std = { path = "../../../../../../snforge_std" }
+
+[[target.starknet-contract]]
+
+[[test]]
+name = "custom_target_integrationtest"
+kind = "test"
+source-path = "./tests/tests.cairo"
+test-type = "integration"
+
+[[test]]
+name = "custom_target_unittest"
+kind = "test"
+test-type = "unit"
+
+[tool.snforge]
+exit_first = false

crates/forge/tests/data/targets/custom_target/src/lib.cairo

@@ -0,0 +1,55 @@
+#[starknet::interface]
+trait IHelloStarknet<TContractState> {
+    fn increase_balance(ref self: TContractState, amount: felt252);
+    fn get_balance(self: @TContractState) -> felt252;
+    fn do_a_panic(self: @TContractState);
+    fn do_a_panic_with(self: @TContractState, panic_data: Array<felt252>);
+}
+
+#[starknet::contract]
+mod HelloStarknet {
+    use array::ArrayTrait;
+
+    #[storage]
+    struct Storage {
+        balance: felt252,
+    }
+
+    #[abi(embed_v0)]
+    impl IHelloStarknetImpl of super::IHelloStarknet<ContractState> {
+        // Increases the balance by the given amount
+        fn increase_balance(ref self: ContractState, amount: felt252) {
+            self.balance.write(self.balance.read() + amount);
+        }
+
+        // Returns the current balance
+        fn get_balance(self: @ContractState) -> felt252 {
+            self.balance.read()
+        }
+
+        // Panics
+        fn do_a_panic(self: @ContractState) {
+            let mut arr = ArrayTrait::new();
+            arr.append('PANIC');
+            arr.append('DAYTAH');
+            panic(arr);
+        }
+
+        // Panics with given array data
+        fn do_a_panic_with(self: @ContractState, panic_data: Array<felt252>) {
+            panic(panic_data);
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use snforge_std::{declare, ContractClassTrait};
+    use snforge_std::cheatcodes::contract_class::DeclareResultTrait;
+
+    #[test]
+    fn declare_contract_from_lib() {
+        let _ = declare("HelloStarknet").unwrap().contract_class();
+        assert(2 == 2, 'Should declare');
+    }
+}

crates/forge/tests/data/targets/custom_target/tests/tests.cairo

@@ -0,0 +1,28 @@
+use array::ArrayTrait;
+use result::ResultTrait;
+use option::OptionTrait;
+use traits::TryInto;
+use starknet::ContractAddress;
+use starknet::Felt252TryIntoContractAddress;
+
+use snforge_std::{declare, ContractClassTrait};
+use snforge_std::cheatcodes::contract_class::DeclareResultTrait;
+
+use custom_target::IHelloStarknetDispatcher;
+use custom_target::IHelloStarknetDispatcherTrait;
+
+#[test]
+fn declare_and_call_contract_from_lib() {
+    let contract = declare("HelloStarknet").unwrap().contract_class();
+    let constructor_calldata = @ArrayTrait::new();
+    let (contract_address, _) = contract.deploy(constructor_calldata).unwrap();
+    let dispatcher = IHelloStarknetDispatcher { contract_address };
+
+    let balance = dispatcher.get_balance();
+    assert(balance == 0, 'balance == 0');
+
+    dispatcher.increase_balance(100);
+
+    let balance = dispatcher.get_balance();
+    assert(balance == 100, 'balance != 100');
+}