Skip to content
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

Best practices for mocking dependencies? #34

Open
christianheussy opened this issue Nov 30, 2024 · 3 comments
Open

Best practices for mocking dependencies? #34

christianheussy opened this issue Nov 30, 2024 · 3 comments

Comments

@christianheussy
Copy link

christianheussy commented Nov 30, 2024

I'm really enjoying statig but am facing challenges with unit testing state transitions. My goal is to:

  1. Initialize the state machine to a specific state.
  2. Mock dependencies within the state machine.
  3. Verify state transitions after handling events.

I'm using serde for state initialization (as suggested in #25), but this prevents me from setting mock expectations beforehand. See a reproducible example here: #33.

The issue is that neither UninitializedStateMachine<M> nor InitializedStateMachine<M> implement core::ops::DerefMut, preventing mutable access for mock configuration before initialization.

A potential solution is implementing DerefMut (#32), but I'm unsure if providing mutable access at all times is ideal. Perhaps implementing it only for UninitializedStateMachine<M> would be better? I'm open to suggestions.

@christianheussy
Copy link
Author

Interior mutability is one option, however this leads to a lot of extra code to deal with the requirement that everything captured in the async state methods must be Send. This leads to wrapping everything owned by the state machine with Arc<Mutex<_>> which is verbose and not ideal.

@christianheussy
Copy link
Author

Ended up using Context instead, which permits passing in mutable references. I like the ergonomics of using items owned by the state machine, but this is an acceptable compromise.

@mdeloof
Copy link
Owner

mdeloof commented Jan 5, 2025

I'm open to adding some way to get a mutable reference to the state machine's internal data, but we probably shouldn't use DerefMut for this, as the docs state that it should only be implemented on smart pointers.

An alternative would be to provide methods such as inner() and inner_mut() to access the internal data. But as you say, having mutable access at all times isn't ideal as it could break the state machine's internal invariants. It should be fine to use on UninitializedStateMachine, but maybe we should mark those methods unsafe if we were to implement them on InitializedStateMachine?

@mdeloof mdeloof reopened this Jan 5, 2025
christianheussy pushed a commit to christianheussy/statig that referenced this issue Jan 7, 2025
Problem: Mutable access to `Inner` is required for ergonomic unit
testing.

Solution: Add `inner` and `inner_mut` to:
- `awaitable::state_machine{UninitializedStateMachine,
  InitializedStateMachine}`
- `blocking::state_machine{UninitializedStateMachine,
  InitializedStateMachine}`
Mark the methods as `unsafe` for the case of an
`InitializedStateMachine` to force users to think critically about
mutating in thise case.

Testing: `cargo test`

Issue: mdeloof#34
christianheussy pushed a commit to christianheussy/statig that referenced this issue Jan 7, 2025
Problem: Mutable access to `Inner` is required for ergonomic unit
testing.

Solution: Add `inner` and `inner_mut` to:
- `awaitable::state_machine{UninitializedStateMachine,
  InitializedStateMachine}`
- `blocking::state_machine{UninitializedStateMachine,
  InitializedStateMachine}`
Mark the methods as `unsafe` for the case of an
`InitializedStateMachine` to force users to think critically about
mutating in thise case.

Testing: `cargo test`

Issue: mdeloof#34
christianheussy pushed a commit to christianheussy/statig that referenced this issue Jan 11, 2025
Problem: Mutable access to `Inner` is required for ergonomic unit
testing.

Solution: Add `inner` and `inner_mut` to:
- `awaitable::state_machine{UninitializedStateMachine,
  InitializedStateMachine}`
- `blocking::state_machine{UninitializedStateMachine,
  InitializedStateMachine}`
Mark the methods as `unsafe` for the case of an
`InitializedStateMachine` to force users to think critically about
mutating in thise case.

Testing: `cargo test`

Issue: mdeloof#34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants