Implement Fuzz.filterMap

elm-explorations / test

Write unit and fuzz tests for Elm code.

https://package.elm-lang.org/packages/elm-explorations/test/latest

BSD 3-Clause "New" or "Revised" License

236 stars 40 forks source link

Implement Fuzz.filterMap #220

Closed mbaechler closed 7 months ago

mbaechler commented 1 year ago

I needed this function on a project I'm working on so I figured it could be interesting to contribute it back upstream.

edit: By the way, I couldn't run the test locally, I expect to CI to help me about that.

Janiczek commented 1 year ago

Hey there! Thanks for the contribution.

Can you please provide more info about your usecase? Just so that I can get my head around when this code would be useful.

Re tests, you can try ./tests/run-tests.sh. The setup is a bit special as this is elm-explorations package with kernel JS modules.

The code itself looks good, I'd just

[ ] add the new function to the docs: on line 65,
```
@docs constant, invalid, filter, filterMap
```
[ ] use a more realistic example in the doc for the filterMap function. Right now it's just the filter doc with Just a instead of True and Nothing instead of False. I don't yet know what example that should be though.
[ ] Add a test for the relationship between filter and filterMap: something along the lines of
```
fuzzer |> filterMap f |> map ((/=) Nothing)
===
fuzzer |> filter (f >> (/=) Nothing)
```

lue-bird commented 1 year ago

Examples of cases when you want filterMap are the same as with filter, only that you get a more narrow type when it succeeds, like

module UnicodeNonLetter exposing (UnicodeNonLetter, fromChar, fuzz)

type UnicodeNonLetter = UnicodeNonLetter Char

fromChar : Char -> Maybe UnicodeNonLetter
fromChar c =
    if (c |> Unicode.isLower |> not) && (c |> Unicode.isUpper |> not) then
        UnicodeNonLetter |> Just

    else
        Nothing

fuzz : Fuzzer UnicodeNonLetter
fuzz =
    Fuzz.char |> Fuzz.filterMap fromChar

Janiczek commented 1 year ago

Sounds lovely - that would be a nice example to have in the docs :+1:

mbaechler commented 11 months ago

Hi @Janiczek , sorry for the delay but thanks for your review.

use a more realistic example in the doc for the filterMap function. Right now it's just the filter doc with Just a instead of True and Nothing instead of False. I don't yet know what example that should be though.

Is it ok if I borrow the example from @lue-bird ?

Add a test for the relationship between filter and filterMap

Could you provide me some guidance about that?

Janiczek commented 11 months ago

Is it ok if I borrow the example from @lue-bird ?

Sure thing!

Could you provide me some guidance about that?

Actually, ignore this requirement. I wasn't fully switched into the "Fuzzers are functions" train of thought when writing that... I was thinking in Lists. I don't think we have an easy way to test equivalence of fuzzers right now. There might be some in the future but let's skip this for now.

mbaechler commented 8 months ago

Hi @Janiczek ,

I finally found some time to update my PR, it should be good now.

Let me know if there are still things to fix.

Janiczek commented 7 months ago

Hey @mbaechler, the CI tests failed because of a forgotten stale version in tests/elm.json. I've fixed it here: https://github.com/elm-explorations/test/commit/81b15e2e9ae39a3a437e1dc80ede11acd86453da

I'm gonna pull master into your branch and try the tests again.

Janiczek commented 7 months ago

Thanks for your work on this!