Avoid unnecessary use of `run`

[edsko]

The top-level of a QSM test typically looks something like this:

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 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 into

forAllCommands ... $ \cmds -> QC.monadicIO $ do
  (hist, prop) <- .... run ( runCommands ... ) ...
  prettyCommands ...

which, crucially, would allow us to do something like

forAllCommands ... $ \cmds -> QC.monadicIO $ do
  (hist, prop) <- .... run (bracket ... $ \r -> runCommands .. r .. ) ...
  prettyCommands ...

NOTE: I'm getting a test failure, but that exists on master already:

  Rqlite
    parallel:                                                      FAIL (0.30s)
      *** Failed! Falsified (after 1 test and 4 shrinks):
      ParallelCommands
        { prefix =
            Commands
              { unCommands =
                  [ Command
                      Spawn
                      0
                      1
                      s
                      1
                      s
                      Nothing
                      Resp
                      (Right (Spawned (Reference (Symbolic (Var 0)))))
                      [ Var 0 ]
                  ]
              }
        , suffixes = []
        }
      Use --quickcheck-replay=727617 to reproduce.
  ErrorEncountered

I don't think that's related to this PR at all though.

[stevana]

Would it make sense to name the run function that doesn't use PropertyM as runCommands' :: m ... and then define the old run function runCommands = run . runCommands' :: PropertyM m..., and thus not break backwards compatibility?

NOTE: I'm getting a test failure, but that exists on master already

Yeah, there's a ticket for this already #356

[edsko]

Would it make sense to name the run function that doesn't use PropertyM as runCommands' :: m ... and then define the old run function runCommands = run . runCommands' :: PropertyM m..., and thus not break backwards compatibility?

Yes, I suppose that would be sensible, if you are worried about breaking compatibility.

[mrBliss]

Would it make sense to name the run function that doesn't use PropertyM as runCommands' :: m ... and then define the old run function runCommands = run . runCommands' :: PropertyM m..., and thus not break backwards compatibility?

I have updated the PR to follow this approach.

[stevana]

Sweet, lets get this merged!

[mrBliss]

Sweet, lets get this merged!

Yeah, let's merge this :+1: