.toEqual()

[aaronabramov]

[cpojer]

As previously discussed, toEqual must be compatible with jasmine 1's version of the same for the time being. The two changes we made in Jasmine 2 to relax the matcher are:

• Do not check for constructor name equality. This one is stupid and I don't want it, it just makes it harder to write tests and people don't care about this. People should care about this in their type-checker but not in a test file.
• Skip undefined fields in both a and b. This one I somewhat think we should fix, but unless you go and do the work on www I see no way of shipping this.

Alternatively we can run a huge codemod using Jest inside of a jscodeshift transform to codemod the failing tests to a new matcher that matches the current implementation of toEqual. I'm not really ready to do it and would like us to focus on pretty-printing the error output first; then make these large changes later.

Another thing I want to bring up is whether you thought about not printing either objects in red. My biggest problem in a sentence like Expected <loads of gibberish> to equal <more gibberish> is always to find where the first object ends and the second one starts. Can we print the two objects in black and the rest of the sentence (to equal and Expected etc.) in red? What do you think?

Finally, if you can find the fastest toEqual implementation on the planet and use that, I would be happy. It is the most used matcher by far and we run some pretty huge objects (in ads code) through it.

[gaearon]

Let's invert the colors.
Like here:

I'm always confused by "actual" being green in the failing test. The whole point is that actual value is the wrong one. So it should be red.

[ForbesLindesay]

I'm confused when it doesn't match what I'd see in a diff on GitHub. i.e. when actual is red. It's unfortunate that we have the same colour conventions for good/bad and added/deleted.

[aaronabramov]

i don't like the actual/expected idea at all. every time i see the diff i spend a few seconds trying to figure out what is actual and what is expected.

also the fact that actual value is passed into the function that is called expect naturally confuses the human brain

expect(actual).toEqual(expected)

my thoughts on this were to get rid of these terms completely and replace them with:
added/missing instead. this way it's very clear what should be green and what should be red

[ForbesLindesay]

I was discussing this with @cpojer and he said something very similar about expected vs. actual. I've never seen this as confusing, but that may just be a privilege of being a native speaker 😄

added/missing sounds like a good idea for diffs. For tests that aren't diff maybe s/expected/correctValue/ and s/actual/observedValue/?

[cpojer]

Let's have this discussion separately from this diff which is about pretty-printing the values. I care more about changing the colors around so that when printing two objects I can still see whether it says "to equal" or "to be". I can do that in a separate diff.

For actual/expected, I think we need to have a bit more of a discussion. I'll think about it.

[gaearon]

I'm confused when it doesn't match what I'd see in a diff on GitHub. i.e. when actual is red. It's unfortunate that we have the same colour conventions for good/bad and added/deleted.

I think GH logic doesn’t quite apply there. You’re not really “starting” from the expected value and then changing your app to output the wrong thing.

I can imagine that when you start thinking about it, it might feel incorrect, but if you just glance without thinking or if you are drunk, I think “red is bad” feels much more intuitive.

[quantizor]

I think green doesn't have a place in this context. Should be red and white, imo. Red being the erroneous value

[ForbesLindesay]

@yaycmyk: in reality there are 3 colours for object diffs:

1. text that is in the expected value but not the actual value.
2. text that is in the actual value but not the expected value.
3. text that is the same in both the actual value and expected value.

The debate is which way round the colour coding is. The alternative would be to choose two completely unrelated colours (e.g. yellow and blue) so you avoid having any intuitive expectations.

@gaearon I think we may always disagree on this point. The way I see it, especially with snapshot testing, is that the first run of the test creates the snapshot, so every subsequent test should be showing me what's changed from that snapshot. I suspect that if you were to run polls for what the most popular answer is, you could get people to prefer either answer with subtly different wording of the question.

[gaearon]

The way I see it, especially with snapshot testing, is that the first run of the test creates the snapshot, so every subsequent test should be showing me what's changed from that snapshot

Oh, I can see how this can be confusing.
I was thinking only about generic toEqual, not about snapshot testing.

[gaearon]

I think that snapshot testing should use diff coloring. This makes total sense because you interpret it as a diff which you can review.

red = old
green = new

However I also think that regular assertions don’t need to be like diffs because most of the time (hopefully) tests will be stable, and regular code will need to be adjusted per tests. So I think this color scheme makes more sense for regular assertions:

red = actual
green = expected

I just wanted object diffs (#1439 (comment)) to follow the same convention.

[cpojer]

I'll send a new PR that uses jasmine's toEqual matcher for now and then another PR on top of that to polish all the messages and their colors.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.