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

ApplicativeError: catchNonFatal, catchNonFatalEval and fromTry are not covered by laws #1908

Open
alexandru opened this issue Sep 12, 2017 · 4 comments

Comments

@alexandru
Copy link
Member

There are currently no laws in ApplicativeErrorLaws that cover:

  • catchNonFatal
  • catchNonFatalEval
  • fromTry

When I check the associated laws for a type-class, I expect to get full code coverage given good enough Arbitrary values.

@alexandru alexandru changed the title ApplicativeError: catchNonFatal, catchNonFatalEval and fromTry are not tested ApplicativeError: catchNonFatal, catchNonFatalEval and fromTry are not covered by laws Sep 12, 2017
@kailuowang
Copy link
Contributor

When I check the associated laws for a type-class, I expect to get full code coverage given good enough Arbitrary values.

Unfortunately that's the not the case for most of type classes when it comes to implemented methods. Some of them are covered by tests for the type classes themselves, some are not covered at all. @non recently added a bunch of such tests to laws and name them with a "Ref" suffix. Maybe we should make it a convention.

@johnynek
Copy link
Contributor

+1 to make it a convention: if there is a default implementation that may be slow, we should add a law that every implementation matches that but may have better ways to implement.

@peterneyens
Copy link
Collaborator

Maybe we can use something like I mentioned here.

In essence create a wrapper for every type class which uses the implementations of the fundamental operations of that type class of the data type it wraps. We can than compare the actual implementation (eg ApplicativeError[Foo, E]) is equal to the default implementation (ApplicativeError[ApplicativeErrorDefaultWrapper[Foo, E, ?], E]).

This way the consistency laws wouldn't need to reimplement the default implementation.
I'll try to create a more concrete proof of concept.

@kailuowang
Copy link
Contributor

kailuowang commented Sep 14, 2017

@peterneyens yeah, we dropped the ball on that one. I like that approach. It seems possible to generate check methods using macro so that all future methods with default implementation can be automatically tested. It maybe even possible to generate the wrappers, but that seems trickier.

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

No branches or pull requests

4 participants