eeue56 / elm-html-test

Test elm-html in Elm!
http://package.elm-lang.org/packages/eeue56/elm-html-test/latest
BSD 3-Clause "New" or "Revised" License
67 stars 15 forks source link

Query.contains [WIP] #20

Closed yormi closed 7 years ago

yormi commented 7 years ago

The goal is to be able to check if a function f returns the returned Html Msg of another function g without knowing the details of g.

test "recognizes if it is has a specific descendant" <|
    \() ->
        let
            aSpecificView data = ... -- some html rendering
            someData = ...
        in
            div [] [ aSpecificView data ]
                |> Query.contains (aSpecificView data)

This is just a silly example but let's imagine a more complex view function with conditions to see why this is useful :)

rtfeldman commented 7 years ago

This is just a silly example but let's imagine a more complex view function with conditions to see why this is useful :)

@yormi thanks for writing this up!

"This is just a silly example, and imagine a better one to see why this is useful" is a red flag to me...it's really easy to expand an API until it's huge based on hypothetical use cases, so I think it's important to push back on additions that don't solve a specific problem that has already come up. 😉

Can you say more about how you'd use this in a specific code base of yours?

yormi commented 7 years ago

Haha I totally get your point about keeping the API to expand.

On the other hand, I don't get how people test their views. Maybe I just missed something. Feel free to let me know ;) This is my attempt to explain why I need it:

I don't have a big code base. I was actually starting a project in elm and I was thinking about how to keep tests from depending on the markup. I don't want to add a div and mess all my tests.

What I want to do is: make small view-functions abstracting the markup. This way, I can reuse those blocks and avoid repeating myself (aka copy-pasting). Pretty standard so far, isn't it ?

The thing is, if I want to use those blocks to test conditional views, or for basically any tests. With the current API, I need to make expectations on the tags or on the attributes. Maybe I'm a bad web designer but I happen to change these pretty often :P

So the point of this new function in the API is to allow the usage of the "view blocks" in the tests too. With the "contains" (really open to other names), I can change the markup and the styles and so on without having to modify all my tests that validate if the block has been rendered.

For example, I'm gonna have to deal with a lot of graphs in many parts of my app. I'm gonna have some kind of abstraction of the markup in a graphView function. The rendering of this graph "block" is gonna be everywhere in my app therefore everywhere in the tests as well.

This graph block might be a div, or an aor even a li. I don't care about that for the test of pageXView. I just want to know if it is rendering graphView with the arguments I want.

To change just a little bit the example of the previous post:

test "PageX has a hapiness graph if a happy user is logged in" <|
    \() ->
        let
            moodData = [1, 2, 3, 4 ]
            authModel = Just User "Happy Dude" moodData
            expected = graphView moodData
        in
            pageXView authModel
                |> Query.contains expected

I think it's the best way to do it. I'm open to be proven wrong :p

Sorry for the long post. I just tried to be clear about why I (at least think) I need it. Did I succeed !? :O

PS: I changed a bit the contains so that it takes a List Html for the expected argument to be able to check for many at the time. Like this it is more coherent with the has expectation. elm-format made a lot of linting changes to the source code so I won't push it right now.

eeue56 commented 7 years ago

think this is a reasonable change, if you push the updated code. You'll likely need to rebase. The new version will be safer

yormi commented 7 years ago

Voilà ! I struggled with git revert-rebase-reset... learned a lot !

Let me know if there's anything!

rtfeldman commented 7 years ago

I'm sold on merging+releasing this...sound good @eeue56?

eeue56 commented 7 years ago

yeah it can be merged after another rebase

yormi commented 7 years ago

Can't do it right now. Will try tomorrow morning !

yormi commented 7 years ago

Thanks guys :)