Skip to content

Commit

Permalink
Move nesting checks in common module
Browse files Browse the repository at this point in the history
  • Loading branch information
SystemFw committed Dec 4, 2021
1 parent d4b7979 commit bc5e9b6
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 118 deletions.
60 changes: 1 addition & 59 deletions ce2/shared/src/main/scala/munit/CatsEffectSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
package munit

import cats.effect.{ContextShift, IO, SyncIO, Timer}
import cats.syntax.all._
import scala.concurrent.{Future, ExecutionContext}
import munit.internal.NestingChecks.{checkNestingIO, checkNestingSyncIO}

abstract class CatsEffectSuite
extends FunSuite
Expand Down Expand Up @@ -47,64 +47,6 @@ abstract class CatsEffectSuite
"SyncIO",
{ case e: SyncIO[_] => Future(checkNestingSyncIO(e).unsafeRunSync())(munitExecutionContext) }
)

// MUnit works by automatically chaining value transforms of shape `Any => Future[Any]`,
// and we rely on this behavior to chain our `IO ~> Future` transform into the rest of MUnit.
//
// Unfortunately, this has an unforeseen consequence in CatsEffectSuite:
// if you return `IO[IO[A]]` by accident, for example by using `map` instead of `flatMap`,
// MUnit will execute both the inner and outer `IO` by applying our `IO` transform twice.
//
// This breaks the `IO` mental model, and can lead to very surprising behaviour, e.g see:
// https://github.com/typelevel/munit-cats-effect/issues/159.
//
// This method checks for such a case, and fails the test with an actionable message.
private def checkNestingIO(fa: IO[_]): IO[Any] = {
def err(msg: String) = IO.raiseError[Any](new Exception(msg))

fa.flatMap {
case _: IO[_] =>
err(
"your test returns an `IO[IO[_]]`, which means the inner `IO` will not execute." ++
" Call `.flatten` if you want it to execute, or `.void` if you want to discard it"
)
case _: SyncIO[_] =>
err(
"your test returns an `IO[SyncIO[_]]`, which means the inner `SyncIO` will not execute." ++
" Call `.flatMap(_.to[IO]) if you want it to execute, or `.void` if you want to discard it"
)
case _: Future[_] =>
err(
"your test returns an `IO[Future[_]]`, which means the inner `Future` might not execute." ++
" Surround it with `IO.fromFuture` if you want it to execute, or call `.void` if you want to discard it"
)
case v => v.pure[IO]
}
}

// same as above, but for SyncIO
private def checkNestingSyncIO(fa: SyncIO[_]): SyncIO[Any] = {
def err(msg: String) = SyncIO.raiseError[Any](new Exception(msg))

fa.flatMap {
case _: IO[_] =>
err(
"your test returns a `SyncIO[IO[_]]`, which means the inner `IO` will not execute." ++
" Call `.to[IO].flatten` if you want it to execute, or `.void` if you want to discard it"
)
case _: SyncIO[_] =>
err(
"your test returns a `SyncIO[SyncIO[_]]`, which means the inner `SyncIO` will not execute." ++
" Call `.flatten` if you want it to execute, or `.void` if you want to discard it"
)
case _: Future[_] =>
err(
"your test returns a `SyncIO[Future[_]]`, which means the inner `Future` might not execute." ++
" Change it to `IO.fromFuture(yourTest.to[IO])` if you want it to execute, or call `.void` if you want to discard it"
)
case v => v.pure[SyncIO]
}
}
}

object CatsEffectSuite {
Expand Down
60 changes: 1 addition & 59 deletions ce3/shared/src/main/scala/munit/CatsEffectSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ package munit

import cats.effect.unsafe.IORuntime
import cats.effect.{IO, SyncIO}
import cats.syntax.all._

import scala.concurrent.{ExecutionContext, Future}
import munit.internal.NestingChecks.{checkNestingIO, checkNestingSyncIO}

abstract class CatsEffectSuite
extends FunSuite
Expand All @@ -47,64 +47,6 @@ abstract class CatsEffectSuite
"SyncIO",
{ case e: SyncIO[_] => Future(checkNestingSyncIO(e).unsafeRunSync())(munitExecutionContext) }
)

// MUnit works by automatically chaining value transforms of shape `Any => Future[Any]`,
// and we rely on this behavior to chain our `IO ~> Future` transform into the rest of MUnit.
//
// Unfortunately, this has an unforeseen consequence in CatsEffectSuite:
// if you return `IO[IO[A]]` by accident, for example by using `map` instead of `flatMap`,
// MUnit will execute both the inner and outer `IO` by applying our `IO` transform twice.
//
// This breaks the `IO` mental model, and can lead to very surprising behaviour, e.g see:
// https://github.com/typelevel/munit-cats-effect/issues/159.
//
// This method checks for such a case, and fails the test with an actionable message.
private def checkNestingIO(fa: IO[_]): IO[Any] = {
def err(msg: String) = IO.raiseError[Any](new Exception(msg))

fa.flatMap {
case _: IO[_] =>
err(
"your test returns an `IO[IO[_]]`, which means the inner `IO` will not execute." ++
" Call `.flatten` if you want it to execute, or `.void` if you want to discard it"
)
case _: SyncIO[_] =>
err(
"your test returns an `IO[SyncIO[_]]`, which means the inner `SyncIO` will not execute." ++
" Call `.flatMap(_.to[IO]) if you want it to execute, or `.void` if you want to discard it"
)
case _: Future[_] =>
err(
"your test returns an `IO[Future[_]]`, which means the inner `Future` might not execute." ++
" Surround it with `IO.fromFuture` if you want it to execute, or call `.void` if you want to discard it"
)
case v => v.pure[IO]
}
}

// same as above, but for SyncIO
private def checkNestingSyncIO(fa: SyncIO[_]): SyncIO[Any] = {
def err(msg: String) = SyncIO.raiseError[Any](new Exception(msg))

fa.flatMap {
case _: IO[_] =>
err(
"your test returns a `SyncIO[IO[_]]`, which means the inner `IO` will not execute." ++
" Call `.to[IO].flatten` if you want it to execute, or `.void` if you want to discard it"
)
case _: SyncIO[_] =>
err(
"your test returns a `SyncIO[SyncIO[_]]`, which means the inner `SyncIO` will not execute." ++
" Call `.flatten` if you want it to execute, or `.void` if you want to discard it"
)
case _: Future[_] =>
err(
"your test returns a `SyncIO[Future[_]]`, which means the inner `Future` might not execute." ++
" Change it to `IO.fromFuture(yourTest.to[IO])` if you want it to execute, or call `.void` if you want to discard it"
)
case v => v.pure[SyncIO]
}
}
}

object CatsEffectSuite {
Expand Down
81 changes: 81 additions & 0 deletions common/shared/src/main/scala/munit/internal.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* Copyright 2021 Typelevel
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package munit.internal

import cats.effect.{IO, SyncIO}
import cats.syntax.all._
import scala.concurrent.Future

private[munit] object NestingChecks {
// MUnit works by automatically chaining value transforms of shape `Any => Future[Any]`,
// and we rely on this behavior to chain our `IO ~> Future` transform into the rest of MUnit.
//
// Unfortunately, this has an unforeseen consequence in CatsEffectSuite:
// if you return `IO[IO[A]]` by accident, for example by using `map` instead of `flatMap`,
// MUnit will execute both the inner and outer `IO` by applying our `IO` transform twice.
//
// This breaks the `IO` mental model, and can lead to very surprising behaviour, e.g see:
// https://github.com/typelevel/munit-cats-effect/issues/159.
//
// This method checks for such a case, and fails the test with an actionable message.
def checkNestingIO(fa: IO[_]): IO[Any] = {
def err(msg: String) = IO.raiseError[Any](new Exception(msg))

fa.flatMap {
case _: IO[_] =>
err(
"your test returns an `IO[IO[_]]`, which means the inner `IO` will not execute." ++
" Call `.flatten` if you want it to execute, or `.void` if you want to discard it"
)
case _: SyncIO[_] =>
err(
"your test returns an `IO[SyncIO[_]]`, which means the inner `SyncIO` will not execute." ++
" Call `.flatMap(_.to[IO]) if you want it to execute, or `.void` if you want to discard it"
)
case _: Future[_] =>
err(
"your test returns an `IO[Future[_]]`, which means the inner `Future` might not execute." ++
" Surround it with `IO.fromFuture` if you want it to execute, or call `.void` if you want to discard it"
)
case v => v.pure[IO]
}
}

// same as above, but for SyncIO
def checkNestingSyncIO(fa: SyncIO[_]): SyncIO[Any] = {
def err(msg: String) = SyncIO.raiseError[Any](new Exception(msg))

fa.flatMap {
case _: IO[_] =>
err(
"your test returns a `SyncIO[IO[_]]`, which means the inner `IO` will not execute." ++
" Call `.to[IO].flatten` if you want it to execute, or `.void` if you want to discard it"
)
case _: SyncIO[_] =>
err(
"your test returns a `SyncIO[SyncIO[_]]`, which means the inner `SyncIO` will not execute." ++
" Call `.flatten` if you want it to execute, or `.void` if you want to discard it"
)
case _: Future[_] =>
err(
"your test returns a `SyncIO[Future[_]]`, which means the inner `Future` might not execute." ++
" Change it to `IO.fromFuture(yourTest.to[IO])` if you want it to execute, or call `.void` if you want to discard it"
)
case v => v.pure[SyncIO]
}
}
}

0 comments on commit bc5e9b6

Please sign in to comment.