Make clickLink look for links with aria-label and alt text #168
Conversation
src/ProgramTest.elm
Outdated
| -- there is a click handler | ||
| -- first make sure the handler properly respects "Open in new tab", etc | ||
| if respondsTo ctrlClick link || respondsTo metaClick link then | ||
| if respondsTo normalClick then |
There was a problem hiding this comment.
I'm having a little bit of trouble convincing myself this refactoring is correct now that repondsTo doesn't take the found link element as an input. I guess it's that repondsTo is re-running the original query, just modified to do the simulate as the last step of each branch?
Apart from being a bit confusing to read, it makes me a little nervous about the performance if we're re-doing that work, since ComplexQuery can be quite slow (especially when a query fails).
Here's one possible thought: maybe we could extract a new function from simulateComplexQuery/assertComplexQuery:
runComplexQuery :
String
-> (ComplexQuery (Query.Single msg) -> ComplexQuery a)
-> (Program model msg effect (SimulatedSub msg) -> TestState model msg effect -> a -> Result Failure (TestState model msg effect))
-> ProgramTest model msg effect
-> ProgramTest model msg effect
simulateComplexQuery functionName complexQuery =
runComplexQuery functionName complexQuery <| \program state msg -> TestState.update msg program state
assertComplexQuery functionName complexQuery =
runComplexQuery functionName complexQuery <| _ state _ -> stateAnd then here we could use runComplexQuery with the findLinkQuery to get the single, and a function that will take the single and use it to "try clicking", and then we wouldn't have to re-find the single in each call of respondsTo?
There was a problem hiding this comment.
Good point! Not sure what I was thinking implementing it that way. I think it was running the query five times! I actually later came up with something along the lines of your suggestion independently. I've now implemented runComplexQuery (with a slight change of argument order) and pushed.
assertComplexQuery is not being used now. Not sure if it's worth keeping it around.
There was a problem hiding this comment.
Yeah, there have been many times on this project where I've implemented something, and then when I show it to someone they ask why I did it so complicated 😄
It's fine to leave assertComplexQuery for now, especially since I haven't finished reviewing if there are more functions that should be ported over to using ComplexQuery internally.
src/ProgramTest.elm
Outdated
| findLinkQuery = | ||
| [ ( "<a> with text" | ||
| , ComplexQuery.find (Just "find link") | ||
| [ "link" ] |
There was a problem hiding this comment.
If I remember correctly, this argument is a list of html tags that will be kept expanded when the ComplexQuery error renderer tries to condense down the HTML out. So this should be [ "a" ], not "link".
There was a problem hiding this comment.
Got it. I was wondering what this did.
| , test "can verify a link with aria-label" <| | ||
| \() -> | ||
| linkProgram | ||
| |> ProgramTest.clickLink "Aria" "https://example.com/link" | ||
| |> ProgramTest.expectPageChange "https://example.com/link" | ||
| , test "can verify a link with img and alt text" <| | ||
| \() -> | ||
| linkProgram | ||
| |> ProgramTest.clickLink "Alt Text" "https://example.com/link" | ||
| |> ProgramTest.expectPageChange "https://example.com/link" |
|
(I'll get to this tomorrow if I don't have time for it today.) |
|
Looks great, thank you!! |
2 participants
Hi @avh4. While I was on a roll I decided to have a look at #158, because I'm also running into this issue where I am using icons in links with an aria-label.
See what you think.
Closes #158
npm test