use GenK for testdata generation in MonadFilter and related laws

[abendt]

this PR is part of (#1858)

all laws functions that were previously deprecated are removed now

open questions:

• one EitherTTest is failing: see comment in file
• i tried to enhance the generators for ObservableK, FluxK & MaybeK to also generate errors (see commented out code & comments in the respective tests). However this lead to various tests failing.
• StateTTests failed with OutOfMemory. i changed the test to use Option instead of ListK (see comment in test)

[1Jajen1]

I'll look over this tomorrow, it's getting a bit late :)
The start looks good, to address your comments: Always write gens that test as much of the input domain as possible, if the tests fail, good, let them fail, shows either a problem with the test or a bug in the code, never is the generator at fault unless you have some wrong invariants (which is rarely the case here).
Some tests actually expect the generated data to be Applicative.just which is fine if the test is explicitly testing it. I'll have a look over the failing tests and we'll decide how to proceed there (either fixing it or commenting them out and opening issues).

Btw this is the pr where I expect a lot of test failures. This covers the most complex parts of arrow and exposes even lesser used apis to better tests, which will make some of them fail and thats a good thing! :)

[1Jajen1]

Ok so the EitherT test:
This was a fun one. The right distributive law comes from https://typelevel.org/cats/typeclasses/alternative.html . But whoever ported it forgot an important detail: In cats the parameter order on ap is reversed, so the law has to be adjusted for that as well (if it even holds). For now comment it out as it's plain wrong and should have never been added like that in the first place. I'll open an issue for it which should be linked in source later.

[1Jajen1]

For the rx2 instances with errors: They are likely not lawful. Just comment out the monad filter laws for them and refer to the other issue I created.

[abendt]

re the rx2 instances: its not just the MonadFilterLaws, its also the TraverseLaws. should there be an extra ticket or is it ok to just link to the existing one?

[1Jajen1]

re the rx2 instances: its not just the MonadFilterLaws, its also the TraverseLaws. should there be an extra ticket or is it ok to just link to the existing one?

Missed that comment. And yeah there should be. The reason for those errors is becomes foldable (for ObservableK) is implemented with blockingGet. That is equal to implementing it for IO by using unsafeRunSync() and I highly doubt we would want that in arrow. I will add a new ticket

[1Jajen1]

Btw how does IO or the project-reactor stuff handle errors?

[abendt]

i have commented out the failing laws now and added links to the github issues you created

Btw how does IO or the project-reactor stuff handle errors?

i have added exceptions to the IO gen. The IOTest runs fine

[abendt]

what about the StateTTest. was it OK to change the used state or is this a bug too?

[1Jajen1]

what about the StateTTest. was it OK to change the used state or is this a bug too?

Ah the change from list as a the monad. The change is good, having lists as the underlying monad is just suboptimal. We are testing StateT anyway so checking it with any monad that's better for testing is fine. (Even Id would be fine, but Option is more fun! 🙈)

[1Jajen1]

Also, do you want to do a pass on the actual tests from the monad hierarchy in this pr as well? Most of them assume no passed gen, so they run on Gen.applicative most of the time afaik. If not I'll probably do it as a follow up pr to this :)

[abendt]

i'd say better in a next PR. i deliberately tried to keep it smaller this time after the first one touched so many parts 😄

[1Jajen1]

Ok, but to avoid duplicate work, do you want to work on it or should I. It is looking at the laws, seeing where Applicative.just is used (or Gen.applicative()) and replacing it with GenK. We can also split this per typeclass but I don't think it's going to be that much. Although this again has the potential to produce many many failed tests.
Either way, then I'll give this a quick review and see if we can't get it merged now ^^

[1Jajen1]

One nit around the comment for eitherT, the rest is good, thanks a lot for doing this 👏

[abendt]

Ok, but to avoid duplicate work, do you want to work on it or should I.

i would likely do it in a couple of days. if you want to do it before go ahead