elm-community / elm-test

moved to elm-explorations/test
https://github.com/elm-explorations/test
BSD 3-Clause "New" or "Revised" License
340 stars 35 forks source link

Expect.andAlso: A way to combine Expectations #214

Open ocharles opened 6 years ago

ocharles commented 6 years ago

The API currently allows this, but it can be made nicer with a combinator:

andAlso : Expect.Expectation -> Expect.Expectation -> Expect.Expectation
andAlso l r =
    Expect.all [ always l, always r ] ()

I'm using this in a behavior test where I need to expect multiple things across multiple models:

suite : Test
suite =
    Test.describe "The part search page"
        [ Test.fuzz2 (Fuzz.list partFuzzer) (Fuzz.list partFuzzer) "Multiple searches only display the results of the most recent search" <|
            \results2 results1 ->
                let
                    apiContext =
                        { host = "testing.local", isSecure = False }

                    ( modelAfterOneSearch, _ ) =
                        PartSearch.init
                            |> PartSearch.setSearchQuery "Q1"
                            |> PartSearch.performSearch apiContext

                    ( modelAfterSecondSearch, _ ) =
                        modelAfterOneSearch
                            |> PartSearch.setSearchQuery "Q2"
                            |> PartSearch.performSearch apiContext

                    modelAfterFirstResponse =
                        PartSearch.setResults { requestIndex = 2, results = results2 } modelAfterSecondSearch

                    modelAfterSecondResponse =
                        PartSearch.setResults { requestIndex = 1, results = results1 } modelAfterFirstResponse
                in
                    Expect.equal (PartSearch.searchInProgress modelAfterFirstResponse) True
                        |> andAlso (Expect.equal (PartSearch.results modelAfterSecondResponse) results2)
        ]
ocharles commented 6 years ago

I guess I could also just make one big Bool and expect it to be True, but this feels like less of an abuse of the API.

mgold commented 6 years ago

In general, there is a tension between a test that is simple and clearly indicates the bug when it fails, versus tests that need to be complicated, and while you're going through the trouble you might as well make multiple expectations -- which can often make for poorer failure messages.

I think the best use of Expect.all is to compose many expectations that are somehow related.

validBusSchedule {origin, destination} =
  Expect.all
     [ Expect.notEqual origin.station destination.station
     , destination.time |> Expect.greaterThan origin.time
     ]

It looks like you have conceptually separate expectations that should be different tests. (Specific test descriptions are the best way to label a failure.) What if you tried moving your let definitions into a top-level function that takes the fuzzed values as inputs and returns a record of the things you need for expectations? Then you could grab the values and make two expectations in two tests. (Alternatively, move all the logic into a fuzzer.)

Also, you are using Expect.equal ... True instead of Expect.true. The latter gives you an opportunity to provide a useful message for when the test failed.

So, with those opinions expressed, let's talk about Expect.andAlso. I think it's a pretty neat trick, especially because you can chain it indefinitely. But, it's starting to look like maybe it encourages tests that do too much. So, maybe not, but I'm totally open to other opinions.

ocharles commented 6 years ago

What if you tried moving your let definitions into a top-level function that takes the fuzzed values as inputs and returns a record of the things you need for expectations? Then you could grab the values and make two expectations in two tests. (Alternatively, move all the logic into a fuzzer.)

Both reasonable suggestions! I will have a play and report back.

Also, you are using Expect.equal ... True instead of Expect.true. The latter gives you an opportunity to provide a useful message for when the test failed.

Thanks, I missed that when reading the documentation.

So, with those opinions expressed, let's talk about Expect.andAlso. I think it's a pretty neat trick, especially because you can chain it indefinitely. But, it's starting to look like maybe it encourages tests that do too much. So, maybe not, but I'm totally open to other opinions.

I hear you on this. I wrote it because I couldn't see the alternatives, so maybe the real solution is to document more examples. I felt like my test scenario was about the right scope for a test - I couldn't really see how to break it down into anything smaller. For context, the test is testing a search page on our app, and I want to assert that if the user performs two searches in quick succession, the results for the first search don't override the results of the second search (as the API might deliver results of the second search quicker, and we don't have a way to currently cancel in flight HTTP requests).

mgold commented 6 years ago

as the API might deliver results of the second search quicker

Ah, that explains why you have the literal 1 and 2. Looks like that's how you are simulating the out-of-order responses, so there's no way to derive them or something.

so maybe the real solution is to document more examples

That's an interesting idea. Typically these examples are somewhat contrived. It might be need to ask production users to submit tests they don't feel good about, along with an extracted implementation of the code under test. Then several other people can have a go at improving the test and we present the best option(s) the group came up with.

ocharles commented 6 years ago

Ah, that explains why you have the literal 1 and 2. Looks like that's how you are simulating the out-of-order responses, so there's no way to derive them or something.

Yes, indeed. Ultimately, here's what I ended up with:

module Tests.Page.PartSearch exposing (multipleSearchTests)

import Expect
import Fuzz
import Page.Parts as PartSearch
import Page.Parts.Model as PartSearch
import Random
import Random.List
import Random.Pcg
import Test exposing (Test, test)
import Urn.Types as Urn
import Maybe.Extra as Maybe

type alias TestSearch =
    { query : String, results : List PartSearch.Part }

multipleSearchTests : Test
multipleSearchTests =
    let
        testSearch : Fuzz.Fuzzer TestSearch
        testSearch =
            Fuzz.map2 TestSearch Fuzz.string (Fuzz.list partFuzzer)

        fuzzer :
            Fuzz.Fuzzer
                { -- The final search performed
                  lastSearch : TestSearch
                , -- A sequence of searches and results leading up to the final search
                  transitions : List Transition
                }
        fuzzer =
            testSearch
                |> Fuzz.andThen
                    (\lastSearch ->
                        Fuzz.list testSearch
                            |> Fuzz.andThen (\searches -> sequenceSearchesAndResults (List.append searches [ lastSearch ]))
                            |> Fuzz.map (\m -> { lastSearch = lastSearch, transitions = m })
                    )
    in
        Test.fuzz fuzzer "Multiple searches return the latest search results" <|
            \{ lastSearch, transitions } ->
                let
                    applyTransition transition model =
                        case transition of
                            DeliverResults query results ->
                                model

                            PerformSearch query ->
                                model
                                    |> PartSearch.setSearchQuery query
                                    |> PartSearch.performSearch { host = "testing.local", isSecure = False }
                                    |> Tuple.first

                    model =
                        List.foldl applyTransition PartSearch.init transitions
                in
                    Expect.equal (PartSearch.results model) lastSearch.results

type Transition
    = PerformSearch String
    | DeliverResults String (List PartSearch.Part)

-- Given a list of searches and mock results, sequence these searches into
--
-- a. A call to perform that search
-- b. Deliver the results of any searches that have been performed.
--
-- Searches are performed in the order of the input list, but the results
-- will be delivered in a non-deterministic order. Results will only be
-- delivered if a search has been made.

sequenceSearchesAndResults :
    List { query : String, results : List PartSearch.Part }
    -> Fuzz.Fuzzer (List Transition)
sequenceSearchesAndResults searches =
    let
        go :
            List { query : String, results : List PartSearch.Part }
            -> List { query : String, results : List PartSearch.Part }
            -> Fuzz.Fuzzer (List Transition)
        go searches results =
            case ( searches, results ) of
                ( [], [] ) ->
                    Fuzz.constant []

                _ ->
                    Fuzz.oneOf
                        (Maybe.values
                            [ uncons searches
                                |> Maybe.map
                                    (\( search, searches ) ->
                                        Fuzz.map (\ops -> PerformSearch search.query :: ops)
                                            (go searches (search :: results))
                                    )
                            , uncons results
                                |> Maybe.map
                                    (\( { query, results }, otherResults ) ->
                                        Fuzz.map (\ops -> DeliverResults query results :: ops)
                                            (go searches otherResults)
                                    )
                            ]
                        )
    in
        go searches []

partFuzzer : Fuzz.Fuzzer PartSearch.Part
partFuzzer =
    Fuzz.constant PartSearch.Part
        |> Fuzz.andMap Fuzz.string
        |> Fuzz.andMap (Fuzz.maybe Fuzz.string)
        |> Fuzz.andMap Fuzz.bool
        |> Fuzz.andMap Fuzz.float
        |> Fuzz.andMap Fuzz.int
        |> Fuzz.andMap Fuzz.string
        |> Fuzz.andMap (Fuzz.maybe Fuzz.int)
        |> Fuzz.andMap (Fuzz.maybe Fuzz.float)
        |> Fuzz.andMap (Fuzz.maybe Fuzz.float)
        |> Fuzz.andMap (Fuzz.map Urn.encode (Fuzz.list Fuzz.string))
        |> Fuzz.andMap Fuzz.int

uncons : List a -> Maybe ( a, List a )
uncons l =
    case l of
        [] ->
            Nothing

        x :: xs ->
            Just ( x, xs )

Unfortunately, this blows the stack. I can fix this by changing

                        Fuzz.list testSearch

with

                        Fuzz.map3 (\a b c -> [a, b, c]) testSearch testSearch testSearch

But essentially the idea is to create a list of random searches and random results for each search, and then build a plan of state transitions - searches are in order, but interleaved with a non-deterministic ordering of search results.

ocharles commented 6 years ago

I'm not sure what to do with this issue. If you're not yet convinced about a need for andAlso then we can close this. I'm not suggesting that I think the above is ample motivation, but maybe you can think of other cases where it would be useful. For my immediate problem above though, I think I'm going to take you up on your suggestions and rewrite my test to test a single behavior.