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

Add events #15

Closed rogeriochaves closed 7 years ago

rogeriochaves commented 7 years ago

Hello,

I was trying to add a way to trigger events and check its Msg for elm-html-test, the reasons for that are:

1 - Be able to unit test what messages an html element is producing 2 - (most important for me) being able to later on elm-testable (rewrite-native branch) write a full integration test like this:

webSocketsComponent
    |> startForTest
    |> find [ tag "input" ]
    |> trigger "input" "{\"target\": {\"value\": \"dogs\"}}"
    |> find [ tag "button" ]
    |> trigger "click" "{}"
    |> assertCalled (WebSocket.send WebSockets.echoServer "dogs")

So, to test that an Elm app is working properly and functions are fully integrated I don't want to manage the messages and updates myself, I just want to simulate events around the screen and check that the right places changed, or that some external data was sent (like this websocket)

This is how I test things in CaronaBoard (https://github.com/CaronaBoard/caronaboard/tree/master/tests/Integration) and it has been a great way to test IMHO. But for that I use my own fork of elm-testable, I want to switch to elm-testable#rewrite-native and deprecate my fork, but I need this feature first.

So, my idea was for elm-html-test to just return the Msg from a triggered component, and elm-testable could use those messages to update the state of the app.

I got it working just fine with onClick, but then I had two problems:

1 - On parsing, Html.Map is ignored, so the Msgs do not get mapped, I believe this issue is not a blocker, since not everyone uses Html.Map, or we can manually remap on the test for now 2 - Since we stringify the HTML, the functions are stripped off, this was intentional so far, but now we need the functions if we want the trigger to work with onInput too, not only clicks

Number 2 is a more profound change, so idk if you have more ideas with taking this forward, or if you want to.

Meanwhile, I was trying to hack the html to ElmHtml parsing directly on native, rather than using Json.Decode (which requires us to strip functions)

Cheers!

rogeriochaves commented 7 years ago

Turns out I've managed to fix problem 2 with a simple change: by returning a Json.Encode.Value rather than a String from the toJson native function. It also made things a lot easier.

rtfeldman commented 7 years ago

Whoa, this is incredible! 😮 😮 😮

rtfeldman commented 7 years ago

Note: test failure as of d01db1356a60ff4120a745ec78dfec06804cdae9 is from master (by way of https://github.com/eeue56/elm-html-test/pull/11)

rogeriochaves commented 7 years ago

@rtfeldman thanks for the awesome reviews! Feeling much better about the code now :D

I have to dig more about Html.map, if anybody with more knowledge on this could help it would be great. But, I believe this could be fixed later without changing the API, so it shouldn't be a blocker.

rogeriochaves commented 7 years ago

@eeue56 @rtfeldman found a solution to the Html.Map problem, but this required more changes on elm-html-in-elm, I've opened the PR #7 there with more details, also check it out the latest commit here. The tests here will now fail cause that PR needs to be merged and published first.

I'll probably need to also update elm-html-query dependencies before this.

But after those I believe we will be done here as well :-)

eeue56 commented 7 years ago

I've improved the errors quite a bit, and merged this

rtfeldman commented 7 years ago

😻 😻 😻

rogeriochaves commented 7 years ago

Cool, that looks beautiful!

captura de pantalla 2017-05-02 a la s 11 44 47

BUT, my original idea, the reason why I've not added expectEvent was because I wanted to use it on elm-testable on rewrite-native branch, to be able to extract the event generated by a node, send it to the update function, recalculate the html and make assertions on the html, not the event. Triggering multiple events should also be possible.

How should we proceed with this? Create another function that instead of returning an expectation just return a Result String Msg, like simulate did before? (downside is forcing the users to nicely output the error message themselves by handling Result)

I've though of maybe moving this logic to elm-testable itself, but I believe there is no way of doing this without ending up reimplementing the whole elm-html-test there. There is also no function that returns the found node, only Expectation.