Skip to content

Commit

Permalink
Merge pull request #527 from ollyw/improve-property-test-coverage
Browse files Browse the repository at this point in the history
Improve determinism/coverage in Scalacheck tests
  • Loading branch information
Baccata authored Jun 29, 2022
2 parents 9d165c5 + dfcbf45 commit eb7a469
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 16 deletions.
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ lazy val scalacheck = projectMatrix
testFrameworks := Seq(new TestFramework("weaver.framework.CatsEffect")),
libraryDependencies ++= Seq(
"org.scalacheck" %%% "scalacheck" % Version.scalacheck
)
) ++ Seq("org.typelevel" %%% "cats-effect-testkit" % Version.CE3.cats % Test).filter(_ => virtualAxes.value.contains(CatsEffect3Axis))
)

lazy val specs2 = projectMatrix
Expand Down
41 changes: 26 additions & 15 deletions modules/scalacheck/src/weaver/scalacheck/Checkers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import cats.{ Applicative, Defer, Show }
import org.scalacheck.rng.Seed
import org.scalacheck.{ Arbitrary, Gen }

import CECompat.Ref

trait Checkers {
self: EffectSuiteAux =>
import Checkers._
Expand Down Expand Up @@ -104,15 +102,24 @@ trait Checkers {

def apply[A: Show, B: PropF](gen: Gen[A])(f: A => B)(
implicit loc: SourceLocation): F[Expectations] =
Ref[F].of(Status.start[A]).flatMap(forall_(gen, liftProp(f)))
forall_(gen, liftProp(f))

private def forall_[A: Show](gen: Gen[A], f: A => F[Expectations])(
state: Ref[F, Status[A]])(
implicit loc: SourceLocation): F[Expectations] = {
paramStream
.parEvalMapUnordered(config.perPropertyParallelism) {
testOneTupled(gen, state, f)
.parEvalMap(config.perPropertyParallelism) {
testOneTupled(gen, f)
}
.mapAccumulate(Status.start[A]) { case (oldStatus, testResult) =>
val newStatus = testResult match {
case TestResult.Success => oldStatus.addSuccess
case TestResult.Discard => oldStatus.addDiscard
case TestResult.Failure(input, seed, exp) =>
oldStatus.addFailure(input, seed, exp)
}
(newStatus, newStatus)
}
.map(_._1)
.takeWhile(_.shouldContinue, takeFailure = true)
.takeRight(1) // getting the first error (which finishes the stream)
.compile
Expand Down Expand Up @@ -144,27 +151,24 @@ trait Checkers {

private def testOneTupled[T: Show](
gen: Gen[T],
state: Ref[F, Status[T]],
f: T => F[Expectations])(ps: (Gen.Parameters, Seed)) =
testOne(gen, state, f)(ps._1, ps._2)
testOne(gen, f)(ps._1, ps._2)

private def testOne[T: Show](
gen: Gen[T],
state: Ref[F, Status[T]],
f: T => F[Expectations])(
params: Gen.Parameters,
seed: Seed): F[Status[T]] = {
seed: Seed): F[TestResult] = {
Defer[F](self.effect).defer {
gen(params, seed)
.traverse(x => f(x).map(x -> _))
.flatTap { (x: Option[(T, Expectations)]) =>
.map { (x: Option[(T, Expectations)]) =>
x match {
case Some((_, ex)) if ex.run.isValid => state.update(_.addSuccess)
case Some((t, ex)) => state.update(_.addFailure(t.show, seed, ex))
case None => state.update(_.addDiscard)
case Some((_, ex)) if ex.run.isValid => TestResult.Success
case Some((t, ex)) => TestResult.Failure(t.show, seed, ex)
case None => TestResult.Discard
}
}
.productR(state.get)
}
}

Expand Down Expand Up @@ -233,4 +237,11 @@ object Checkers {
}
}

private sealed trait TestResult
private object TestResult {
case object Success extends TestResult
case object Discard extends TestResult
case class Failure(input: String, seed: Seed, exp: Expectations)
extends TestResult
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package weaver
package scalacheck

import java.util.concurrent.TimeUnit
import java.util.concurrent.atomic.AtomicInteger

import scala.concurrent.duration._

import cats.effect.Outcome._
import cats.effect.testkit.TestControl
import cats.effect.{ IO, Ref }
import cats.syntax.all._

import weaver.TestStatus

import org.scalacheck.Gen

object CheckersConcurrencyTest extends SimpleIOSuite {
test("tests should wait for slower tests to succeed before completion") {

val minTests = 10
object CheckersConcurrencyTestNested extends SimpleIOSuite with Checkers {

override def checkConfig: CheckConfig =
super.checkConfig.copy(perPropertyParallelism = minTests * 5,
minimumSuccessful = minTests,
maximumDiscardRatio = 10)

loggedTest("nested") { log =>
val atomicInt = new AtomicInteger(0)
forall(Gen.delay(Gen.const(atomicInt.incrementAndGet()))) { i =>
val sleepFor =
Duration.create(math.max(0L, minTests - i.toLong), TimeUnit.SECONDS)
IO.sleep(sleepFor) *> log.info(s"Ran gen $i").as(success)
}
}
}

val expectedLogs = (1 to minTests).map(i => s"Ran gen $i").toList
TestControl.execute(runSuite(CheckersConcurrencyTestNested))
.flatMap { control =>
control.advanceAndTick(1.second).replicateA(minTests) *> control.results
}
.map {
case Some(Succeeded(r)) =>
expect(r.count(_.status == TestStatus.Success) == 1) and
expect(r.forall(_.status == TestStatus.Success)) and
expect.same(
expectedLogs,
expectedLogs.intersect(r.flatMap(_.log.toList.map(_.msg)))
)
case Some(Errored(e)) => failure(e.toString)
case Some(Canceled()) => failure("property test was cancelled")
case None => failure("property test didn't complete")
}
}

test("tests should wait for slower tests to fail before completion") {

object CheckersConcurrencyTestNested extends SimpleIOSuite with Checkers {

override def checkConfig: CheckConfig =
super.checkConfig.copy(perPropertyParallelism = 50,
minimumSuccessful = 10,
maximumDiscardRatio = 10)

test("nested") {
val atomicInt = new AtomicInteger(0)
forall(Gen.delay(Gen.const(atomicInt.incrementAndGet()))) { i =>
if (i == 1) IO.sleep(1.second).as(failure("first one fails"))
else IO.pure(success)
}
}
}

TestControl.execute(runSuite(CheckersConcurrencyTestNested))
.flatMap { control =>
control.tick *> control.advanceAndTick(1.second) *> control.results
}
.map {
case Some(Succeeded(r)) =>
expect(r.count(_.status == TestStatus.Failure) == 1) and
expect(r.forall(_.status == TestStatus.Failure))
case Some(Errored(e)) => failure(e.toString)
case Some(Canceled()) => failure("property test was cancelled")
case None => failure("property test didn't complete")
}
}

private def runSuite(suite: SimpleIOSuite) = for {
ref <- Ref.of(List.empty[TestOutcome])
_ <- suite.run(Nil)(result =>
ref.update(result :: _))
results <- ref.get
} yield results
}

0 comments on commit eb7a469

Please sign in to comment.