Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ForEachTest resource #331

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ForEachTest resource #331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions modules/core/cats/src/weaver/Suites.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ abstract class MutableIOSuite
with BaseIOSuite
with Expectations.Helpers

abstract class MutableForEachIOSuite extends MutableForEachSuite[IO]
with BaseIOSuite
with Expectations.Helpers

abstract class SimpleMutableIOSuite extends MutableIOSuite {
type Res = Unit
def sharedResource: Resource[IO, Unit] = Resource.pure[IO, Unit](())
Expand Down
1 change: 1 addition & 0 deletions modules/core/cats/src/weaver/package.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import cats.effect.IO
package object weaver {

type IOSuite = MutableIOSuite
type IOForEachSuite = MutableForEachIOSuite
type SimpleIOSuite = SimpleMutableIOSuite
type GlobalResource = IOGlobalResource
type GlobalRead = GlobalResourceF.Read[IO]
Expand Down
65 changes: 45 additions & 20 deletions modules/core/src/weaver/suites.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,9 @@ abstract class RunnableSuite[F[_]] extends EffectSuite[F] {
effectCompat.sync(run(args)(outcome => effectCompat.effect.delay(report(outcome))))
}

abstract class MutableFSuite[F[_]] extends RunnableSuite[F] {
abstract class MutableBaseFSuite[F[_]] extends RunnableSuite[F] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so I think you're on the right track, however what I'd like to see is this :

trait MutableBaseFSuite[F[_]] {
    type Res 
    // PerTestRes would be replace Res in the "test" methods 
    type PerTestRes 

    def sharedResource: Resource[F, Res] 
    def perTestResource(res: Res) : Resource[F, PerTestRes] 
}

and then MutableFSuite would become :

trait MutableFSuite[F[_]] extends MutableBaseFSuite[F] {
   type PerTestRes = Res
   final def perTestResource(res: Res) : Resource[F, Res] = Resource.pure(res)  
}

The reason for doing it this way is that :

  1. it works for your usecase
  2. people can still share resources a little bit more granularly than your current version, which is "shared XOR per test", whereas this version would be "shared OR per test"
  3. it works on the javascript backend where cross-suite sharing is not possible, but per suite sharing is possible
  4. You can still have your MutableForEachSuite in userland :
trait MutableForEachSuite[F[_]] extends MutableBaseFSuite[F] {
   type Res = Unit 
   def uniqueResource : Resource[F, PerTestRes]
   final def perTestResource(res: Unit) : Resource[F, PerTestRes] = uniqueResource
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amumurst, just to touch base : what do you think about the suggestion above ?


type Res
def sharedResource : Resource[F, Res]

def maxParallelism : Int = 10000

Expand All @@ -81,37 +80,63 @@ abstract class MutableFSuite[F[_]] extends RunnableSuite[F] {
def apply(run : (Res, Log[F]) => F[Expectations]) : Unit = registerTest(name)(res => Test(name.name, log => run(res, log)))
}

private[this] var testSeq = Seq.empty[(TestName, Res => F[TestOutcome])]

def plan: List[TestName] = testSeq.map(_._1).toList

private[this] var isInitialized = false

private[this] def initError() =
new AssertionError(
"Cannot define new tests after TestSuite was initialized"
)


private[weaver] def getTests(args: List[String]) = {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuff is just abstracted back up to the common base trait between forEach and standard suite, no changes made.

if (!isInitialized) isInitialized = true
val argsFilter = Filters.filterTests(this.name)(args)
val filteredTests = if (testSeq.exists(_._1.tags(TestName.Tags.only))){
testSeq.filter(_._1.tags(TestName.Tags.only)).map { case (_, test) => (res: Res) => test(res)}
} else testSeq.collect {
case (name, test) if argsFilter(name) => (res : Res) => test(res)
}
val parallism = math.max(1, maxParallelism)

(filteredTests, parallism)
}
}

abstract class MutableFSuite[F[_]] extends MutableBaseFSuite[F]{
def sharedResource : Resource[F, Res]
override def spec(args: List[String]) : Stream[F, TestOutcome] =
synchronized {
if (!isInitialized) isInitialized = true
val argsFilter = Filters.filterTests(this.name)(args)
val filteredTests = if (testSeq.exists(_._1.tags(TestName.Tags.only))){
testSeq.filter(_._1.tags(TestName.Tags.only)).map { case (_, test) => (res: Res) => test(res)}
} else testSeq.collect {
case (name, test) if argsFilter(name) => (res : Res) => test(res)
}
val parallism = math.max(1, maxParallelism)
val (filteredTests, parallism) = getTests(args)
if (filteredTests.isEmpty) Stream.empty // no need to allocate resources
else for {
resource <- Stream.resource(sharedResource)
tests = filteredTests.map(_.apply(resource))
testStream = Stream.emits(tests).lift[F](effectCompat.effect)
result <- if (parallism > 1 ) testStream.parEvalMap(parallism)(identity)(effectCompat.effect)
else testStream.evalMap(identity)
else testStream.evalMap(identity)
} yield result
}
}
abstract class MutableForEachSuite[F[_]] extends MutableBaseFSuite[F]{
def uniqueResource : Resource[F, Res]

private[this] var testSeq = Seq.empty[(TestName, Res => F[TestOutcome])]

def plan: List[TestName] = testSeq.map(_._1).toList

private[this] var isInitialized = false
override def spec(args: List[String]) : Stream[F, TestOutcome] =
synchronized {
val (filteredTests, parallism) = getTests(args)

private[this] def initError() =
new AssertionError(
"Cannot define new tests after TestSuite was initialized"
)
if (filteredTests.isEmpty) Stream.empty // no need to allocate resources
else {
val testStream = Stream.emits(filteredTests).lift[F](effectCompat.effect)

if (parallism > 1 )
testStream.parEvalMap(parallism)(test => uniqueResource.use(test(_)))(effectCompat.effect)
else testStream.evalMap(test => uniqueResource.use(test(_)))
}
}
}

trait FunSuiteAux {
Expand Down
44 changes: 43 additions & 1 deletion modules/framework/cats/test/src/Global.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,19 @@ package weaver
package framework
package test

import cats.effect.{ IO, Resource }
import cats.effect.{ IO, Resource}

object SharedResources extends IOGlobalResource {
class RefFactory(init: Int){
val generate: Resource[IO, CECompat.Ref[IO, Int]] = Resource.eval(CECompat.Ref.of[IO, Int](init))
}

def sharedResources(global: GlobalResourceF.Write[IO]): Resource[IO, Unit] =
for {
foo <- Resource.pure[IO, String]("hello world!")
bar <- Resource.pure[IO, RefFactory](new RefFactory(0))
_ <- global.putR(foo)
_ <- global.putR(bar)
} yield ()
}

Expand All @@ -34,3 +40,39 @@ class OtherResourceSharingSuite(globalResources: GlobalResourceF.Read[IO])
}

}

class UniqueResourceSuite(global: GlobalRead) extends IOForEachSuite{
type Res = CECompat.Ref[IO, Int]
def uniqueResource = global.getOrFailR[SharedResources.RefFactory]().flatMap(_.generate)

override val maxParallelism = 1

test("start value is set to 0") { refIO =>
refIO.get.map(i => expect(i == 0))
}
test("value is updated locally to 1") { refIO=>
refIO.update(_ + 1) *>
refIO.get.map(i => expect(i == 1))
}
test("value is still 0 in another test") { refIO=>
refIO.get.map(i => expect(i == 0))
}
}

class SharedResourceSuite(global: GlobalRead) extends IOSuite{
type Res = CECompat.Ref[IO, Int]
def sharedResource = global.getOrFailR[SharedResources.RefFactory]().flatMap(_.generate)

override val maxParallelism = 1

test("start value is set to 0") { refIO =>
refIO.get.map(i => expect(i == 0))
}
test("value is updated in whole suit to 1") { refIO=>
refIO.update(_ + 1) *>
refIO.get.map(i => expect(i == 1))
}
test("value is changed to 1 in another test") { refIO=>
refIO.get.map(i => expect(i == 1))
}
}