Skip to content

chore: add dev adr for guidelines on writting test utilities + remove test_tools and full features from mithril-common #2653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions DEV-ADR.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
1 change: 0 additions & 1 deletion internal/mithril-aggregator-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
1 change: 0 additions & 1 deletion internal/mithril-dmq/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
1 change: 0 additions & 1 deletion internal/mithril-era/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
1 change: 0 additions & 1 deletion internal/mithril-metric/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
1 change: 0 additions & 1 deletion internal/mithril-persistence/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
1 change: 0 additions & 1 deletion internal/mithril-resource-pool/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
2 changes: 1 addition & 1 deletion internal/tests/mithril-api-spec/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
7 changes: 2 additions & 5 deletions mithril-aggregator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion mithril-client-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
3 changes: 0 additions & 3 deletions mithril-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
5 changes: 0 additions & 5 deletions mithril-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 }
Expand Down
21 changes: 0 additions & 21 deletions mithril-common/Makefile
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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
1 change: 0 additions & 1 deletion mithril-common/src/api_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<OpenAPIFileName, Version>) {
self.open_api_versions = open_api_versions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,6 @@ impl KeyRegWrapper {
}
}

#[cfg(any(test, feature = "test_tools"))]
mod test_extensions {
use crate::test::crypto_helper::ProtocolInitializerTestExtension;

Expand Down
1 change: 0 additions & 1 deletion mithril-common/src/entities/block_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ impl ExactSizeIterator for BlockRangesSequence {
}
}

#[cfg(any(test, feature = "test_tools"))]
mod test_extensions {
use crate::test::entities_extensions::BlockRangeTestExtension;

Expand Down
2 changes: 0 additions & 2 deletions mithril-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
2 changes: 1 addition & 1 deletion mithril-relay/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
3 changes: 1 addition & 2 deletions mithril-signer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion mithril-test-lab/mithril-end-to-end/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
Loading