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

Avoid QuickCheck for properties without free variables #237

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bjornbm
Copy link

@bjornbm bjornbm commented Jul 14, 2019

Properties that have no free variables and have type Bool can be checked
to be True without using QuickCheck. Properties with free variables need
QuickCheck. Some properties may not have free variables but if the type
is not Bool we use QuickCheck on the assumption that they are of some
other Test.QuickCheck.Testable instance.

The purpose of this pull request is to be able to benefit from the prop> syntax for simple properties without pulling in QuickCheck as a dependency.

An example in the wild:

--   >>> isRight (refine @(Not NonEmpty) @[Int] [])
--   True
--
--   >>> isLeft (refine @(Not NonEmpty) @[Int] [1,2])
--   True

Can be clearer (IMO) and more concisely written as:

prop> isRight (refine @(Not NonEmpty) @[Int] [])
prop> isLeft (refine @(Not NonEmpty) @[Int] [1,2])prop> 

(I will note that even when QuickCheck is not used prop> will be slower than >>> due to calling the repl twice.)

bjornbm added 2 commits July 14, 2019 23:39
Properties that have no free variables and have type `Bool` can be checked
to be `True` without using QuickCheck. Properties with free variables need
QuickCheck. Some properties may not have free variables but if the type
is not `Bool` we use QuickCheck on the assumption that they are of some
other `Test.QuickCheck.Testable` instance.
@bjornbm
Copy link
Author

bjornbm commented Jul 15, 2019

Sorry, that failing test case got lost amongst noise related to cabal-v2 and GHC-8.6.5 on my local machine.

In this case with a single use it seems more appropriate to `fmap`
rather than CPP the import.
@quasicomputational
Copy link
Collaborator

Hmm. I can appreciate this as an optimisation, but the UX worries me a shade. At the moment, the rule is simple: if you use prop>, you need a QuickCheck dependency. With this patch, the rule gets more complicated. Is the win in test speed and not needing the QC dependencies sometimes worth the decrease in uniformity?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants