fantasyland / fantasy-land

Specification for interoperability of common algebraic structures in JavaScript
MIT License
10.12k stars 374 forks source link

Correct way to test laws. #271

Closed dustinws closed 6 years ago

dustinws commented 7 years ago

Hello fantasy crew!

I'm wondering, what is the "correct" way to ensure that a custom data type is lawful?

My initial thought was to include modules from the /laws directory and define a custom eq method.

Here is an example for an IO implementation conforming to Functor (using jest, if you're interested)

const VAL = 1;
const eq = (a, b) =>
  a.run() === b.run();

test('Functor', () => {
  const identity = functor.identity(IO.of)(eq)(VAL);
  const composition =
    functor.composition(IO.of)(eq)(add(1))(multiply(10))(VAL);

  expect(identity).toBe(true);
  expect(composition).toBe(true);
});

This was going quite well until I tried to write a test for Foldable. It seems that the test for Foldable depends on a toArray method that isn't mentioned anywhere in the spec. This leads me to believe that the the checks defined in /laws are more guidelines for writing tests, instead of being the tests themselves.

I was hoping someone could provide some clarification. I would love to be able to prove in tests that implementations are law-abiding for any given version of FL.

davidchambers commented 7 years ago

@dustinws, have a look at fantasyland/fantasy-laws#1. The code is complete, but I'd like to improve the readme before merging the pull request. Please share any thoughts that occur to you. :)

dustinws commented 7 years ago

Awesome, that's definitely what I'm looking for! Do you have any clue when it'll be merged in?

davidchambers commented 7 years ago

Awesome, that's definitely what I'm looking for!

Excellent!

Do you have any clue when it'll be merged in?

It's on my list, but I don't like to commit to dates. Next weekend, perhaps?

dustinws commented 7 years ago

Cool, I'm looking forward to it. Since the only thing is touching up the readme, would you say it's safe to use that branch until everything is merged / published?

davidchambers commented 7 years ago

That depends on your use case, @dustinws. It's probably a good idea to point to a commit hash rather than the branch name as I'll delete the branch once the pull request has been merged. GitHub never seems to garbage collect dangling commits, so even if I rebase you should be able to continue to access the existing commits by their hashes.

dustinws commented 7 years ago

That makes sense. Thanks for the tip!

I'll stay tuned for when fantasy-laws gets published and switch over, until then I'll reference the last commit from that branch.

Thanks for the help @davidchambers

dustinws commented 7 years ago

Oh, one last question before we close this out -

I noticed in my original testing adventure (see example in the OP) that when testing sum types, the "error" case would never pass.

Examples include Nothing, Left, etc..

When testing a type like Maybe or Either, should we only test the default type? Maybe.of => Just, Either.of => Right, or should we be testing all cases?

If we are testing all cases, could those failures just be in how /laws is setup, and will the new fantasy-laws allow us to do that?

This would always pop up in the Applicative and Functor tests.

davidchambers commented 7 years ago

When testing a type like Maybe or Either, should we only test the default type?

We should definitely be testing every one of a type's data constructors. Have a look at MaybeArb :: Arbitrary a -> Arbitrary (Maybe a) and see how it is used in the Sanctuary test suite.

davidchambers commented 6 years ago

fantasy-laws is now available via npm. :tada: