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):
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.
@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.
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.
Seems like this is still an issue?
Can I reopen a pull request for this, most of the work has actually already been done by @rgroothuijsen.
This has been fixed with #2303