Skip to content

Make seed part of config and pass into recheck data#482

Open
njlr wants to merge 1 commit intohedgehogqa:masterfrom
njlr:feature/issue-450
Open

Make seed part of config and pass into recheck data#482
njlr wants to merge 1 commit intohedgehogqa:masterfrom
njlr:feature/issue-450

Conversation

@njlr
Copy link
Copy Markdown
Contributor

@njlr njlr commented Mar 23, 2026

  • Adds the type SeedConfig for defining starting seed configuration
  • Extends IPropertyConfig with a SeedConfig
  • Adds helpers for setting the SeedConfig in a IPropertyConfig
  • Uses SeedConfig to set initial RecheckData in reportWith
  • Fixes seed in PropertyConfig.defaults
  • Adds a snapshot test for determinism

Relates to #450

Copy link
Copy Markdown
Member

@moodmosaic moodmosaic left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

The core feature is a good addition, but the default should be RandomSeed to preserve existing behavior. The withSeed API lets users opt into determinism explicitly, which is the right trade-off for a property testing library.

secondValues.Count > 10 |> Expect.isTrue
thirdValues.Count > 10 |> Expect.isTrue

testCase "Seed is deterministic" <| fun () ->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is great! Though I think more cases are warranted:

  • same fixed seed, same results
  • same fixed seed in sync and async path give same results
  • withRandomSeed differs from fixed-seed behavior
  • recheck path preserves configured seed semantics

{ TestLimit = 100<tests>
ShrinkLimit = None }
ShrinkLimit = None
SeedConfig = FixedSeed (Seed.from 0UL) }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This changes the default config from “effectively random every run” to always deterministic.

I'd have to thoroughly check but if the shrinking/recheck model depends on initial seed plus shrink path, changing default seed behavior affects:

  • reproduced counterexamples
  • golden tests
  • user workflows that expect “rerun gives new samples”

This is a significant semantic change. Every user who calls Property.check, Property.checkBool, or any function that flows through PropertyConfig.defaults will now:

  • Always test the same inputs across runs (unless they opt into withRandomSeed)
  • Potentially miss bugs that only manifest with certain random seeds
  • Get different behavior from what they've been relying on without changing any of their own code

AFAIK, in most property-based testing systems, the default is random seeds with the seed printed on failure for reproducibility.

The deterministic-by-default philosophy might exist, but flipping an established default can be surprising to the users, if not risky.

Comment thread src/Hedgehog/Property.fs
/// This blocks until all tests complete.
let reportWith (config : IPropertyConfig) (p : Property<unit>) : Report =
p |> reportWith' (PropertyArgs.init ()) config
let seed = SeedConfig.init config.SeedConfig
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's good: it means only one concrete seed per property run, not reevaluating randomness multiple times.

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 this pull request may close these issues.

2 participants