elm-explorations / test

Write unit and fuzz tests for Elm code.
https://package.elm-lang.org/packages/elm-explorations/test/latest
BSD 3-Clause "New" or "Revised" License
237 stars 39 forks source link

Improve Expect.equal's diff display #38

Open j-maas opened 6 years ago

j-maas commented 6 years ago

When viewing a diff, I frequently forget which one is the actual value and which the expected one.

There are two similar alternatives that are popular that both use the pipe operator in the display and that are both compatible with the reasoning that the current output is based on:

"first value"
╷
│ |> Expect.equal
╵
"second value"
"first value"
|> Expect.equal
"second value"

In order to play around with alternatives, check out the Ellie.

mgold commented 6 years ago

+1 for the caret and v form. [EDIT: Or @drathier's pipe form.]

drathier commented 6 years ago

This says that the first value is the test value and the second value is the expected value. That's doesn't have to be true. 1+1 |> Expect.equal 2 says nothing about which is expected and which is the test value.

If anything, maybe we could add a > to the pipe before Expect.equals, so it reads something like

"I am actual"
╷
│> Expect.equal
╵
"I am expected"

And maybe drop the small pipe extensions so it reads like a normal elm pipe?

j-maas commented 6 years ago

Both approaches are just conventions. You could document the first argument as being the one that is considered the expected and the second to be considered the actual. You could keep the convention of piping the actual into the function.
So actually, this problem seems to be deeper than I thought as it comes down to what convention should be officially supported.

Since @drathier's idea expresses the reasoning of the documentation, I think that is the best way to go for now. It's possible to decipher which argument is which in

"I am actual"

│> Expect.equal

"I am expected"

However, we might discuss the advantages and disadvantages of the expected/actual vs the piping convention. I personally prefer the expected/actual convention more, because it leads to the piping convention, whereas the piping convention still does not give guidance to which argument is where.
I opened a thread on the Elm Discourse.

drathier commented 6 years ago

The piping convention should map 1-1 to what your code looks like, if you use pipes. The first argument in the output is the first argument in your code.

Keep in mind that not all expectations are commutative. https://package.elm-lang.org/packages/project-fuzzball/test/latest/Expect#lessThan is one example. That makes the argument order, and pipe vs no pipe important.

If we use actual vs expected, I'd prefer to switch the api over to take a single record argument with two fields, like Expect.equal {expected=1, actual=a}. But then you have other problems, like remembering what to call each field, see Expect.lessThan {smaller=a, larger=14}. Expect.lessThan {expected=a, actual=14} makes no sense to me.

j-maas commented 6 years ago

@drathier, you actually convinced me that the expected/actual convention would basically be a breaking change, because the documentation states it's following the piping convention. So for the sake of this issue here on GitHub, I would just propose your idea to make the piping convention a bit more explicit by using e. g.

"I am actual"
╷
│ |> Expect.equal
╵
"I am expected"

This is a small change that might alleviate the confusion ever so slightly.

The discussion about whether the expected/actual convention is worth considering should be held at the Discourse thread, in my opinion.

drathier commented 6 years ago

If you feel like it, here's an emoji vote. React with your vote. 😕 to vote against all.

"first value"
╷
│ Expect.equal              👍 
╵
"second value"
"first value"
╷
│> Expect.equal             👎 
╵
"second value"
"first value"
╷
│ |> Expect.equal           😄 
╵
"second value"
"first value"
|> Expect.equal             🎉
"second value"
"first value"
╷\
│ 〉Expect.equal            ❤️
╵/
"second value"
gyzerok commented 6 years ago

Why use symbols for something which can be identified with words? What is the problem of explicitly writing “actual” and “expected” instead of mimicing pipe operator?

j-maas commented 6 years ago

@gyzerok, I would like to move the discussion to the Discourse thread. I think this GitHub issue is more valuable if we propose a "quickfix" that is consistent with the library, which means deciding on one of @drathier's proposals.

I hope I don't sound prescriptive or condescending... I would be glad to keep up the discussion! It's just about keeping this issue actionable, so I would appreciate it if we could move the discussion to Discourse. ;)

rtfeldman commented 6 years ago

EDIT: moved post to discourse!

j-maas commented 5 years ago

I finally started to try out the options with different examples in this Ellie.