elm-html-test
elm-html-test copied to clipboard
`Query.contains` gives unexpected output when using event handlers
Originally reported in https://github.com/eeue56/elm-html-test/issues/37
Suppose we write this test code. It should fail because it's requesting an onInput handler that isn't there:
import Html exposing (div, li, text, ul)
import Html.Attributes exposing (class)
import Html.Events exposing (onInput)
import Test exposing (Test, test)
import Test.Html.Query as Query
fails : Test
fails =
test "The list has two li: one with the text \"third item\" and another one with \"first item\"" <|
\() ->
div []
[ ul [ class "items active" ]
[ li [] [ text "first item" ]
, li [] [ text "second item" ]
, li [] [ text "third item" ]
]
]
|> Query.fromHtml
|> Query.contains
[ li [ onInput (always ()) ] [ text "third item" ]
, li [] [ text "first item" ]
]
Here's the error we get:
▼ Query.fromHtml
<div>
<ul class="items active">
<li>
first item
</li>
<li>
second item
</li>
<li>
third item
</li>
</ul>
</div>
▼ Query.contains
✗ /1\ missing descendants:
1) <li>
third item
</li>
This error message is surprising because it says it couldn't find the given descendant, even though it was there.
I can think of a few ways of resolving this:
- Make
Query.containstakeList (Html Never)instead ofList (Html msg). This would be less surprising, but would also make the function nearly useless in practice. We'd also still have to explain that an event handler was involved when giving an error that the output was different. This doesn't seem like a good option. - Make
Query.containsdo a referential equality check on event handlers, and succeed if that succeeds, but fail otherwise. This would make it Just Work in quite a few cases, but in the cases where it failed—e.g. because you switched your implementation fromonInput SetTexttoonInput (String.toLower >> SetText)—it would be really confusing to explain why it failed, and how you'd need to adjust (read: rewrite) your test not to run afoul of that. Probably in this world it would become best practice to recommend using plain queries instead ofQuery.contains. - Make
Query.containssilently ignore event handlers - both in theHtmlyou give it as well as in theHtmlit checks. Even if we put this in the documentation, it's not intuitive that it would consider some values of typeHtml.Attributeand silently ignore others, so this will likely lead to some surprising outcomes. It's also not hard to imagine that in this world it would also become best practice to recommend using plain queries instead ofQuery.contains. - Remove
Query.containsfrom the API. Maybe it is a footgun that seems like it will save your team time, but is actually likely to end up costing your team time overall due to its surprising behavior around event handlers. Maybe it's better to have the only way to describe DOM structures be to use queries that were designed to be good at that.
Removing Query.contains from the API is the option that appeals to me most. In retrospect, think it was probably a mistake for me to have added it in in the first place.
I'd prefer 1. Why would it make it less valuable? If you're looking for a particular element, being able to create <li>Something</li> and search for that is very handy. I'd actually like some kind of fuzzy matching along these lines, like:
someView
|> contains a li with the text "Something"
where <li>Something</li> and <li data-blah="fish">Something</li> both match
@eeue56 Fortunately, that use case can be covered by containing! (PR: https://github.com/eeue56/elm-html-test/pull/60)
Query.findAll
[ tag "li"
, containing [ text "Something" ]
]
containing also covers the use case of #53, so I think it makes contains obsolete!
Thoughts?
containing also covers the use case of #53, so I think it makes contains obsolete!
Yes! I think in general it would be nice to have less query functions and more selectors.