quickcheck-dynamic icon indicating copy to clipboard operation
quickcheck-dynamic copied to clipboard

Introduce a QuickCheck-style `MoreActions` type modifier to make it easier to increase the number of actions on average in tests

Open MaximilianAlgehed opened this issue 1 year ago • 3 comments
trafficstars

I'm not 100% sure I like this design, but now you can do quickCheck $ prop_BlaBla . getMoreActions to increase the number of actions generated in traces without changing other stuff.

The thing I don't like about this is that it doesn't take into account wanting to add other modifiers at the same time. What if we wanted a modifier PostiveActions for polarity for example? quickCheck $ prop_BlaBla . getMoreActions . getPositiveActions would be a bit messy to implement as it stands now. Something to think about?

Checklist:

  • [x] Check source-code formatting is consistent

MaximilianAlgehed avatar Sep 19 '24 15:09 MaximilianAlgehed

I'm not happy with the design here. The current design makes it possible to do something like: newtype MoreActions state and newtype PositiveActions state but it doesn't make it possible to combine MoreActions and PositiveActions. I think that's a shame and it would be nice to be able to combine type-level modifiers. One way to get around this would be to do something like:

class ArbitraryWithOptions o a where
  arbitraryWithOptions :: o -> Gen a
  shrinkWithOptions :: o -> a -> [a]
instance ArbitraryWithOptions (GenActionsOptions state) a => Arbitrary a
instance ArbitraryWithOptions (GenActionsOptions state) (Actions state)
instance ArbitraryWithOptions (GenActionsOptions state) a => ArbitraryWithOptions (GenActionsOptions state) (MoreActions a)
instance ArbitraryWithOptions (GenActionsOptions state) a => ArbitraryWithOptions (GenActionsOptions state) (PositiveActions a)

But that would introduce the extra overhead of the ArbitraryWithOptions class, but maybe that's worth-while? It would certainly be more flexible than the current approach.

Thoughts very much welcome from @abailly-iohk and @UlfNorell.

MaximilianAlgehed avatar Sep 20 '24 09:09 MaximilianAlgehed

Another more promising approach might be something like this:

data QCDProperty state
toQCD :: Testable p => (Actions state -> p) -> QCDProperty state
instance StateModel state => Testable (QCDProperty state)
withMoreCommands :: Rational -> QCDProperty state -> QCDProperty state
withPositiveActions :: QCDProperty state -> QCDProperty state

We could add some sugar to this to avoid having to write crap like withMoreCommands . toQCD . prop_whatever and make withMoreCommands slightly more polymorphic.

I think this introduces a nicer interface than having the MoreActions type based interface that is more compositional and it addresses @UlfNorell's issue with fixed modifer in MoreActions. Furthermore, it avoids the orphaned instances for Arbitrary that my proposal above suffers from.

MaximilianAlgehed avatar Sep 20 '24 10:09 MaximilianAlgehed

I've re-designed this to be in line with my last comment. It's much nicer now than it was before. It wouldn't hurt to have @abailly-iohk and @UlfNorell look it over and see if there is anything else we'd like to do here.

MaximilianAlgehed avatar Sep 21 '24 06:09 MaximilianAlgehed

closes #85

MaximilianAlgehed avatar Oct 22 '24 08:10 MaximilianAlgehed

@abailly-iohk I think we are ready to merge this, but the CI rules are a little bit in the way: image

MaximilianAlgehed avatar Oct 24 '24 08:10 MaximilianAlgehed

I never remember how to disable those, but will do

ghost avatar Oct 24 '24 08:10 ghost