Why isn't there a simpler Expect.all test?

[mattpiz]

By simpler I mean:

all : List Expectation -> Expectation

Instead of:

all : List (subject -> Expectation) -> subject -> Expectation

To give a little more context, I’m currently working on a data structure (more on that in few days/weeks) and testing a filter function like so:

filter : Test
filter =
    describe "filter"
        [ fuzz2 TestFuzz.length (Fuzz.intRange 0 255) "Split at index" <|
            \length index ->
                let
                    rangeArray =
                        JsUint8Array.initialize length identity

                    biggerThanIndex n =
                        index < n

                    biggerArray =
                        JsTypedArray.filter biggerThanIndex rangeArray

                    smallerArray =
                        JsTypedArray.filter (not << biggerThanIndex) rangeArray

                    bigLength =
                        JsTypedArray.length biggerArray

                    smallLength =
                        JsTypedArray.length smallerArray
                in
                Expect.all
                    [ \_ -> Expect.equal length (bigLength + smallLength)
                    , \_ -> Expect.true "" <| JsTypedArray.all biggerThanIndex biggerArray
                    , \_ -> Expect.false "" <| JsTypedArray.any biggerThanIndex smallerArray
                    ]
                    ()
        ]

An all function with signature like proposed would change the last part to:

Expect.all
    [ Expect.equal length (bigLength + smallLength)
    , Expect.true "" <| JsTypedArray.all biggerThanIndex biggerArray
    , Expect.false "" <| JsTypedArray.any biggerThanIndex smallerArray
    ]

Instead of:

Expect.all
    [ \_ -> Expect.equal length (bigLength + smallLength)
    , \_ -> Expect.true "" <| JsTypedArray.all biggerThanIndex biggerArray
    , \_ -> Expect.false "" <| JsTypedArray.any biggerThanIndex smallerArray
    ]
    ()

I guess there is a reason behind this choice, I’m curious to know what is the tradeof there.

[rtfeldman]

Great question!

The idea is that it’s generally better to split separate expectations into multiple tests, each checking one of those properties. This way, failures are more specific and granular. You can extract the common setup logic into a helper function, so each of those tests becomes very small.

The purpose of Expect.all is to make it easier to create helper functions for checking multiple things at once. For example if you want to write a function which checks "this value must be between the given x and y" you can write that function quickly using Expect.all. Otherwise you’d have to write a bunch of ifs.

It might be good for us to revise the documentation to clarify this.

[mattpiz]

You can extract the common setup logic into a helper function, so each of those tests becomes very small.

Following your advice with the helper, I obtained the following form, wich in my opinion is really painful (sorry for long code block):

filter : Test
filter =
    let
        splitHelper length index =
            let
                rangeArray =
                    JsUint8Array.initialize length identity

                biggerThanIndex n =
                    index < n

                biggerArray =
                    JsTypedArray.filter biggerThanIndex rangeArray

                smallerArray =
                    JsTypedArray.filter (not << biggerThanIndex) rangeArray

                bigLength =
                    JsTypedArray.length biggerArray

                smallLength =
                    JsTypedArray.length smallerArray
            in
            (smallLength, bigLength, biggerThanIndex, smallerArray, biggerArray)
    in
    describe "filter"
        [ fuzz2 TestFuzz.length (Fuzz.intRange 0 255) "Sum of lengths equal original length" <|
            \length index ->
                let
                    (smallLength, bigLength, _, _, _) =
                        splitHelper length index
                in
                Expect.equal length (bigLength + smallLength)
        , fuzz2 TestFuzz.length (Fuzz.intRange 0 255) "Bigger array contains bigger elements" <|
            \length index ->
                let
                    (_, _, biggerThanIndex, _, biggerArray) =
                        splitHelper length index
                in
                Expect.true "" <| JsTypedArray.all biggerThanIndex biggerArray
        , fuzz2 TestFuzz.length (Fuzz.intRange 0 255) "Smaller array contains smaller elements" <|
            \length index ->
                let
                    (_, _, biggerThanIndex, smallerArray, _) =
                        splitHelper length index
                in
                Expect.false "" <| JsTypedArray.any biggerThanIndex smallerArray
        ]

I might have done it wrong but I’m not sure there is a better way of doing so.

Having specificity and granularity of failure in mind, I wonder if there is space in the API for a pair of functions like these:

Expect.allDescribed : List (Described Expectation) -> Expectation

Expect.describe : String -> Expectation -> Described Expectation

Which would be used like this:

Expect.allDescribed
    [ Expect.describe "Sum of lengths equal original length" <|
        Expect.equal length (bigLength + smallLength)
    , Expect.describe "Bigger array contains bigger elements" <|
        Expect.true "" <| JsTypedArray.all biggerThanIndex biggerArray
    , Expect.describe "Smaller array contains smaller elements" <|
        Expect.false "" <| JsTypedArray.any biggerThanIndex smallerArray
    ]

What do you think?

[mattpiz]

The idea is that it’s generally better to split separate expectations into multiple tests

Actually, I just realized that the issue is that it is an Expectation and not a Test so it cannot have multiple descriptions levels.

Probably the function I’m missing is more something like:

-- see the (a -> Test) instead of (a -> Expectation)
fuzzTest : Fuzzer a -> String -> (a -> Test) -> Test

Which would allow me to have tests sharing the same fuzzed context:

filter : Test
filter =
    fuzzTest2 TestFuzz.length (Fuzz.intRange 0 255) "Filter" <|
        \length index ->
            let
                -- some definitions
            in
            Test.concat
                [ test "Sum of lengths equal original length" <|
                    \_ -> Expect.equal length (bigLength + smallLength)
                , test "Bigger array contains bigger elements" <|
                    \_ -> Expect.true "" <| JsTypedArray.all biggerThanIndex biggerArray
                , test "Smaller array contains smaller elements" <|
                    \_ -> Expect.false "" <| JsTypedArray.any biggerThanIndex smallerArray
                ]

Or maybe this idea is worse, or not implementable?

[rtfeldman]

What do you think of this approach?

makeArrays length index =
    let
        rangeArray =
            JsUint8Array.initialize length identity

        biggerThanIndex n =
            index < n

        biggerArray =
            JsTypedArray.filter biggerThanIndex rangeArray

        smallerArray =
            JsTypedArray.filter (not << biggerThanIndex) rangeArray
    in
    { smaller = smallerArray, bigger = biggerArray }


filter : Test
filter =
    describe "filter"
        [ fuzz2 TestFuzz.length (Fuzz.intRange 0 255) "Sum of lengths equal original length" <|
            \length index ->
                let
                    { smaller, bigger } =
                        makeArrays length index
                in
                (JsTypedArray.length smaller + JsTypedArray.length bigger)
                    |> Expect.equal length
        , fuzz2 TestFuzz.length (Fuzz.intRange 0 255) "Bigger array contains bigger elements according to JsTypedArray.all" <|
            \length index ->
                makeArrays length index
                    |> .bigger
                    |> JsTypedArray.all (\num -> index < num)
                    |> Expect.true "All elements should have been bigger than their index in the array"
        , fuzz2 TestFuzz.length (Fuzz.intRange 0 255) "Smaller array contains smaller elements according to JsTypedArray.any" <|
            \length index ->
                makeArrays length index
                    |> .smaller
                    |> JsTypedArray.any (\num -> index < num)
                    |> Expect.false "No elements should have been bigger than their index in the array"
        ]

[mattpiz]

What do you think of this approach?

That’s cleaner than with tuples, thanks alot for your feedback!

[rtfeldman]

Glad it was helpful! ️

[gampleman]

It’s also pretty trivial to make your own helper that behaves like what you suggested. I use one like that in elm-visualization:

gampleman/elm-visualization/blob/1.6.0/tests/Helper.elm#L69-L71