typelevel / munit-cats-effect

Integration library for MUnit & cats-effect
Apache License 2.0
149 stars 34 forks source link

Add 'strict' Suite #32

Open hugo-vrijswijk opened 3 years ago

hugo-vrijswijk commented 3 years ago

MUnit allows overriding a TestValue to a value to enforce a test always returns that type. In the example it is used for Future.

Due to the lazy evaluation of IO I think it'd be very useful if I could enforce my test would always return an IO to prevent code from never being run. ScalaTest has recently changed their suites to be AnySpec (can return Any) or AsyncSpec (must return a Future). munit-cats-effect could introduce something similar for returned IO's by overriding MUnit's TestValue.

If added to munits-cats-effect, users should probably still have a choice (just like in ScalaTest) between the two styles. And maybe 'strict' should not be the default? I don't have a strong opinion on what the naming should be.

For implementation some refactoring would probably be needed. TestType is made final in munit.FunSuite so the new suite would have to extend from munit.Suite. Code sharing between the two styles is probably also wanted. I'd be interested in helping out with a PR if this is wanted

mpilquist commented 3 years ago

@hugo-vrijswijk This sounds good to me. I do worry a bit about providing too many options. One of the things I really like about munit is that there aren't 25 style traits. Would be great to hear from others as to whether they'd use this feature. Maybe a proof of concept PR to get started?

hugo-vrijswijk commented 3 years ago

Yeah I'd be fine with starting on something.

I definitely agree the perceived simplicity of munit is very nice to work with. Perhaps this feature would also be possible while keeping a single spec trait somehow?

hugo-vrijswijk commented 3 years ago

I spend a little bit of time on this, and didn't get very far. The problem is that munit has defined test in FunSuite with the Any signature, without any customizable type as I first thought. And because all the other related code like ValueTransforms have a this: FunSuite => assertion it's not possible to reuse any of that logic from munit. So outside of copy-pasting half of munit there's not a real solution other than to add a customizable type to munit itself.

An alternative would be to add these two functions:

def testIO(name: String)(body: => IO[Any])(implicit loc: Location): Unit = {
  test(name)(body)
}

def testIO(options: TestOptions)(body: => IO[Any])(implicit loc: Location): Unit = {
  test(options)(body)
}

Upside being that it's very simple and all existing code is not touched. Downside is that def testIO(name: String)(body: => Any) is still there and easy to mis-use if the intention is to enforce a return type.

What do you think? Would the above two functions add value? I could also go knocking on munit's door to change the Any type to a parameter