error message for assert.calledWith breaks when representations are equal

[rensbaardman]

Describe the bug
When the (string) representations of the expected outcome and the actual outcome of e.g. assert.calledWith() are equal, the resulting error message is very confusing and hinting at a bug in the display logic. Instead of <red>actual</red> <green>expected</green> (with <color>...</color> my attempt at showing the expected terminal output color), it displays <black>actual</black>, as if the outcome is actually correct.

To Reproduce
Running the following with node:

const sinon = require('sinon')

mySpy = sinon.spy()
mySpy(1234)
sinon.assert.calledWith(mySpy, '1234')

generates this output:

        throw error;
        ^

AssertError: expected spy to be called with arguments 
1234
    at Object.fail [...]

Expected behavior
I expected something like this:

        throw error;
        ^

AssertError: expected spy to be called with arguments 
<red>1234</red> <green>1234</green>
    at Object.fail [...]

so <red>1234</red> <green>1234</green> instead of <black>1234</black>.

Or, as assert.match does it (run e.g. sinon.assert.match(1234, '1234')):

        throw error;
        ^

AssertError: expected value to match
    expected = 1234
    actual = 1234
    at Object.fail [...]

It would be even better if it recognizes when string representations are equal, since this remains a confusing way to display the error: on first sight, it seems the actual outcome is equal to expected outcome, when it is not! Or simply include apostrophes for strings.

Compare how Chai handles this:

const assert = require('chai').assert;
assert.strictEqual(1234, '1234')

throws:

AssertionError: expected 1234 to equal '1234'

Context (please complete the following information):

• Library version: 7.4.2
• Environment: macOS 10.11.6, Node v10.16.0

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[fatso83]

@rensbaardman Would you like to have a stab at fixing this? I think it's a one-line fix, preferably with a small regression test.

[rensbaardman]

Sorry for the late reply – I certainly want to take a look at it, but I am quite busy right now so can't promise a quick resolution.

[romanbalayan]

Seems like this is still an issue?

[romanbalayan]

Can I reopen a pull request for this, most of the work has actually already been done by @rgroothuijsen.

[mroderick]

This has been fixed with #2303