Avoid unnecessary use of run
#358
Conversation
|
Would it make sense to name the run function that doesn't use
Yeah, there's a ticket for this already #356 |
Yes, I suppose that would be sensible, if you are worried about breaking compatibility. |
The top-level of a QSM test typically looks something like this: ```haskell forAllCommands ... $ \cmds -> QC.monadicIO $ do (hist, prop) <- .... runCommands ... prettyCommands ... ``` The problem is that `runCommands` lives in `PropertyM`. This makes it impossible to do any kind of bracketing. Fortunately, it turns out that this use of `PropertyM` is actually not needed at all. In this commit, I add `runCommands'`, which lives in `m`. To avoid breaking backwards compatibility, `runCommands` still lives in `PropertyM` and is a combination of `run` and `runCommands'`. This will allow us to do something like: ```haskell forAllCommands ... $ \cmds -> QC.monadicIO $ do (hist, prop) <- .... run (bracket ... $ \r -> runCommands' .. r .. ) ... prettyCommands ... ``` Co-authored-by: Thomas Winant <[email protected]> Signed-off-by: Edsko de Vries <[email protected]>
I have updated the PR to follow this approach. |
|
Sweet, lets get this merged! |
Yeah, let's merge this 👍 |
The top-level of a QSM test typically looks something like this:
The problem is that
runCommandslives in PropertyM. This makes it impossible to do any kind of bracketing. Fortunately, it turns out that this use ofPropertyMis actually not needed at all. In this commit, I remove it without having to change much at all, except having to insert a few extra runs in the test. In particular, the above will now turn intowhich, crucially, would allow us to do something like
NOTE: I'm getting a test failure, but that exists on
masteralready:I don't think that's related to this PR at all though.