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

Re-add Fuzz.andThen? #17

Closed
rtfeldman opened this issue Aug 22, 2018 · 13 comments
Closed

Re-add Fuzz.andThen? #17

rtfeldman opened this issue Aug 22, 2018 · 13 comments
Labels
Design Question Needs design discussion fuzzers Concerns randomness or simplifiers

Comments

@rtfeldman
Copy link
Collaborator

rtfeldman commented Aug 22, 2018

It's nontrivial to get Fuzz.andThen working in 0.19 because the 0.18 version implementation relied on Debug.crash.

Perhaps we should re-add Fuzz.andThen as a MINOR release, with more helpful error messages in the event that you give it an invalid fuzzer.

@drathier
Copy link
Collaborator

I'd say let's wait with re-adding this one, and see if we can improve the shrinking/fuzzing significantly first, because we don't have andThen. Also, see how many people actually miss it in 0.19.

@rtfeldman rtfeldman changed the title Re-add Fuzz.andThen Re-add Fuzz.andThen? Sep 1, 2018
@rtfeldman rtfeldman added the Design Question Needs design discussion label Sep 1, 2018
@ravicious
Copy link

I miss it. :( I have tests for my game which uses a 2D board. There's a fuzzer which takes a board dimension fuzzer (which returns (Int, Int)) and then based on the values from the tuple I generate an index which points at a random cell on that board.

https://github.com/ravicious/elm-sweeper/blob/64bb68cb9562043ac399e699a922a1b143c8db12/tests/Tests.elm#L24-L35

@andys8
Copy link

andys8 commented Nov 26, 2018

I miss it. Having a structure with fuzzed uuids and foreign key relation and constraints between data, that should be generated. It's possible with andMap, but not pretty. It's a workaround.

@AionDev
Copy link

AionDev commented Jul 3, 2019

i miss it. I cant move forward with my project - i spent 3 days on this issue already. Given i didn't know much about shrinkers and stuff - needed to learn it first - still was not very fast. And im not sure how to do it even after all the research and testing i've done.
Given :

type A
   = A
       { id : A_id
       , b : B    -- A contains B.
       }

type A_id
   = A_id String

type B
   = B { id : B_id }

type B_id
   = B_id ( A_id, String ) 

How would you build a Fuzzer A? What if A contains another C and D.. not just a B inside - all of them need stuff from A.. This promises to be super easy using andThen.
The relevant stack overflow question is here: https://stackoverflow.com/questions/56876283/how-to-flatten-fuzz-fuzz-fuzz-a-without-having-andthen-in-the-fuzz-test-m

@1dr0z
Copy link

1dr0z commented Jul 31, 2019

I have a case that I don't think I can solve without something like andThen. I have a helper like this:

repeat : Int -> Fuzzer a -> Fuzzer (List a)

This helper works! But I want to repeat a variable number of times by using Fuzz.intRange. Since repeat already returns a Fuzzer I end up with a signature like Fuzzer (Fuzzer (List a)).

What I need is a way to return a new Fuzzer based on the result of a previous Fuzzer, which seems like exactly what andThen would do:

andThen: (a -> Fuzzer b) -> Fuzzer a -> Fuzzer b

Anyway, hoping this can serve as an example of where andThen would be useful!

@mgold
Copy link
Collaborator

mgold commented Aug 1, 2019

Fuzzers have two pieces: generators, and shrinkers. You can use Random.andThen to create a complex generator. You can use Shrink.noShrink as shrinker. Then pass them both into Fuzz.custom.

We know it's hard to write your own shrinker right now; there are some changes coming in the next version. But disregarding any technical limitations, we think that making a general-purpose shrinker with andThen is not feasible.

As a small concession, maybe we could say andThen removes the shrinker, and you should provide your own as another argument?

@neoncrisis What's wrong with Fuzz.list?

@1dr0z
Copy link

1dr0z commented Aug 2, 2019

@mgold I was able to work around my problem using Fuzz.custom 🙂. The reason Fuzz.list doesn’t work for me is that I’m testing encoders/decoders round trip and our encoders enforce the length and allowable characters of certain string e.g., zip code, phone number, ssn, &c. The best way I was able to control the length of strings is by building them from a list of Char whose length I control.

@mgold
Copy link
Collaborator

mgold commented Aug 3, 2019

That confirms what we think: for types/data structures with sufficiently complex business logic, you need to write separate generators and shrinkers. A generic shrinker wouldn't know anything about your length requirements; a custom one would know not to turn a legitimate failure into malformed data.

@jcornaz
Copy link

jcornaz commented Oct 30, 2019

As a small concession, maybe we could say andThen removes the shrinker, and you should provide your own as another argument?

Yes, I think it would be good. I think shriking is nice and all, but not as valuable as data generation itself. Being able to create fuzzer using andThen is very valuable by itself, even if it comes without shrinker.

@gwils
Copy link

gwils commented Nov 29, 2019

I miss andThen.

@mgold
Copy link
Collaborator

mgold commented Dec 4, 2019

So, this is clearly an issue that a lot of people have weighed in on. Although the top of the thread mentions Debug.crash the real problem is: if you are generating data in a way complicated enough to warrant andThen, there is no useful general-purpose shrinking strategy for that data. What I implemented tried to be that, but it was exceedingly complicated and apparently not too helpful for shrinking inputs. It should be noted that andThen and random generation work well together; only the shrinking is difficult.

We've been saying to use Fuzz.custom, but that has two problems. One, the generators used in Fuzz.int and similar aren't available (I think there's another issue for that), and that's a problem because these generators are specifically designed to favor edge cases rather than be unbiased, like you'll find in elm/random. Two, you need to provide your own shrinker, and I think shrinkers are currently unusable beyond Shrink.noShrink. (On master, Shrinkers are renamed Simplifiers.)

So here's an idea:

  1. Re-add Fuzz.andThen based on Random.andThen and no shrinking.
  2. Add a function to replace the shrinker of a function: f: Shrinker a -> Fuzzer a -> Fuzzer a. This allows users of Fuzz.andThen to add their own shrinker.

Optionally:
3. Add a function f: Fuzzer a -> Random.Generator a to extract the random generator from a fuzzer. This makes Fuzz.custom more usable, and makes implementing step 2 easier.
4. Detect when a fuzzer does not provide any shrinking options, and inform the user of this when displaying a test failure with many smallest examples.

I think all of that is, you know, actually possible to implement.

@mgold mgold added the fuzzers Concerns randomness or simplifiers label Dec 8, 2019
@ravicious
Copy link

Fuzz.andThen has been re-added in 2.0.0. 🙌

https://discourse.elm-lang.org/t/elm-explorations-test-2-0-0-released/8705

@Janiczek
Copy link
Collaborator

I'm glad we finally have Fuzz.andThen without any shrinking compromises 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Question Needs design discussion fuzzers Concerns randomness or simplifiers
Projects
None yet
Development

No branches or pull requests