typelevel / cats-effect

The pure asynchronous runtime for Scala
https://typelevel.org/cats-effect/
Apache License 2.0
2k stars 513 forks source link

Laws testing with Arbitrary[IO[A]] from TestInstances #2778

Open majk-p opened 2 years ago

majk-p commented 2 years ago

I've been recently playing around with migration to CE3. Some of my libraries provide algebras that are law tested. Some of them are meant to be used against IO, thus the laws were also tested against it. In CE2 I would use cats.effect.laws.discipline.arbitrary._ and as discussed on Discord the right replacement would be to have my test suite extend TestInstances and instantiate implicit Ticker().

With this approach my migrated tests wouldn't work out well, but I've learned that Parallel is a tricky one to law test, so I've eliminated that one from my tests.

Despite that effort, I still face issues with law testing quite simple algebras' properties. I've prepared a repository reproducing my issues: https://github.com/majk-p/io-laws-playground/

For the demo I've created two simple algebras, here are the simplified descriptions

trait Storage[F[_], A] {
  def store(a: A): F[Unit]
}

and

trait Consumer[F[_], +A] {
  def consume(f: A => F[Unit]): F[Unit]
}

Say I want to test that the laws of Monoid hold for those two. I've prepared a test suite in: https://github.com/majk-p/io-laws-playground/blob/master/src/test/scala/net/michalp/StorageLaws.scala

While the tests in DummyStorageLaws (that use IO.pure as ArbitraryIO implementation) and SyncIOStorageLaws (that use TestInstances provided Arbitrary[SyncIO[A]]) or even just OptionLawSpec work perfectly well, implementation based on Arbitrary[IO[A]] from TestInstances fails. The error is more or less like the one below for most tests

[info] IOStorageLaws                                                                                     
[info] monoid laws must hold for Monoid[Storage[IO, Int]]                                  
[error] ! monoid.associative                                                                             
[error]  java.lang.UnsupportedOperationException: Exception raised on property evaluation.> ARG_0: <funct
ion1>                                                                                                    
[error]  > ARG_1: <function1>                                                                            
[error]  > ARG_2: <function1>> Exception: java.lang.UnsupportedOperationException: empty.min             
[error]  The seed is 2ZuVq_ssmSpdX6GM8VKNsiFWtPudTuIZT-WgWQIafPP= (TestContext.scala:98)                 
[error] cats.effect.kernel.testkit.TestContext.nextInterval(TestContext.scala:98)                        
[error] cats.effect.kernel.testkit.TestContext.tickAll(TestContext.scala:179)                            
[error] cats.effect.testkit.TestInstances.unsafeRun(TestInstances.scala:193)                             
[error] cats.effect.testkit.TestInstances.unsafeRun$(TestInstances.scala:183)                            
[error] net.michalp.IOStorageLaws.unsafeRun(StorageLaws.scala:23)                                  
[error] cats.effect.testkit.TestInstances.$anonfun$eqIOA$1(TestInstances.scala:145)
[error] cats.kernel.Eq$$anon$2.eqv(Eq.scala:71)
[error] cats.laws.discipline.eq$.$anonfun$catsLawsEqForFn1Exhaustive$2(Eq.scala:17)
[error] cats.laws.discipline.eq$.$anonfun$catsLawsEqForFn1Exhaustive$2$adapted(Eq.scala:17)
[error] cats.laws.discipline.eq$.$anonfun$catsLawsEqForFn1Exhaustive$1(Eq.scala:17)
[error] cats.laws.discipline.eq$.$anonfun$catsLawsEqForFn1Exhaustive$1$adapted(Eq.scala:17)
[error] cats.kernel.Eq$$anon$5.eqv(Eq.scala:97)
[error] cats.kernel.laws.discipline.package$.catsLawsIsEqToProp(package.scala:11)
[error] cats.kernel.laws.discipline.SemigroupTests.$anonfun$semigroup$2(SemigroupTests.scala:18)
[error] org.scalacheck.Prop$.$anonfun$forAllShrink$2(Prop.scala:768)
[error] org.scalacheck.Prop$.secure(Prop.scala:478)
[error] org.scalacheck.Prop$.result$1(Prop.scala:768)
[error] org.scalacheck.Prop$.$anonfun$forAllShrink$1(Prop.scala:807)
[error] org.scalacheck.Prop$.$anonfun$apply$1(Prop.scala:308)
[error] org.scalacheck.PropFromFun.apply(Prop.scala:21)
[error] org.scalacheck.Prop$.result$1(Prop.scala:769)
[error] org.scalacheck.Prop$.$anonfun$forAllShrink$1(Prop.scala:807)
[error] org.scalacheck.Prop$.$anonfun$apply$1(Prop.scala:308)
[error] org.scalacheck.PropFromFun.apply(Prop.scala:21)
[error] org.scalacheck.Prop$.result$1(Prop.scala:769)
[error] org.scalacheck.Prop$.$anonfun$forAllShrink$1(Prop.scala:807)
[error] org.scalacheck.Prop$.$anonfun$apply$1(Prop.scala:308)
[error] org.scalacheck.PropFromFun.apply(Prop.scala:21)
[error] org.scalacheck.Prop$.$anonfun$delay$1(Prop.scala:483)
[error] org.scalacheck.Prop$.$anonfun$apply$1(Prop.scala:308)
[error] org.scalacheck.PropFromFun.apply(Prop.scala:21)
[error] org.scalacheck.Prop.$anonfun$viewSeed$1(Prop.scala:40)
[error] org.scalacheck.Prop$.$anonfun$apply$1(Prop.scala:308)
[error] org.scalacheck.PropFromFun.apply(Prop.scala:21)
[error] org.scalacheck.Prop$.$anonfun$apply$1(Prop.scala:308)
[error] org.scalacheck.PropFromFun.apply(Prop.scala:21)
[error] org.scalacheck.Test$.workerFun$1(Test.scala:434)
[error] org.scalacheck.Test$.$anonfun$check$2(Test.scala:466)
[error] org.scalacheck.Test$.$anonfun$check$2$adapted(Test.scala:466)
[error] org.scalacheck.Platform$.runWorkers(Platform.scala:40)
[error] org.scalacheck.Test$.check(Test.scala:466)
[error] net.michalp.IOStorageLaws.check(StorageLaws.scala:23)
[error] org.typelevel.discipline.specs2.mutable.Discipline.$anonfun$checkAll$2(Discipline.scala:37)

Please advise if I'm doing something wrong when using library provided arbitraryIO, or is it really a bug?

djspiewak commented 2 years ago

This is related to #2615, which notes that nextInterval raises an exception in some cases. At the very least, fixing that should help narrow down what's going on here.

majk-p commented 2 years ago

Thank you! This indeed seems related. On my fork I've modified nextInterval slightly:

  def nextInterval(): FiniteDuration = {
    val s = state
    if (s.tasks.nonEmpty) {
      val task = s.tasks.min
      require(
        (task.runsAt - s.clock)> Duration.Zero,
        s"Calculated negative next interval step. Min task: ${task} runs at ${task.runsAt}, clock: ${s.clock}, diff: ${(task.runsAt - s.clock)}"
      )
      task.runsAt - s.clock
    } else Duration.Zero
  }

Now I end up with this kind of error:

[error] ! monoid.intercalateRepeat2                                                                      
[error]  java.lang.IllegalArgumentException: Exception raised on property evaluation.> ARG_0: <function1>
[error]  > ARG_1: <function1>> Exception: java.lang.IllegalArgumentException: requirement failed: Calculated negative next interval step. Min task: Task(197,cats.effect.IOFiber$$Lambda$9792/0x00000008428a9840@6a551b6a,260016048000 microseconds) runs at 260016048000 microseconds, clock: 260016048047968 nanoseconds, diff: -47968 nanoseconds                                                                                
[error]  The seed is 8wotMV45FnXThYD8IEOgfuwFiSh_MxkTujPrb4xpPDL= (TestContext.scala:101)
[error] cats.effect.kernel.testkit.TestContext.nextInterval(TestContext.scala:101) 
[error] cats.effect.kernel.testkit.TestContext.tickAll(TestContext.scala:184)                            
[error] cats.effect.testkit.TestInstances.unsafeRun(TestInstances.scala:193)                             
[error] cats.effect.testkit.TestInstances.unsafeRun$(TestInstances.scala:183)                      
[error] net.michalp.IOConsumerLawsSpec.unsafeRun(ConsumerLawsSpec.scala:23)            

Looks like the runtime would sometimes face tasks that should be finished in the past already. Obviously advance allows only positive duration (perhaps it could accept zero?). Should we in this case just return zero duration? Or perhaps it's a bigger issue?