Ability to ignore (or fail) on usage of @Property(seed=)

[osi]

Being able to add the seed argument to the @Property annotation is superb for reproducing failures.

However, sometimes they get accidentally committed, which can lead to a CI server runnings tests on a fixed seed.

The ability to ignore or fail on the presence of an explicit seed, for use in CI systems, would help prevent this error.

A system property or environment variable feels to be the easiest to make conditional upon the environment (as users may have jqwik.properties committed to SCM)

[jlink]

There’s two aspects here:

1. Configuring special behaviour when seed is specified (use, warn, fail, ignore)
2. Overriding config values in system prop

1 is crucial for you. 2 is optional, right?

[osi]

Correct. My goal is to prevent builds running in certain environments from using a fixed seed. (developer laptop, ok. CI server, no)

[jlink]

I’m a bit in doubt if a global (overwritable) config is all that helpful. I’ve seen a few codebases where some properties are purposefully fixed to a seed for a reason. Those properties should IMO not lead to failures.

What about having a second attribute in @Property, e.g. temporarySeed, which will always log a warning? I’m just rambling. Maybe/probably there are better options to reconcile both scenarios.

[osi]

What if the source of the seed for a property was available in a AroundPropertyHook ? It could be as simple of a change as having PropertyExecutionResult#seed return a tuple of the seed and the source (perhaps an enum of PREVIOUS_SEED, RANDOM_SEED, FIXED_SEED). A hook could transition the result to failed if it was using a FIXED_SEED in an inappropriate execution environment.

[jlink]

@osi Extending the AroundPropertyHook with such a specific trait does not feel adequate.

Here's my current suggestion:

1. Add a configuration parameter to jqwik.properties:
   onFixedSeed: ALLOW | WARN | FAIL
2. Allow configuration parameters be overridden by system properties handed in via
   -D jqwik.onFixedSeed=FAIL
3. Extend configuration to allow something like:
   onFixedSeed.net.myproject.MyTestclass.myPropertyMethod=ALLOW

What do you think?

Maybe jqwik should switch the whole configuration approach to use junit-platform.properties as described
here.
This would require a bigger initial effort to migrate jqwik.properties once but bring overridability for free.

[osi]

That suggestion would satisfy my goals.

I'm also open to helping switch to use junit-platform.properties. If we take that as a goal, I think your first point is all that'd be needed now, with the 2nd and 3rd being delegated to the platform support.

What were you thinking for the migration?

• Properties from jqwik.properties loaded from junit-platform.properties with an additional jqwik. prefix
• Backwards-compatible loading from jqwik.properties; Definitions here would take ultimate precedence.
• Warn/Fail if a property is declared using the platform mechanism and jqwik.properties

[jlink]

@osi I moved that aspect out into a new issue: #139

If you want to tackle it I can support you.

[osi]

thanks - i may try and take it up over my new years break

[jlink]

Now that configuration is using the platform mechanism, here's the plan:

• Add configuration parameter
  jqwik.seeds.whenfixed=ALLOW | WARN | FAIL
• Default value is ALLOW
• Mapped on enum FixedSeedMode

Setting fixed seed mode for individual test in configuration is currently out of scope.

Optional: Add whenFixedSeed attribute to @Property and @PropertyDefaults to override global default.

[osi]

got it. i'll pick this up.

…

-- (peter.royal|osi)@pobox.com - http://fotap.org/~osi

On Sun, Jan 10, 2021, at 9:14 AM, Johannes Link wrote: Now that configuration is using the platform mechanism, here's the plan: * Add configuration parameter `jqwik.seeds.whenfixed=ALLOW | WARN | FAIL` * Default value is `ALLOW` * Mapped on `enum FixedSeedMode` Setting fixed seed mode for individual test in configuration is currently out of scope. Optional: Add `whenFixedSeed` attribute to ***@***.***` and ***@***.***` to override global default. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#138 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAE3V77VQQGZ43UPHNQDM3SZHN77ANCNFSM4UOCBVWQ>.

[jlink]

got it. i'll pick this up.

This might have to dive in some dirty parts of the engine. If you have ANY doubt how to tackle this, just ask!

[osi]

Will do. Capturing the intent has been straightforward. I'm looking to add tests for the actual behavior and believe that net.jqwik.engine.properties.CheckedPropertyTests has similar tests / is the place to put them,