typelevel / cats-effect-testing Goto Github PK

Info:

cats-effect-testing-scalatest" % "1.4.0" 
openjdk 11.0.12

Given following example:

import cats.effect._
import cats.effect.testing.scalatest.{AsyncIOSpec, CatsResourceIO}
import org.scalatest.freespec.FixtureAsyncFreeSpec

class TestExample extends FixtureAsyncFreeSpec with AsyncIOSpec with CatsResourceIO[Int] {
  val resource: Resource[IO, Int] = Resource.make(IO.println("acquired").as(1))(_ => IO.println("released"))

  "should close resource" in { outer =>
    val inner = Resource.make(IO.println("acquired inner"))(_ => IO.println("released inner"))
    inner
      .use { _ =>
        IO.println(outer).as(succeed)
      }
  }
}

I would expect scalatest to release the fixture resource. Instead the resource is not being released.

I think that there are two problems with the current solution.

First, we schedule the shutdown action here: https://github.com/typelevel/cats-effect-testing/blob/series/1.x/scalatest/shared/src/main/scala/cats/effect/testing/scalatest/CatsResource.scala#L76 which is asynchronous and then we immediately clear the shutdown variable. This clear action has a potential to be executed before the scheduled future is run resulting in the release code not being evaluated.

However, only rewriting that to something like:

  override def afterAll(): Unit = {
    UnsafeRun[F].unsafeToFuture(
      shutdown >> Sync[F] delay {
        gate = None
        value = None
        shutdown = ().pure[F]
      },
      finiteResourceTimeout
    )
  }

Doesn't fix the problem.

The second part is somehow related to the execution context used by scalatest.

I didn't dig deep enough into the scalatest codebase to prove that but it seems to me that the execution context gets closed(?) as soon as the test suite completes.

Changing the EC used to evaluate future actions together with previous solution fixes the problem.

  private lazy val _ResourceUnsafeRun =
    new UnsafeRun[IO] {
      private implicit val runtime: IORuntime = createIORuntime(ExecutionContext.global) //here using global instead of inherited exectionContext

      override def unsafeToFuture[B](ioa: IO[B]): Future[B] =
        unsafeToFuture(ioa, None)

      override def unsafeToFuture[B](ioa: IO[B], timeout: Option[FiniteDuration]): Future[B] =
        timeout.fold(ioa)(ioa.timeout).unsafeToFuture()
    }

I am not saying that this is the correct solution, I am only including that to give more information about the underlying issue.

While trying to understand that issue I also came across these two issues which I think might be relevant:

• scalatest/scalatest#953 - TLDR: there is no beforeAllAsync and afterAllAsync for now
• scalatest/scalatest#1828 - TLDR: you should not block within before and after methods as default ExecutionContext is single threaded

I couldn't find any links to a gitter or other question space so I figured I'd create an issue for my question.

ScalaTest provides its own ExecutionContext for AsyncTestSuites (explained here under 'Asynchronous execution model') which is a serial execution context. The relevant reason for this is:

asynchronous-style tests need not be complete when the test body returns, because the test body returns a Future[Assertion]. This Future[Assertion] will often represent a test that has not yet completed. As a result, when using a more traditional execution context backed by a thread-pool, you could potentially start many more tests executing concurrently than there are threads in the thread pool. The more concurrently execute tests you have competing for threads from the same limited thread pool, the more likely it will be that tests will intermitently fail due to timeouts.

In this library the ExecutionContext is overridden to the Scala global EC. What is the reasoning behind this? I can imagine there is a very good reason, but maybe there should be a comment explaining why?

The reason I am asking is because the global EC is causing issues for me in combination with mockito-scala and its AsyncMockitoSugar which resets the mockito session after each test. Because the global EC will start tests in the same suite in parallel mockito-scala thinks that a previous test has not properly closed mocking, which will throw an exception and fail a bunch of tests semi-randomly. This is fixed when I make my own AsyncIOSpec which does not override the EC set by ScalaTest

Due to lack of a name setting in SBT. Not sure what the best fix is, or defer to the next major version bump?

cats-effect 3.x is coming.
This library would need some changes to support it.
For example: https://github.com/http4s/http4s/blob/52caa170f4c4859050f7876492a112c6f40c2856/testing/src/main/scala/org/http4s/testing/CatsEffect.scala

I don't know if there is a plan how to move to cats-effect 3.x.
I'd be happy to contribute those changes.
Should I open a PR?

In order to use the ioRetrying defined below, we need a concrete IO. I think it would better for this to take a generic effect type (even though it would use IO in the base trait), so that helper traits don't need to be tied to IO.

It's currently in the RC stage, would be awesome to start testing it:
https://github.com/etorreborre/specs2/releases

Note that it's Scala 3 only. See also typelevel/discipline-specs2#192.

Right now working with .asserting when requiring multiple assertions over the same value is not very straightforward, requiring users to combine multiple assertions into one.

Would be nice if this lib had some mechanism for handling this.

The snapshot publish job failed because there are currently no sonatype credentials installed as secrets on this repo.

  env:
    PGP_PASSPHRASE: 
    SONATYPE_PASSWORD: 
    SONATYPE_USERNAME: 
    PGP_SECRET: ***
    GITHUB_TOKEN: ***
    JAVA_HOME: /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/8.0.312-7/x64

https://github.com/typelevel/cats-effect-testing/runs/5107888927

Linking to typelevel/governance#23 which will fix this.

This library uses IORuntime.global for tests, which causes it to never close the threadpools that are created by this global IORuntime. Combined with sbt creating a new classloader for every test run and every submodule, this results in a lot of IORuntimes and threadpools being created and never cleaned up. Eventually, it causes sbt to run of out memory after a number of test runs, especially in projects with many submodules.

I'm not sure what the best approach would be to avoid this issue, as creating a new IORuntime in every test suite will probably slow down running tests?

Just glancing at the readme:

• Timeout vs timeout
• CatsEffect vs IOSuite

Some of these should try to stay within the idiom of the surrounding test framework, so maybe we're already doing okay on the second point. We should try to unify on the first one, though.

The docs reference a cats-effect-testing-scalatest module, but it looks like the last version of this published was 0.5.4 and I see no reference to it in the build definition.

Was it deleted?

Hi folks! Thanks for the great library.

This issue isn't strictly speaking an issue in cats-effect-testing, but I believe to be an issue with utest. Raising this issue here, just incase anyone else encounters this problem.

Noticed a small ergonomic issue with the current package naming, basically when you do import cats.effect._, the utest package is brought into scope. The assert macro references utest.asserts.Asserts.assertImpl which doesn't exist in cats.effect.utest causing the following error.

object asserts is not a member of package cats.effect.utest

Ammonite Example

@ import $ivy.`com.codecommit::cats-effect-testing-utest:0.2.0`
@ import cats.effect.utest.IOTestSuite
@ import utest._
@ import cats.effect._

@ object FooTests extends IOTestSuite {
    val tests = Tests {
      test("foo") - {
        assert(true)
      }
    }
  }
cmd4.sc:4: object asserts is not a member of package cats.effect.utest
      assert(true)
            ^
Compilation Failed

Workaround

Suppress utest import when importing cats.effect._, i.e.

import cats.effect.{utest => _, _}

Next Steps

Raise a PR for utest to use _root_.utest in all macros, I'll try and do that some point this week.

Hi!, I wonder if cats-effect-testing-scalatest-scalacheck was deprecated, I can see that the README is still referencing it, but there is no EffectCheckerAsserting in the default branch.

Hi!

There is no "org.typelevel" %% "cats-effect-testing-specs2" % "<version>" but there is "com.codecommit" %% "cats-effect-testing-specs2" % "<version>" (com.codecommit instead of org.typelevel)

Hi!

Here's something I've found while doing dependency upgrades:

scalaVersion := "2.13.7"

libraryDependencies ++= Seq(
  "org.typelevel" %% "cats-effect" % "3.2.8",
  "org.specs2" %% "specs2-core" % "4.13.0" %  Test,
  "org.typelevel" %% "cats-effect-testing-specs2" % "1.3.0" % Test,
)

import cats.effect.IO
import cats.effect.testing.specs2.CatsEffect
import org.specs2.mutable.SpecificationLike

class CatsEffectSpecs extends CatsEffect with SpecificationLike {
  "test" should {
    "test" in {
      IO(ok)
    }
  }
}

The above fails with:

Exception in thread "specs2-3" java.lang.NoSuchMethodError: 'org.specs2.specification.core.Execution org.specs2.specification.core.Execution.copy(scala.Option, org.specs2.specification.core.Executing, scala.Option, boolean, scala.Function1, boolean, scala.Option, scala.Option)'
        at cats.effect.testing.specs2.CatsEffect$$anon$1.execute(CatsEffect.scala:40)
        at org.specs2.specification.dsl.mutable.ExampleDsl0$BlockExample0.$greater$greater(ExampleDsl.scala:84)
        at org.specs2.specification.dsl.mutable.ExampleDsl0$BlockExample0.in(ExampleDsl.scala:100)
        at CatsEffectSpecs.$anonfun$new$1(CatsEffectSpecs.scala:7)
        at org.specs2.specification.dsl.mutable.EffectBlocks.tryBlock(EffectBlocks.scala:131)
        at org.specs2.specification.dsl.mutable.EffectBlocks.$anonfun$nestBlock$2(EffectBlocks.scala:110)
        at org.specs2.specification.dsl.mutable.EffectBlocks.record(EffectBlocks.scala:61)
        at org.specs2.specification.dsl.mutable.MutableFragmentBuilder.$anonfun$replayFragments$2(MutableFragmentBuilder.scala:47)
        at scala.Option.getOrElse(Option.scala:201)
        at org.specs2.specification.dsl.mutable.MutableFragmentBuilder.replayFragments(MutableFragmentBuilder.scala:47)
        at org.specs2.specification.dsl.mutable.MutableFragmentBuilder.specificationFragments(MutableFragmentBuilder.scala:37)
        at org.specs2.specification.dsl.mutable.MutableFragmentBuilder.specificationFragments$(MutableFragmentBuilder.scala:36)
        at CatsEffectSpecs.specificationFragments(CatsEffectSpecs.scala:5)
        at org.specs2.specification.dsl.mutable.MutableFragmentBuilder.$anonfun$is$1(MutableFragmentBuilder.scala:44)
        at org.specs2.specification.core.SpecStructure.fragments$lzycompute(SpecStructure.scala:26)
        at org.specs2.specification.core.SpecStructure.fragments(SpecStructure.scala:26)
        at org.specs2.specification.core.SpecStructure.$anonfun$map$1(SpecStructure.scala:29)
        at org.specs2.specification.core.SpecStructure.fragments$lzycompute(SpecStructure.scala:26)
        at org.specs2.specification.core.SpecStructure.fragments(SpecStructure.scala:26)
        at org.specs2.specification.core.SpecStructure.$anonfun$$bar$greater$1(SpecStructure.scala:30)
        at org.specs2.specification.core.SpecStructure.fragments$lzycompute(SpecStructure.scala:26)
        at org.specs2.specification.core.SpecStructure.fragments(SpecStructure.scala:26)
        at org.specs2.specification.core.SpecStructure.$anonfun$$bar$greater$1(SpecStructure.scala:30)
        at org.specs2.specification.core.SpecStructure.fragments$lzycompute(SpecStructure.scala:26)
        at org.specs2.specification.core.SpecStructure.fragments(SpecStructure.scala:26)
        at org.specs2.specification.core.SpecStructure.contents(SpecStructure.scala:28)
        at org.specs2.reporter.Reporter.$anonfun$report$1(Reporter.scala:43)
        at org.specs2.runner.Runner$.runSpecStructure(Runner.scala:109)
        at org.specs2.runner.SbtTask.$anonfun$specificationRun$1(SbtRunner.scala:183)
        at org.specs2.control.eff.Arrs.go$1(Eff.scala:380)
        at org.specs2.control.eff.Arrs.apply(Eff.scala:399)
        at org.specs2.control.eff.Interpret.$anonfun$interpretLoop$11(Interpret.scala:198)
        at org.specs2.control.eff.Arrs.go$1(Eff.scala:380)
        at org.specs2.control.eff.Arrs.apply(Eff.scala:399)
        at org.specs2.control.eff.CollectedUnions.$anonfun$othersEff$1(Unions.scala:97)
        at org.specs2.control.eff.Arrs.go$1(Eff.scala:383)
        at org.specs2.control.eff.Arrs.apply(Eff.scala:399)
        at org.specs2.control.eff.Arrs.apply(Eff.scala:348)
        at org.specs2.control.eff.CollectedUnions.$anonfun$continuation$1(Unions.scala:84)
        at org.specs2.control.eff.Arrs.go$1(Eff.scala:380)
        at org.specs2.control.eff.Arrs.apply(Eff.scala:399)
        at org.specs2.control.eff.Interpret$$anon$1.$anonfun$onEffect$1(Interpret.scala:53)
        at org.specs2.fp.EitherOps$.bimap$extension(EitherSyntax.scala:82)
        at org.specs2.control.eff.Interpret$$anon$1.onEffect(Interpret.scala:53)
        at org.specs2.control.eff.Interpret$$anon$1.onApplicativeEffect(Interpret.scala:61)
        at org.specs2.control.eff.Interpret$$anon$1.onApplicativeEffect(Interpret.scala:45)
        at org.specs2.control.eff.Interpret.go$1(Interpret.scala:200)
        at org.specs2.control.eff.Interpret.interpretLoop(Interpret.scala:207)
        at org.specs2.control.eff.Interpret.interpretLoop$(Interpret.scala:142)
        at org.specs2.control.eff.Interpret$.interpretLoop(Interpret.scala:635)
        at org.specs2.control.eff.Interpret.interpret(Interpret.scala:71)
        at org.specs2.control.eff.Interpret.interpret$(Interpret.scala:44)
        at org.specs2.control.eff.Interpret$.interpret(Interpret.scala:635)
        at org.specs2.control.eff.Interpret.interpret1(Interpret.scala:78)
        at org.specs2.control.eff.Interpret.interpret1$(Interpret.scala:77)
        at org.specs2.control.eff.Interpret$.interpret1(Interpret.scala:635)
        at org.specs2.control.eff.ErrorInterpretation.runError(ErrorEffect.scala:87)
        at org.specs2.control.eff.ErrorInterpretation.runError$(ErrorEffect.scala:68)
        at org.specs2.control.eff.ErrorEffect$.runError(ErrorEffect.scala:187)
        at org.specs2.control.eff.syntax.error$ErrorEffectOps.runError(error.scala:14)
        at org.specs2.control.ExecuteActions.executeActionFuture(ExecuteActions.scala:36)
        at org.specs2.control.ExecuteActions.executeActionFuture$(ExecuteActions.scala:31)
        at org.specs2.control.ExecuteActions$.executeActionFuture(ExecuteActions.scala:93)
        at org.specs2.runner.SbtTask.$anonfun$executeFuture$3(SbtRunner.scala:146)
        at scala.concurrent.impl.Promise$Transformation.run(Promise.scala:470)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
        at java.base/java.lang.Thread.run(Thread.java:829)

A release with the scalatest module will be really helpful. I will be happy to provide any help if needed.

Right now, timeouts skip tests but do not fail them. I would argue that a timing out test is a failure and should be reported as such.

Probably makes sense. Will have to be a bit more careful though.

Can probably leverage the pre-existing Future support in ScalaTest by using unsafeToFuture.

Minimal example: https://scastie.scala-lang.org/NU86Wy4kTTukZzaSTUM2VA
I think this is because AsyncIOSpec passes scalatest's executionContext which is serialExecutionContext and not really suitable for general-purpose use.

At the moment, ScalaTest implementation forces user to use FreeSpec testing style. However, as user, I would like to choose which testing style to use. I would like to suggest to change AsyncIOSpec to

trait AsyncIOSpec extends AssertingSyntax with EffectTestSupport {
  self: AsyncTestSuite =>
.....
}

so user can choose any testing style that implements AsyncTestSuite.

How should tests be structured to best take advantage of this and reflect the capability to users in an idiomatic fashion? Not sure! typelevel/cats-effect#2276 exposes a new TestControl runtime which makes it relatively straightforward to test IO programs under artificial time control.

Related to #145