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

Reproduce via random seed #50

Closed
rtfeldman opened this issue Aug 10, 2016 · 4 comments
Closed

Reproduce via random seed #50

rtfeldman opened this issue Aug 10, 2016 · 4 comments

Comments

@rtfeldman
Copy link
Owner

This is a two-part feature (and neither makes much sense without the other.)

  1. Add something like this to the output: Fuzzing with initial seed 1234 (Not sure if it should go at the beginning or at the end, or what the exact wording should be.)
  2. Accept a CLI flag for what numeric seed to use on init.

This is no substitute for copying out the inputs of fuzz tests into failing unit tests, but would let you give the seed to a coworker so they could quickly reproduce exactly the set of failures you saw.

@rtfeldman
Copy link
Owner Author

Also I feel like when picking one at random, we should start with the initial time and then do like Random.int 1000 9999 to generate a relatively concise int for a seed that you could quickly holler across a room at a coworker who wants to reproduce what you're seeing.

Four digits seems like a plenty large enough possibility space given that we're not doing crypto or anything, but @mgold would be the expert on that.

@mgold
Copy link
Collaborator

mgold commented Aug 10, 2016

As with anything involving reusing or seeds or generating seeds from randomized output, the answer is: theoretically you are compromising the validity of the RNG and you should feel bad; in practice it's probably fine (until you get a really obscure and difficult bug).

I don't think it's worth limiting the 4 billion-ish possibilities to 9000 just to make the seeds easier to shout across the room. I've done similar things with flaky rspecs and I just wind up copying and pasting the seed at one machine.

I'm totally on board with indicating and being able to specify the initial seed, but the opaque type is working against you. With Random.Pcg you can do it by abusing toJson. If that's not merged into core (as is likely), you might have to rely on toString.

@rtfeldman
Copy link
Owner Author

I just wind up copying and pasting the seed at one machine.

Yeah fair point.

the opaque type is working against you

Oh I'd just change the API to accept an Int instead of a Random.Seed

@mgold
Copy link
Collaborator

mgold commented Aug 11, 2016

Oh I'd just change the API to accept an Int instead of a Random.Seed

Sure, just document why this is the case and that the integer should be 32 random bits, etc. Again in the short-term you can avoid an API change thanks to Random.Pcg.toJson. Wait this is the runner that hasn't been released yet, not elm-test itself, so let's make the breaking API change now.

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

No branches or pull requests

2 participants