Merge pull request #112 from zainab-ali/fix-scalacheck-config

Fail property test when discard count exceeds `maximumDiscarded`

modules/scalacheck/shared/src/main/scala/weaver/scalacheck/CheckConfig.scala

@@ -14,12 +14,21 @@ case class CheckConfig private (
   assert(maximumDiscardRatio <= 100)
   assert(minimumSuccessful > 0)
 
+  /**
+   * The maximum number of values that can be discarded by the generator. The
+   * test will fail if the generator discards more values.
+   */
   def maximumDiscarded = minimumSuccessful * maximumDiscardRatio / 100
 
+  /** The number of successful runs required for a test to succeed */
   def withMinimumSuccessful(minimumSuccessful: Int) = copy(
     minimumSuccessful = minimumSuccessful
   )
 
+  /**
+   * The proportion of values discarded by the generator allowed before the test
+   * is considered failed.
+   */
   def withMaximumDiscardRatio(maximumDiscardRatio: Int) = copy(
     maximumDiscardRatio = maximumDiscardRatio
   )

modules/scalacheck/shared/src/main/scala/weaver/scalacheck/Checkers.scala

@@ -176,15 +176,15 @@ trait Checkers {
     def shouldStop(config: CheckConfig) =
       failure.isDefined ||
         succeeded >= config.minimumSuccessful ||
-        discarded >= config.maximumDiscarded
+        discarded > config.maximumDiscarded
 
     def shouldContinue(config: CheckConfig) = !shouldStop(config)
 
     def endResult(config: CheckConfig)(implicit loc: SourceLocation) =
       failure.getOrElse {
         if (succeeded < config.minimumSuccessful)
           Expectations.Helpers.failure(
-            s"Discarded more inputs ($discarded) than allowed")
+            s"Discarded more inputs ($discarded) than allowed (${config.maximumDiscarded})")
         else Expectations.Helpers.success
       }
   }

modules/scalacheck/shared/src/test/scala/weaver/scalacheck/PropertyDogFoodTest.scala

@@ -63,16 +63,23 @@ object PropertyDogFoodTest extends IOSuite {
   }
 
   test("Config can be overridden") { dogfood =>
+    val maximumDiscarded =
+      Meta.ConfigOverrideChecks.configOverride.maximumDiscarded
     expectErrorMessage(
-      s"Discarded more inputs (${Meta.ConfigOverrideChecks.configOverride.maximumDiscarded}) than allowed",
+      s"Discarded more inputs (${maximumDiscarded + 1}) than allowed (${maximumDiscarded})",
       dogfood.runSuite(Meta.ConfigOverrideChecks))
   }
 
   test("Discarded counts should be accurate") { dogfood =>
+    val maximumDiscarded = Meta.DiscardedChecks.checkConfig.maximumDiscarded
     expectErrorMessage(
-      s"Discarded more inputs (${Meta.DiscardedChecks.checkConfig.maximumDiscarded}) than allowed",
+      s"Discarded more inputs (${maximumDiscarded + 1}) than allowed (${maximumDiscarded})",
       dogfood.runSuite(Meta.DiscardedChecks))
   }
+  test("Discard ratio of zero should still run tests") {
+    dogfood =>
+      expectNoErrorMessage(dogfood.runSuite(Meta.NoDiscardsChecks))
+  }
 
   def expectErrorMessage(
       msg: String,
@@ -85,6 +92,14 @@ object PropertyDogFoodTest extends IOSuite {
         expect(log.contains(msg))
       }
     }
+
+  def expectNoErrorMessage(state: IO[DogFood.State]): IO[Expectations] =
+    state.map { case (logs, _) =>
+      val errorLogs = logs.collect {
+        case LoggedEvent.Error(msg) => msg
+      }
+      expect(errorLogs.size == 0)
+    }
 }
 
 object Meta {
@@ -127,6 +142,15 @@ object Meta {
     }
   }
 
+  trait SucceededChecks extends MetaSuite {
+
+    test("foobar") {
+      partiallyAppliedForall { (_: Int) =>
+        IO(expect(true))
+      }
+    }
+  }
+
   trait DiscardedChecks extends MetaSuite {
 
     test("Discards all the time") {
@@ -156,4 +180,19 @@ object Meta {
           1
         ) // to avoid overcounting of discarded checks
   }
+
+  object NoDiscardsChecks extends SucceededChecks {
+
+    override def partiallyAppliedForall: PartiallyAppliedForall = forall
+
+    override def checkConfig =
+      super.checkConfig
+        .withMinimumSuccessful(5)
+        // Set the discard ratio to 0. No discarded tests are permitted.
+        .withMaximumDiscardRatio(0)
+        .withPerPropertyParallelism(
+          1
+        ) // to avoid overcounting of discarded checks
+  }
+
 }