Skip to content

Commit

Permalink
chore(ci): remove testing feature from business logic and test it
Browse files Browse the repository at this point in the history
Signed-off-by: Dori Medini <[email protected]>
  • Loading branch information
dorimedini-starkware committed Feb 9, 2025
1 parent 63895ea commit 6660613
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 9 deletions.
2 changes: 1 addition & 1 deletion crates/papyrus_node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion crates/starknet_committer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 3 additions & 2 deletions crates/starknet_consensus_orchestrator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand All @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion crates/starknet_gateway/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions crates/starknet_http_server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
2 changes: 1 addition & 1 deletion crates/starknet_state_sync/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
62 changes: 62 additions & 0 deletions workspace_tests/package_integrity_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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::<Vec<String>>();
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:
Expand Down
2 changes: 1 addition & 1 deletion workspace_tests/toml_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub(crate) enum PackageEntryValue {
#[derive(Clone, Debug, Serialize, Deserialize)]
pub(crate) struct CrateCargoToml {
pub(crate) package: HashMap<String, PackageEntryValue>,
dependencies: Option<HashMap<String, DependencyValue>>,
pub(crate) dependencies: Option<HashMap<String, DependencyValue>>,
#[serde(rename = "dev-dependencies")]
pub(crate) dev_dependencies: Option<HashMap<String, DependencyValue>>,
pub(crate) lints: Option<HashMap<String, LintValue>>,
Expand Down

0 comments on commit 6660613

Please sign in to comment.