TheSeamau5 / elm-check

Property Based Testing in Elm
70 stars 20 forks source link

Applicative-style combinators for properties #1

Closed sgraf812 closed 9 years ago

sgraf812 commented 9 years ago

Mostly the stuff I wrote down in the comment of the first commit.

This makes it possible to write code like

prop_identity_n = "Identity" `describedBy` (\x -> x == identity x) `on` int 0 1000 `sample` 200

instead of

prop_identity_n = propertyN 200 "Identity" (\x -> x == identity x) (int 0 1000) 

I also applied a minor change to TestResult, making it a single result instead of a list thereof. Additionally, the values are now formatted as a list of strings rather than as tuples (sorry about that).

This mostly shines for properties with arity greater than 1.

TheSeamau5 commented 9 years ago

Wow. This looks really cool. I'm totally playing with this today and then see how it goes.

So, if I get this right, for multi arity, you can just do:

prop_inverseAdd = 
  "Addition and subtraction are inverse operations" 
  `describedBy` 
    (\x y = x + -y == x - y) 
  `on` anyFloat 
  `on` anyFloat 
  `sample` 300
TheSeamau5 commented 9 years ago

I like that you flattened TestResult down to a Result, but I'll have to test stuff to see that everything works correctly.

The idea originally was that property would output a list of results. The list would detail every attempt it has made (the input, the seed used, whether the test passed or failed). And in theory you could use that list for debugging so you could see for which values your function is returning correct values and for which values it isn't and thus, spot a pattern.

So, I don't know, I'll have to think about that change. I agree that it's one list less to think about (and that's always a good thing). But is it the best way?

sgraf812 commented 9 years ago

You are right, of course. I'm generally not really comfy about the data representations, so my next step would be to refactor that... Something like TestOutput rather feels like a Map String (List TestResult) (only if we keep naming checks as a requirement though). Also I don't feel quite satisfied about using Result on two identical record types if all we need was a testFailed : Bool. Also I'm unsure if we should keep successful TestResults at all.

I won't have the time to do that anytime soon because of my exam period, but I'll come back to it in 4 weeks or so.

TheSeamau5 commented 9 years ago

Yeah, sure. Take your time. And good luck on your exams.

The two identical record types is kind of an accident really. They weren't originally identical. Now that you point it out it does look weird.

In the meantime I'll see how people will use the library and see what kind of information they'd like to see reported and what other feedback I get from them.

sgraf812 commented 9 years ago

I made quite a few changes, also to the used types such as TestResult. Most notably, TestOutput is now an alias for Dict String (List TestResult). I had to touch the mergeTestResult* as a consequence.

I'm quite confident that I could preserve most of the behavior, but take as much time as you need for testing things out.

TheSeamau5 commented 9 years ago

Cool. Thanks. I'm going to merge it but piece-meal. Don't worry, it's going in for the next version, but since I'm adding shrinking, I realized that I need to make a lot of structural changes. Thanks again!