From 66606138048698e5c992777e4486e3042e6558ef Mon Sep 17 00:00:00 2001 From: Dori Medini Date: Fri, 7 Feb 2025 20:03:13 +0200 Subject: [PATCH] chore(ci): remove testing feature from business logic and test it Signed-off-by: Dori Medini --- crates/papyrus_node/Cargo.toml | 2 +- crates/starknet_committer/Cargo.toml | 5 +- .../Cargo.toml | 5 +- crates/starknet_gateway/Cargo.toml | 2 +- crates/starknet_http_server/Cargo.toml | 5 +- crates/starknet_state_sync/Cargo.toml | 2 +- workspace_tests/package_integrity_test.rs | 62 +++++++++++++++++++ workspace_tests/toml_utils.rs | 2 +- 8 files changed, 76 insertions(+), 9 deletions(-) diff --git a/crates/papyrus_node/Cargo.toml b/crates/papyrus_node/Cargo.toml index ca63e2e9d6c..275a7ba5ec6 100644 --- a/crates/papyrus_node/Cargo.toml +++ b/crates/papyrus_node/Cargo.toml @@ -35,7 +35,7 @@ papyrus_sync.workspace = true reqwest = { workspace = true, features = ["blocking", "json"] } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["arbitrary_precision"] } -starknet_api = { workspace = true, features = ["testing"] } +starknet_api.workspace = true starknet_class_manager_types.workspace = true starknet_client.workspace = true starknet_consensus.workspace = true diff --git a/crates/starknet_committer/Cargo.toml b/crates/starknet_committer/Cargo.toml index 16aa7e0d444..043533caeea 100644 --- a/crates/starknet_committer/Cargo.toml +++ b/crates/starknet_committer/Cargo.toml @@ -12,10 +12,13 @@ pretty_assertions.workspace = true rstest.workspace = true serde_json.workspace = true starknet-types-core = { workspace = true, features = ["hash"] } -starknet_patricia = { workspace = true, features = ["testing"] } +starknet_patricia.workspace = true thiserror.workspace = true tokio = { workspace = true, features = ["rt"] } tracing.workspace = true +[dev-dependencies] +starknet_patricia = { path = "../starknet_patricia", features = ["testing"] } + [lints] workspace = true diff --git a/crates/starknet_consensus_orchestrator/Cargo.toml b/crates/starknet_consensus_orchestrator/Cargo.toml index ca4fc00402b..50cce906907 100644 --- a/crates/starknet_consensus_orchestrator/Cargo.toml +++ b/crates/starknet_consensus_orchestrator/Cargo.toml @@ -26,11 +26,11 @@ serde.workspace = true serde_json = { workspace = true, features = ["arbitrary_precision"] } starknet-types-core.workspace = true starknet_api.workspace = true -starknet_batcher_types = { workspace = true, features = ["testing"] } +starknet_batcher_types.workspace = true starknet_class_manager_types.workspace = true starknet_consensus.workspace = true starknet_infra_utils.workspace = true -starknet_state_sync_types = { workspace = true, features = ["testing"] } +starknet_state_sync_types.workspace = true thiserror.workspace = true tokio = { workspace = true, features = ["full"] } tokio-util = { workspace = true, features = ["rt"] } @@ -52,6 +52,7 @@ rstest.workspace = true serde_json.workspace = true starknet_batcher_types = { path = "../starknet_batcher_types", features = ["testing"] } starknet_infra_utils.path = "../starknet_infra_utils" +starknet_state_sync_types = { path = "../starknet_state_sync_types", features = ["testing"] } test-case.workspace = true [lints] diff --git a/crates/starknet_gateway/Cargo.toml b/crates/starknet_gateway/Cargo.toml index 1048eaf3b65..d51c8aa08ba 100644 --- a/crates/starknet_gateway/Cargo.toml +++ b/crates/starknet_gateway/Cargo.toml @@ -14,7 +14,7 @@ testing = [] [dependencies] async-trait.workspace = true axum.workspace = true -blockifier = { workspace = true, features = ["testing"] } +blockifier.workspace = true blockifier_test_utils.workspace = true cairo-lang-starknet-classes.workspace = true futures.workspace = true diff --git a/crates/starknet_http_server/Cargo.toml b/crates/starknet_http_server/Cargo.toml index 4a7dcafad2f..92fd3623f73 100644 --- a/crates/starknet_http_server/Cargo.toml +++ b/crates/starknet_http_server/Cargo.toml @@ -6,7 +6,7 @@ license.workspace = true repository.workspace = true [features] -testing = ["reqwest", "starknet_api/testing"] +testing = ["reqwest", "starknet_api/testing", "starknet_gateway_types/testing"] [lints] workspace = true @@ -20,7 +20,7 @@ reqwest = { workspace = true, optional = true } serde.workspace = true serde_json.workspace = true starknet_api.workspace = true -starknet_gateway_types = { workspace = true, features = ["testing"] } +starknet_gateway_types.workspace = true starknet_infra_utils.workspace = true starknet_sequencer_infra.workspace = true starknet_sequencer_metrics.workspace = true @@ -38,4 +38,5 @@ metrics-exporter-prometheus.workspace = true reqwest.workspace = true serde_json.workspace = true starknet-types-core.workspace = true +starknet_gateway_types = { path = "../starknet_gateway_types", features = ["testing"] } tracing-test.workspace = true diff --git a/crates/starknet_state_sync/Cargo.toml b/crates/starknet_state_sync/Cargo.toml index e0f93b8b56f..35978a1164c 100644 --- a/crates/starknet_state_sync/Cargo.toml +++ b/crates/starknet_state_sync/Cargo.toml @@ -19,7 +19,7 @@ papyrus_storage.workspace = true papyrus_sync.workspace = true serde.workspace = true starknet-types-core.workspace = true -starknet_api = { workspace = true, features = ["testing"] } +starknet_api.workspace = true starknet_class_manager_types.workspace = true starknet_client.workspace = true starknet_sequencer_infra.workspace = true diff --git a/workspace_tests/package_integrity_test.rs b/workspace_tests/package_integrity_test.rs index fdde859ee9b..510de4df48a 100644 --- a/workspace_tests/package_integrity_test.rs +++ b/workspace_tests/package_integrity_test.rs @@ -2,6 +2,20 @@ use std::path::PathBuf; use crate::toml_utils::{CrateCargoToml, DependencyValue, PackageEntryValue, MEMBER_TOMLS}; +/// Hard-coded list of crates that are allowed to use test code in their (non-dev) dependencies. +/// Should only contain test-related crates. +static CRATES_ALLOWED_TO_USE_TEST_CODE: [&str; 6] = [ + "starknet_integration_tests", + "papyrus_test_utils", + "blockifier_test_utils", + "papyrus_load_test", + "mempool_test_utils", + // The CLI crate exposes tests that require test utils in dependencies. + // TODO(Dori): Consider splitting the build of the CLI crate to a test build and a production + // build. + "starknet_committer_and_os_cli", +]; + #[test] fn test_package_names_match_directory() { let mismatched_packages: Vec<_> = MEMBER_TOMLS @@ -24,6 +38,54 @@ fn test_package_names_match_directory() { ); } +/// Tests that no dependency activates the "testing" feature (dev-dependencies may). +#[test] +fn test_no_testing_feature_in_business_logic() { + let mut testing_feature_deps: Vec<_> = MEMBER_TOMLS + .iter() + // Ignore test-specific crates. + .filter_map(|(package_name, toml)| { + if CRATES_ALLOWED_TO_USE_TEST_CODE.contains(&package_name.as_str()) { + None + } else { + Some((package_name, toml)) + } + }) + // Ignore crates without dependencies. + .filter_map(|(package_name, toml)| { + toml.dependencies.as_ref().map(|dependencies| (package_name, dependencies)) + }) + .filter_map(|(package_name, dependencies)| { + let testing_feature_deps = dependencies + .iter() + .filter_map(|(name, value)| { + if let DependencyValue::Object { features: Some(features), .. } = value { + if features.contains(&"testing".to_string()) { + Some(name.clone()) + } else { + None + } + } else { + None + } + }) + .collect::>(); + if testing_feature_deps.is_empty() { + None + } else { + Some((package_name.clone(), testing_feature_deps)) + } + }) + .collect(); + testing_feature_deps.sort(); + assert!( + testing_feature_deps.is_empty(), + "The following crates have (non-testing) dependencies with the 'testing' feature \ + activated. If the crate is a test crate, add it to {}.\n{testing_feature_deps:#?}", + stringify!(CRATES_ALLOWED_TO_USE_TEST_CODE) + ); +} + /// For smooth publishing: all *local* (workspace member) dependencies in [dev-dependencies] should /// be via path, and not `workspace = true`. /// Reason: diff --git a/workspace_tests/toml_utils.rs b/workspace_tests/toml_utils.rs index c21e9b0612b..9103107f0fb 100644 --- a/workspace_tests/toml_utils.rs +++ b/workspace_tests/toml_utils.rs @@ -46,7 +46,7 @@ pub(crate) enum PackageEntryValue { #[derive(Clone, Debug, Serialize, Deserialize)] pub(crate) struct CrateCargoToml { pub(crate) package: HashMap, - dependencies: Option>, + pub(crate) dependencies: Option>, #[serde(rename = "dev-dependencies")] pub(crate) dev_dependencies: Option>, pub(crate) lints: Option>,