-
Notifications
You must be signed in to change notification settings - Fork 51
refactor: rethink dummies test doubles #2647
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a standardized Dummy trait in mithril-common to replace scattered pub fn dummy() methods across the codebase, centralizing test double creation and improving code organization.
- Adds a new
Dummytrait for standardized test double creation - Migrates all existing
dummy()methods to implement the new trait - Establishes clear placement guidelines for test doubles based on type visibility
Reviewed Changes
Copilot reviewed 131 out of 132 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| mithril-common/src/test_utils/double/mod.rs | Introduces the new Dummy trait definition |
| mithril-common/src/test_utils/double/dummies.rs | Implements Dummy for entities and messages in a centralized location |
| Multiple entity/message files | Removes old dummy() methods and adds Dummy trait imports |
| Multiple test files | Updates imports to use the new test_utils::double::Dummy |
| DEV-ADR.md | Adds architectural decision record for dummy test double guidelines |
mithril-aggregator/src/dependency_injection/builder/support/compatibility.rs
Show resolved
Hide resolved
dlachaume
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
jpraynaud
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
turmelclem
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🦄
A trait similar to the standard `Default` but dedicated to dummy test double creation.
…d make them impl Dummy trait
and, when applicable, move the `Dummy` impl to a `test::double::dummies` module.
* mithril-cardano-node-chain from `0.1.4` to `0.1.5` * mithril-dmq from `0.1.4` to `0.1.5` * mithril-era from `0.1.2` to `0.1.3` * mithril-signed-entity-preloader from `0.0.7` to `0.0.8` * mithril-api-spec from `0.1.1` to `0.1.2` * mithril-aggregator from `0.7.74` to `0.7.75` * mithril-client-cli from `0.12.22` to `0.12.23` * mithril-client from `0.12.22` to `0.12.23` * mithril-common from `0.6.9` to `0.6.10` * mithril-relay from `0.1.47` to `0.1.48` * mithril-signer from `0.2.260` to `0.2.261`
475ef3d to
dd5d4b8
Compare
Content
This PR add a new
Dummytrait inmithril-commonthat supersede and standardize the previouspub fn dummy()methods that are scattered across the codebase.Implementations of this traits are placed in crates
testmodule (ortest-utilsfor common) when all implementers members are public which is almost always the case, if they have not public members implementations are placed below the implementers types.This PR also add a dev-adr to specify how dummies should be added to the project.
Pre-submit checklist
Issue(s)
Relates to #2580