diff --git a/DEV-ADR.md b/DEV-ADR.md index ccbd48d9eb9..cf9df6c3a76 100644 --- a/DEV-ADR.md +++ b/DEV-ADR.md @@ -23,6 +23,51 @@ To complete To complete --> +### 4. Guidelines for crate test utilities + +date: 2025-07-25 +status: Accepted + +### Context + +- For testing, crates may need to define reusable test utilities +- Some of those utilities may need to be exposed to child crates to enable or facilitate writing tests +- We want to isolate test utilities from production code as much as possible +- We want to limit the number of feature flags so the Rust compiler may reuse built artifacts effectively, reducing build time + +### Decision + +Test utilities should be organized following these rules: + +1. Each crate must define its private and public test utilities in a `test` module +2. A test utility should be made public if it is reused in a child crate or in the crate's integration tests +3. Making a test utility public should not add any dependency to the crate +4. Private test utilities should be behind `cfg(test)` +5. Importing a public test utility should always require an import path that contains a `test` module, so their misuse + in production code can be easily identified +6. No feature flag should be added to isolate test utilities from production code + +Specific cases: + +- Test doubles (fakes, dummies, mocks, stubs, etc.): should be located in a `test::double` module +- Builders of test data should be located in a `test::builder` module +- Extending a type with a test-only method should be done using extension traits located in the `test` module, which + forces the import of a `test`-scoped symbol + - Their name should be suffixed with `TestExtension` + - Implementation of those traits should preferably be below the trait definition, but if some methods need to access + private properties, then the implementation may be located below their extended type + +#### Consequences + +- Enhanced consistency in code organization +- Improved discoverability of test utilities +- Clearer distinction between production and test code +- Simplified maintenance of test utilities +- Child crates can reuse common test utilities when needed +- Feature flag usage is minimized, avoiding unnecessary recompilation + +--- + ### 3. Guidelines for Dummy Test Doubles **Date:** 2025-07-22 diff --git a/flake.nix b/flake.nix index afa3edfb065..73618eea2ba 100644 --- a/flake.nix +++ b/flake.nix @@ -100,7 +100,7 @@ // args); mithril-stm = buildPackage ./mithril-stm/Cargo.toml null {}; - mithril-common = buildPackage ./mithril-common/Cargo.toml mithril-stm.cargoArtifacts { cargoExtraArgs = "-p mithril-common --features full"; }; + mithril-common = buildPackage ./mithril-common/Cargo.toml mithril-stm.cargoArtifacts { cargoExtraArgs = "-p mithril-common"; }; mithril = buildPackage null mithril-common.cargoArtifacts { doCheck = false; }; diff --git a/internal/cardano-node/mithril-cardano-node-chain/Cargo.toml b/internal/cardano-node/mithril-cardano-node-chain/Cargo.toml index 70d4eeae3ac..19573fc6824 100644 --- a/internal/cardano-node/mithril-cardano-node-chain/Cargo.toml +++ b/internal/cardano-node/mithril-cardano-node-chain/Cargo.toml @@ -35,7 +35,6 @@ kes-summed-ed25519 = { version = "0.2.1", features = [ "serde_enabled", "sk_clone_enabled", ] } -mithril-common = { path = "../../../mithril-common", features = ["test_tools"] } mockall = { workspace = true } pallas-crypto = "0.33.0" slog-async = { workspace = true } diff --git a/internal/cardano-node/mithril-cardano-node-internal-database/Cargo.toml b/internal/cardano-node/mithril-cardano-node-internal-database/Cargo.toml index 265181a7f9f..6d76286f8c0 100644 --- a/internal/cardano-node/mithril-cardano-node-internal-database/Cargo.toml +++ b/internal/cardano-node/mithril-cardano-node-internal-database/Cargo.toml @@ -29,7 +29,6 @@ walkdir = "2.5.0" [dev-dependencies] criterion = { version = "0.6.0", features = ["html_reports", "async_tokio"] } -mithril-common = { path = "../../../mithril-common", version = ">=0.5", features = ["test_tools"] } mockall = { workspace = true } slog-async = { workspace = true } slog-term = { workspace = true } diff --git a/internal/mithril-aggregator-client/Cargo.toml b/internal/mithril-aggregator-client/Cargo.toml index 437c9ca7e6c..c9b1e90dda3 100644 --- a/internal/mithril-aggregator-client/Cargo.toml +++ b/internal/mithril-aggregator-client/Cargo.toml @@ -28,7 +28,6 @@ tokio = { workspace = true } [dev-dependencies] http = "1.3.1" httpmock = "0.7.0" -mithril-common = { path = "../../mithril-common", version = ">=0.5", features = ["test_tools"] } mockall = { workspace = true } slog-async = { workspace = true } slog-term = { workspace = true } diff --git a/internal/mithril-dmq/Cargo.toml b/internal/mithril-dmq/Cargo.toml index 4e657f037f4..662284125c9 100644 --- a/internal/mithril-dmq/Cargo.toml +++ b/internal/mithril-dmq/Cargo.toml @@ -24,7 +24,6 @@ slog = { workspace = true } tokio = { workspace = true, features = ["sync"] } [dev-dependencies] -mithril-common = { path = "../../mithril-common", features = ["test_tools"] } mockall = { workspace = true } slog-async = { workspace = true } slog-term = { workspace = true } diff --git a/internal/mithril-era/Cargo.toml b/internal/mithril-era/Cargo.toml index aa6e6eb3e93..279e24137e2 100644 --- a/internal/mithril-era/Cargo.toml +++ b/internal/mithril-era/Cargo.toml @@ -21,5 +21,4 @@ serde_json = { workspace = true } thiserror = { workspace = true } [dev-dependencies] -mithril-common = { path = "../../mithril-common", features = ["test_tools"] } tokio = { workspace = true, features = ["macros"] } diff --git a/internal/mithril-metric/Cargo.toml b/internal/mithril-metric/Cargo.toml index 214efc34972..02f0b30b123 100644 --- a/internal/mithril-metric/Cargo.toml +++ b/internal/mithril-metric/Cargo.toml @@ -22,7 +22,6 @@ slog = { workspace = true } tokio = { workspace = true } [dev-dependencies] -mithril-common = { path = "../../mithril-common", features = ["test_tools"] } prometheus-parse = "0.2.5" slog-async = { workspace = true } slog-term = { workspace = true } diff --git a/internal/mithril-persistence/Cargo.toml b/internal/mithril-persistence/Cargo.toml index b314733bbc1..2ae4e2b059c 100644 --- a/internal/mithril-persistence/Cargo.toml +++ b/internal/mithril-persistence/Cargo.toml @@ -26,5 +26,4 @@ thiserror = { workspace = true } tokio = { workspace = true } [dev-dependencies] -mithril-common = { path = "../../mithril-common", features = ["test_tools"] } tokio = { workspace = true, features = ["macros", "time"] } diff --git a/internal/mithril-resource-pool/Cargo.toml b/internal/mithril-resource-pool/Cargo.toml index e346b928ef0..17f5b6764e1 100644 --- a/internal/mithril-resource-pool/Cargo.toml +++ b/internal/mithril-resource-pool/Cargo.toml @@ -18,5 +18,4 @@ thiserror = { workspace = true } tokio = { workspace = true } [dev-dependencies] -mithril-common = { path = "../../mithril-common", features = ["test_tools"] } tokio = { workspace = true, features = ["macros", "time"] } diff --git a/internal/signed-entity/mithril-signed-entity-preloader/Cargo.toml b/internal/signed-entity/mithril-signed-entity-preloader/Cargo.toml index 3b75ef97e53..69cfb73cc6a 100644 --- a/internal/signed-entity/mithril-signed-entity-preloader/Cargo.toml +++ b/internal/signed-entity/mithril-signed-entity-preloader/Cargo.toml @@ -21,7 +21,6 @@ slog = { workspace = true } tokio = { workspace = true } [dev-dependencies] -mithril-common = { path = "../../../mithril-common", features = ["test_tools"] } mockall = { workspace = true } slog-async = { workspace = true } slog-term = { workspace = true } diff --git a/internal/tests/mithril-api-spec/Cargo.toml b/internal/tests/mithril-api-spec/Cargo.toml index b77d61e6237..42192c2cff3 100644 --- a/internal/tests/mithril-api-spec/Cargo.toml +++ b/internal/tests/mithril-api-spec/Cargo.toml @@ -17,4 +17,4 @@ serde_yml = "0.0.12" warp = { workspace = true } [dev-dependencies] -mithril-common = { path = "../../../mithril-common", features = ["test_tools"] } +mithril-common = { path = "../../../mithril-common" } diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index 0d706f8e19a..bbe93c6811f 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -29,7 +29,7 @@ hex = { workspace = true } mithril-cardano-node-chain = { path = "../internal/cardano-node/mithril-cardano-node-chain" } mithril-cardano-node-internal-database = { path = "../internal/cardano-node/mithril-cardano-node-internal-database" } mithril-cli-helper = { path = "../internal/mithril-cli-helper" } -mithril-common = { path = "../mithril-common", features = ["full"] } +mithril-common = { path = "../mithril-common" } mithril-dmq = { path = "../internal/mithril-dmq", optional = true } mithril-doc = { path = "../internal/mithril-doc" } mithril-era = { path = "../internal/mithril-era" } @@ -82,10 +82,7 @@ criterion = { version = "0.6.0", features = ["html_reports", "async_tokio"] } http = "1.3.1" httpmock = "0.7.0" mithril-api-spec = { path = "../internal/tests/mithril-api-spec" } -mithril-common = { path = "../mithril-common", features = [ - "allow_skip_signer_certification", - "test_tools", -] } +mithril-common = { path = "../mithril-common", features = ["allow_skip_signer_certification"] } mithril-test-http-server = { path = "../internal/tests/mithril-test-http-server" } mockall = { workspace = true } slog-scope = "4.4.0" diff --git a/mithril-client-cli/Cargo.toml b/mithril-client-cli/Cargo.toml index 6a51bb4f904..241d9e4140b 100644 --- a/mithril-client-cli/Cargo.toml +++ b/mithril-client-cli/Cargo.toml @@ -62,5 +62,5 @@ zip = "4.3.0" [dev-dependencies] httpmock = "0.7.0" -mithril-common = { path = "../mithril-common", features = ["test_tools"] } +mithril-common = { path = "../mithril-common" } mockall = { workspace = true } diff --git a/mithril-client/Cargo.toml b/mithril-client/Cargo.toml index 1c8a93c1e22..19b55dd74bd 100644 --- a/mithril-client/Cargo.toml +++ b/mithril-client/Cargo.toml @@ -90,9 +90,6 @@ bon = "3.6.4" hex = { workspace = true } http = "1.3.1" httpmock = "0.7.0" -mithril-common = { path = "../mithril-common", version = ">=0.5", default-features = false, features = [ - "test_tools", -] } mockall = { workspace = true } sha2 = "0.10.9" slog-async = { workspace = true } diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml index c9cf9bece2b..4d7d6a21a39 100644 --- a/mithril-common/Cargo.toml +++ b/mithril-common/Cargo.toml @@ -24,9 +24,6 @@ crate-type = ["lib", "cdylib", "staticlib"] [features] default = ["rug-backend"] -# Full feature set -full = ["test_tools"] - # Enables `rug-backend` features for `mithril-stm` dependency rug-backend = ["mithril-stm/rug-backend"] # Enables `num-integer-backend` features for `mithril-stm` dependency @@ -35,8 +32,6 @@ num-integer-backend = ["mithril-stm/num-integer-backend"] # Disable signer certification, to be used only for tests allow_skip_signer_certification = [] -# Enable all tests tools -test_tools = [] [dependencies] anyhow = { workspace = true } diff --git a/mithril-common/Makefile b/mithril-common/Makefile index ecd1289a532..c3e68deb9a6 100644 --- a/mithril-common/Makefile +++ b/mithril-common/Makefile @@ -1,11 +1,6 @@ .PHONY: all build test check doc check-all-features-set CARGO = cargo -# All the crates features excluding the two that have little to no impact to the code -FEATURES := $(shell ${CARGO} metadata --quiet --no-deps \ - | jq -r '.packages[] | select(.name=="mithril-common") | .features \ - | del(.allow_skip_signer_certification) | del(.portable) \ - | keys | join(" ")') all: test build @@ -25,19 +20,3 @@ doc: bench: ${CARGO} bench --features full --verbose - -# Compute the powerset of all the given features and save it to a file -.feature-sets: - powerset() { [ $$# -eq 0 ] && echo || (shift; powerset "$$@") | while read r ; do echo "$$1 $$r"; echo "$$r"; done };\ - powerset $$(echo "$(FEATURES)") > .features-sets - -check-all-features-set: .feature-sets - # Read the file to run cargo clippy on all those features sets - cat .features-sets | while read features_set; do \ - echo "Clippy common with feature '$$features_set''"; \ - ${CARGO} clippy -p mithril-common --features "$$features_set"; \ - done - echo "Clippy common without features"; \ - ${CARGO} clippy -p mithril-common - - rm .features-sets diff --git a/mithril-common/src/api_version.rs b/mithril-common/src/api_version.rs index c34c35c3430..1fcb52eaba7 100644 --- a/mithril-common/src/api_version.rs +++ b/mithril-common/src/api_version.rs @@ -79,7 +79,6 @@ impl Default for APIVersionProvider { } } -#[cfg(any(test, feature = "test_tools"))] impl crate::test::api_version_extensions::ApiVersionProviderTestExtension for APIVersionProvider { fn update_open_api_versions(&mut self, open_api_versions: HashMap) { self.open_api_versions = open_api_versions; diff --git a/mithril-common/src/crypto_helper/cardano/key_certification.rs b/mithril-common/src/crypto_helper/cardano/key_certification.rs index f80bbebc378..3bf8b12095a 100644 --- a/mithril-common/src/crypto_helper/cardano/key_certification.rs +++ b/mithril-common/src/crypto_helper/cardano/key_certification.rs @@ -292,7 +292,6 @@ impl KeyRegWrapper { } } -#[cfg(any(test, feature = "test_tools"))] mod test_extensions { use crate::test::crypto_helper::ProtocolInitializerTestExtension; diff --git a/mithril-common/src/entities/block_range.rs b/mithril-common/src/entities/block_range.rs index c678dcdb902..fbe3d5793d1 100644 --- a/mithril-common/src/entities/block_range.rs +++ b/mithril-common/src/entities/block_range.rs @@ -201,7 +201,6 @@ impl ExactSizeIterator for BlockRangesSequence { } } -#[cfg(any(test, feature = "test_tools"))] mod test_extensions { use crate::test::entities_extensions::BlockRangeTestExtension; diff --git a/mithril-common/src/lib.rs b/mithril-common/src/lib.rs index b1973536571..b6bbf84d1bc 100644 --- a/mithril-common/src/lib.rs +++ b/mithril-common/src/lib.rs @@ -18,8 +18,6 @@ pub mod messages; pub mod protocol; pub mod signable_builder; -#[cfg(any(test, feature = "test_tools"))] -#[cfg_attr(docsrs, doc(cfg(feature = "test_tools")))] pub mod test; pub use entities::{CardanoNetwork, MagicId}; diff --git a/mithril-relay/Cargo.toml b/mithril-relay/Cargo.toml index f8cb6c9de05..3d434e3ca95 100644 --- a/mithril-relay/Cargo.toml +++ b/mithril-relay/Cargo.toml @@ -34,7 +34,7 @@ libp2p = { version = "0.56.0", features = [ "websocket", "yamux", ] } -mithril-common = { path = "../mithril-common", features = ["full"] } +mithril-common = { path = "../mithril-common" } mithril-doc = { path = "../internal/mithril-doc" } mithril-test-http-server = { path = "../internal/tests/mithril-test-http-server" } reqwest = { workspace = true, features = [ diff --git a/mithril-signer/Cargo.toml b/mithril-signer/Cargo.toml index ecc5ef080fb..0c5711422d0 100644 --- a/mithril-signer/Cargo.toml +++ b/mithril-signer/Cargo.toml @@ -26,7 +26,7 @@ hex = { workspace = true } mithril-cardano-node-chain = { path = "../internal/cardano-node/mithril-cardano-node-chain" } mithril-cardano-node-internal-database = { path = "../internal/cardano-node/mithril-cardano-node-internal-database" } mithril-cli-helper = { path = "../internal/mithril-cli-helper" } -mithril-common = { path = "../mithril-common", features = ["full"] } +mithril-common = { path = "../mithril-common" } mithril-dmq = { path = "../internal/mithril-dmq", optional = true } mithril-doc = { path = "../internal/mithril-doc" } mithril-era = { path = "../internal/mithril-era" } @@ -64,7 +64,6 @@ tikv-jemallocator = { version = "0.6.0", optional = true } criterion = { version = "0.6.0", features = ["html_reports", "async_tokio"] } http = "1.3.1" httpmock = "0.7.0" -mithril-common = { path = "../mithril-common" } mockall = { workspace = true } prometheus-parse = "0.2.5" slog-scope = "4.4.0" diff --git a/mithril-test-lab/mithril-end-to-end/Cargo.toml b/mithril-test-lab/mithril-end-to-end/Cargo.toml index 8f2c6c36041..dd7f413f469 100644 --- a/mithril-test-lab/mithril-end-to-end/Cargo.toml +++ b/mithril-test-lab/mithril-end-to-end/Cargo.toml @@ -26,7 +26,7 @@ clap = { workspace = true } indicatif = { version = "0.18.0", features = ["tokio"] } mithril-cardano-node-chain = { path = "../../internal/cardano-node/mithril-cardano-node-chain" } mithril-cardano-node-internal-database = { path = "../../internal/cardano-node/mithril-cardano-node-internal-database" } -mithril-common = { path = "../../mithril-common", features = ["full"] } +mithril-common = { path = "../../mithril-common" } mithril-doc = { path = "../../internal/mithril-doc" } reqwest = { workspace = true, features = ["default"] } serde = { workspace = true }