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

StateMachineTest's apply method should use the current state, not the next state. #488

Open
liarokapisv opened this issue Jul 31, 2024 · 3 comments
Labels
quality-of-life This issue proposes a change that will improve the UX of proptest but isn't necessarily a "feature"

Comments

@liarokapisv
Copy link

liarokapisv commented Jul 31, 2024

Assuming that the model is implemented properly, it can be useful to have access to the model corresponding to the system under test before the transition is applied.

This is useful when the transition result depends on the current state. For example, when modeling an actual state machine, we may have certain side effects that only apply to specific states.

I think the current state should be provided and if needed, the user can apply the transition to the state to get the next one. This is how rapidcheck does it, for instance.

This can be currently emulated by keeping the current state as a field and explicitly passing the next state to the next SUT at the end of apply, but it's pretty cumbersome.

@liarokapisv liarokapisv changed the title StateMachineTest's apply method should use the current state, not the next state. StateMachineTest's apply method should use the current state, not the next state. Jul 31, 2024
@matthew-russo matthew-russo added the quality-of-life This issue proposes a change that will improve the UX of proptest but isn't necessarily a "feature" label Aug 10, 2024
@tzemanovic
Copy link
Contributor

hi, you should also be able to override the StateMachineTest::test_sequential to achieve this which may be less cumbersome than state passing from a field.

Could you pls describe a scenario in which this is useful? I'd be happy to improve support for this, but I'd like to understand the motivation for it first.

@liarokapisv
Copy link
Author

here is a use-case for embassy.
We store the previous state in sut.status and set that to ref_state at the end of apply. We need this because the transition action has different side-effects depending on which state it occurs at.

@tzemanovic
Copy link
Contributor

Thanks for sharing your use-case!

Because the current impl of StateMachineTest::test_sequential runs a loop over the generated transitions, it has to call ReferenceStateMachine::apply on ref_state anyway to use for the next iteration, but we could clone it first to give StateMachineTest::apply both the previous and current ref state, rather than

I think the current state should be provided and if needed, the user can apply the transition to the state to get the next one.

It would be a breaking change either way though and I'm not sure it's totally necessary yet, so let's leave this open for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quality-of-life This issue proposes a change that will improve the UX of proptest but isn't necessarily a "feature"
Projects
None yet
Development

No branches or pull requests

3 participants