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

Jump to bottom

Reduce maxSize and minSuccessful in Scalacheck params #952

Merged
merged 2 commits into from
Apr 18, 2016
Merged

Reduce maxSize and minSuccessful in Scalacheck params #952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8608018
Reduce maxSize and minSuccessful in Scalacheck params
ceedubs Mar 27, 2016
2e7b4ff
Use more/bigger scalacheck samples on JVM than JS
ceedubs Apr 18, 2016
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
5 changes: 4 additions & 1 deletion laws/src/main/scala/cats/laws/discipline/Eq.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cats
package laws
package discipline

import catalysts.Platform
import algebra.Eq
import org.scalacheck.Arbitrary

Expand All @@ -12,8 +13,10 @@ object eq {
* and comparing the application of the two functions.
*/
implicit def function1Eq[A, B](implicit A: Arbitrary[A], B: Eq[B]): Eq[A => B] = new Eq[A => B] {
val sampleCnt: Int = if (Platform.isJvm) 50 else 5
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to keep the JVM side at 100? Is there example builds where this has failed for lack of output when building the JVM side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a fair question. When I wrote this my thought process was that these tests run (presumably with a different RNG seed) on every PR and every merge to master, so 50 samples should be plenty, especially given the general nature of cats. I thought this might just speed things up a bit. I don't have any strong opinions on this and am happy to change it if people would prefer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mikejcurry I'm going to go ahead and merge this PR to help make the build reliable. Feel free to open up a followup issue if you have any concerns about this lower number.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ceedubs - I've no strong feelings. I just wanted to raise the question, and the build speed is a good argument as the cats end to end build has certainly gotten longer. I just wanted to raise the question. :-)


def eqv(f: A => B, g: A => B): Boolean = {
val samples = List.fill(100)(A.arbitrary.sample).collect{
val samples = List.fill(sampleCnt)(A.arbitrary.sample).collect{
case Some(a) => a
case None => sys.error("Could not generate arbitrary values to compare two functions")
}
Expand Down
9 changes: 6 additions & 3 deletions tests/src/test/scala/cats/tests/CatsSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import catalysts.Platform
import cats.std.AllInstances
import cats.syntax.{AllSyntax, EqOps}

import org.scalactic.anyvals.{PosZDouble, PosInt}
import org.scalactic.anyvals.{PosZDouble, PosInt, PosZInt}
import org.scalatest.{FunSuite, Matchers}
import org.scalatest.prop.{Configuration, GeneratorDrivenPropertyChecks}
import org.typelevel.discipline.scalatest.Discipline
Expand All @@ -20,8 +20,11 @@ trait TestSettings extends Configuration with Matchers {

lazy val checkConfiguration: PropertyCheckConfiguration =
PropertyCheckConfiguration(
minSuccessful = if (Platform.isJvm) PosInt(100) else PosInt(10),
maxDiscardedFactor = if (Platform.isJvm) PosZDouble(5.0) else PosZDouble(50.0))
minSuccessful = if (Platform.isJvm) PosInt(50) else PosInt(5),
maxDiscardedFactor = if (Platform.isJvm) PosZDouble(5.0) else PosZDouble(50.0),
minSize = PosZInt(0),
sizeRange = if (Platform.isJvm) PosZInt(10) else PosZInt(5),
workers = PosInt(1))

lazy val slowCheckConfiguration: PropertyCheckConfiguration =
if (Platform.isJvm) checkConfiguration
Expand Down