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

Update q-s-m to allow bracketing #1739

Merged
merged 13 commits into from
Mar 4, 2020
Merged

Update q-s-m to allow bracketing #1739

merged 13 commits into from
Mar 4, 2020

Conversation

mrBliss
Copy link
Contributor

@mrBliss mrBliss commented Mar 3, 2020

Fixes #1715

  • Update dependency on quickcheck-state-machine

  • Use runCommands' in *.StateMachine

    The new runCommands' command no longer lives in PropertyM, which means we can finally do proper bracketing.

    We don't use each bracketed function where possible, as in some cases we do some manual closing and reopening.

@mrBliss mrBliss added the consensus issues related to ouroboros-consensus label Mar 3, 2020
@mrBliss mrBliss requested a review from edsko March 3, 2020 13:15
Copy link
Contributor

@edsko edsko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice :)

-- TODO label tags
prettyCommands smUnused hist prop
where
test :: QSM.Commands (At Cmd Blk IO) (At Resp Blk IO)
-> QC.PropertyM IO
-> IO
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay :)

The new `runCommands'` command no longer lives in `PropertyM`, which means we
can finally do proper bracketing.

We don't use each bracketed function where possible, as in some cases we do
some manual closing and reopening.
Use the infrastructure we wrote that has since been integrated in q-s-m.
Use the infrastructure we wrote that has since been integrated in q-s-m.
Use the infrastructure we wrote that has since been integrated in q-s-m.
Use the infrastructure we wrote that has since been integrated in q-s-m.
Use the infrastructure we wrote that has since been integrated in q-s-m.
It has become part of quickcheck-state-machine.
@mrBliss
Copy link
Contributor Author

mrBliss commented Mar 4, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 4, 2020

@iohk-bors iohk-bors bot merged commit 5a9677f into master Mar 4, 2020
@iohk-bors iohk-bors bot deleted the mrBliss/update-q-s-m branch March 4, 2020 13:01
iohk-bors bot added a commit that referenced this pull request Jun 9, 2020
2228: Fix broken q-s-m tests and all bugs that appeared r=mrBliss a=mrBliss

In #1739, I made some changes as some of our q-s-m infrastructure had been adopted by quickcheck-state-machines. Unfortunately, because of confusion with different types adopted in quickcheck-state-machines than the ones we use, our postconditions started checking the result of the implementation with the result of the *implementation* instead of the result of the model 🤦.

Revert the changes that led to this and fix all bugs there were hidden because of it.


Co-authored-by: Thomas Winant <[email protected]>
coot pushed a commit that referenced this pull request May 16, 2022
2228: Fix broken q-s-m tests and all bugs that appeared r=mrBliss a=mrBliss

In #1739, I made some changes as some of our q-s-m infrastructure had been adopted by quickcheck-state-machines. Unfortunately, because of confusion with different types adopted in quickcheck-state-machines than the ones we use, our postconditions started checking the result of the implementation with the result of the *implementation* instead of the result of the model 🤦.

Revert the changes that led to this and fix all bugs there were hidden because of it.


Co-authored-by: Thomas Winant <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus issues related to ouroboros-consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid PropertyM in tests
2 participants