-
-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update to elm-test 2.0 #96
Conversation
[ test "[]] Accumulate" | ||
(assertEqual [] (accumulate square [])) | ||
(\() -> Expect.equal [] (accumulate square [])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this syntax is the convention the Elm Test community are trying to promote:
test "square Accumulate" <|
\() ->
accumulate square [ 1, 2, 3 ]
|> Expect.equal [ 1, 4, 9 ]
I recognise this is probably hard to change without lots of busy work though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... To be honest I was able to do 99% of this via regex flavored find/replace because I was keeping the existing closing paren at the end of each test. Switching to that style will require a bit more work. That's really my strongest objection :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should do it, but perhaps it's something we can leave for a later pull request, or just do incrementally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, you got me. I figured how to get 95% of it in bulk, and elm-format helped me sort out the rest pretty quickly.
I like where this is going so far :) Do we run Elm Format on the exercises? |
Not in CI. There's an issue for that (#47), but no one has bothered to try installing it in Travis yet. |
Anyone have any objections to merging this (squashed, etc...)? @lpil @parkerl @lukewestby |
Looks great! Thank you for doing all this work. |
This is an example of what an exercise upgraded to elm-test 2.0 looks like (fixes #95). Among other highlights, I've reenabled the node elm-test runner workflow (
npm install -g elm-test
) as before, so the output is much nicer and by ditching the runtests scripts we can close #76.