elm-html-test
elm-html-test copied to clipboard
Query.has matches children, not just root element
Query.has is documented as follows:
Expect the element to match all of the given selectors. http://package.elm-lang.org/packages/eeue56/elm-html-test/5.1.0/Test-Html-Query#has
Here's the example given:
test "The list has both the classes 'items' and 'active'" <|
\() ->
div []
[ ul [ class "items active" ]
[ li [] [ text "first item" ]
, li [] [ text "second item" ]
, li [] [ text "third item" ]
]
]
|> Query.fromHtml
|> Query.find [ tag "ul" ]
|> Query.has [ tag "ul", classes [ "items", "active" ] ]
https://github.com/eeue56/elm-html-test/blob/5.1.0/src/Test/Html/Query.elm#L394-L405
Based on this example, it sounds like it's making assertions against the node itself, but in practice, it looks like it traverses all of the node's children. Is this expected behavior? If so, perhaps the docs could be clearer.
Here's an example test that I'm not sure should fail or pass (it does pass currently):
test "Query.has doesn't consider children" <|
\() ->
div []
[ div []
[ span [ class "nested-span" ]
[ text "hello" ]
]
]
|> Query.fromHtml
|> Query.has [ Selector.class "nested-span" ]
@stoeffel and I ran up against this earlier, as this behavior causes the test used as documentation for Query.children
to be a false positive. Children was returning the ul
, and the test was still passing because there were li
elements within the ul
:
test "The <ul> only has <li> children" <|
\() ->
div []
[ ul [ class "items active" ]
[ li [] [ text "first item" ]
, li [] [ text "second item" ]
, li [] [ text "third item" ]
]
]
|> Query.fromHtml
|> Query.find [ tag "ul" ]
|> Query.children []
|> Query.each (Query.has [ tag "li" ])
http://package.elm-lang.org/packages/eeue56/elm-html-test/5.1.0/Test-Html-Query#children
Also, it's not clear whether it enforces that a single element must match all the selectors, as this version of the example also passes:
test "The list has both the classes 'items' and 'active'" <|
\() ->
div []
[ ul []
[ li [] [ text "first item" ]
, li [] [ text "second item" ]
, li [ class "items active" ] [ text "third item" ]
]
]
|> Query.fromHtml
|> Query.find [ tag "ul" ]
|> Query.has [ tag "ul", classes [ "items", "active" ] ]
So I think we should probably add is
and isNot
, which match the root node. Then has
and hasNot
are for children only. What do we think? @rtfeldman
Based on this example, it sounds like it's making assertions against the node itself, but in practice, it looks like it traverses all of the node's children. Is this expected behavior?
I think this is unexpected, and a bug.
I think it'll be nicest if it works like this:
- Make it so that
has
only applies to the root node. - If you explicitly want to involve children or descendants, do a
Query.children
/Query.find
/Query.findAll
to get them, and then useQuery.each (Query.has [ ... ])
This keeps the API the same size as it is today, and supports either behavior.
This would be a breaking bugfix though. I think it might be too surprising for a patch release.
Here's what I think we should do:
- Change
has
andhasNot
to work this way. - Clarify the documentation so it's more apparent this is how it works.
- Wait until the next major release to publish the change.
Thoughts?