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.

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

How should Properties.overrideParameters work? #360

Closed
ssanj opened this issue Oct 8, 2017 · 5 comments · Fixed by #522
Closed

How should Properties.overrideParameters work? #360

ssanj opened this issue Oct 8, 2017 · 5 comments · Fixed by #522

Comments

@ssanj
Copy link
Contributor

ssanj commented Oct 8, 2017

This is a question around how Properties.overrideParameters should work and is linked to #233 and #234. I'm seeing slightly different behaviour depending on whether I'm running the Property through SBT test or through SBT run-main (command line) with and without overrides.

Given the following property:

import org.scalacheck.Properties
import org.scalacheck._

object BasicProps extends Properties("Basic") {

  override def overrideParameters(p: Test.Parameters): Test.Parameters =
     p.withMinSuccessfulTests(1000)

  property("should test basic things") =
    Prop.forAll { n: Int => n != 42 }
}

I wanted to run the above property more than a 100 times to provoke a failure. I've overridden overrideParameters and set withMinSuccessfulTests to 1000.

When I run this through SBT with the test command I get the following:

[info] + Basic.should test basic things: OK, passed 100 tests.

I'm a bit confused as to why the above property wasn't run with a 1000 tests.

When I run this through SBT with test:run-main (command line) I get the following:

> test:run-main com.example.validation.extra.BasicProps
[info] Running com.example.validation.extra.BasicProps
+ Basic.should test basic things: OK, passed 1000 tests.

In the above case it honours the withMinSuccessfulTests of a 1000.

If I try to override the value of withMinSuccessfulTests from the command line through run-main with -s I get the following:

> test:run-main com.example.validation.extra.BasicProps -s 5
[info] Running com.example.validation.extra.BasicProps -s 5
+ Basic.should test basic things: OK, passed 1000 tests.

The command line override doesn't seem to work if withMinSuccessfulTests has been set on the property.

If I define the following property without any overrides:

import org.scalacheck.Properties
import org.scalacheck._

object Basic2Props extends Properties("Basic2") {

  property("should test basic things") =
    Prop.forAll { n: Int => n != 42 }
}

Running test:run-main with an override for the number of minimum successful tests, I get:

> test:run-main com.example.validation.extra.Basic2Props -s 5
[info] Running com.example.validation.extra.Basic2Props -s 5
+ Basic2.should test basic things: OK, passed 5 tests.

It looks like the default number of minimum successful tests are overridden in this case.

Running with SBT test uses the defaults:

[info] + Basic2.should test basic things: OK, passed 100 tests.

Running with SBT test:run-main uses the defaults:

[info] Running com.example.validation.extra.Basic2Props
+ Basic2.should test basic things: OK, passed 100 tests.

I would have assumed the following would be the precedence for overriding values to ScalaCheck:

  1. If a parameter is supplied on the command line it takes precedence over any values defined in overrideParameters.
  2. If a parameter is not supplied on the command line, values defined in overrideParameters would take effect or the defaults would be used if there are no overrides.
  3. When running from SBT test the values defined in overrideParameters would take effect or the defaults would be used if there are no overrides.

Is this not the expected behaviour?

@ashawley
Copy link
Contributor

ashawley commented Oct 9, 2017

Did it work after #234 was merged and released in version 0.13.2? Or did it stop working more recently? Or has it never worked?

@ssanj
Copy link
Contributor Author

ssanj commented Oct 11, 2017

I tried with ScalaCheck version 0.13.1 (before #234 was merged) and with 0.13.2 (after #234 was merged). The results are the same as above:

ScalaCheck: 1.13.1

BasicProps with override (1000)

Method Tests run
sbt test 100
sbt run-main 1000
sbt run-main -s 5 1000

Basic2Props without override

Method Tests run
sbt test 100
sbt run-main 100
sbt run-main -s 5 5

ScalaCheck 1.13.2

BasicProps with override (1000)

Method Tests run
sbt test 100
sbt run-main 1000
sbt run-main -s 5 1000

Basic2Props without override

Method Tests run
sbt test 100
sbt run-main 100
sbt run-main -s 5 5

From the above it looks like it never worked as expected since 1.13.1 at least.

@ashawley
Copy link
Contributor

When it was introduced in #188 it was suggested by @rickynils that it was a good first start, but would only work for property collections, and that a better approach was in the works for ScalaCheck 2.0.

@ssanj
Copy link
Contributor Author

ssanj commented Oct 11, 2017

The above configurations all run property collections not individual props - which I presume would also be supported in ScalaCheck 2.0. It should be possible to fix the current implementation to work as expected at least until ScalaCheck 2.0 drops. I'm happy to take a crack at it if the expected behaviour is as per the precedence outlined in my original issue.

@tfinneid
Copy link

tfinneid commented May 5, 2018

Hi, I am experiencing the same issue as @ssanj and would be very glad if this issues could be resolved somehow. Is the suggested PR in #365 an acceptable and working solution (disregarding, for the moment, the current merge conflict)? Is there something I could do to help?

(I tested running a similar test in IntelliJ and on the commandline and it seems to work fine. But, as noted, does not work in SBT.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants