Sinon: error message for assert.calledWith breaks when representations are equal

Created on 20 Sep 2019  Â·  6Comments  Â·  Source: sinonjs/sinon

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
Easy Help wanted Improvement hacktoberfest pinned

All 6 comments

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

Was this page helpful?
0 / 5 - 0 ratings