Sinon: spy.calledWithMatch fails for objects containing a Symbol as value.

[havardh]

Sinon version : 1.17.3 Environment : OSX Chrome 48.0.2564.116 Example URL : http://jsbin.com/qeloruvupi/edit?js,console

Bug Description

Calling a spy with an object containing a Symbol as value and then calling calledWithMatch on the spy with a similar object makes Object.sinon.match throw a TypeError.

TypeError: Cannot convert a Symbol value to a string

[fatso83]

This is quite an easy fix, but we are focused on getting Sinon 2 out the door these days. So if you can spare the time to make a PR, we would appreciate it :)

[havardh]

Yes, fixing the error at hand is quite easy.

My take on it, is to call the String() function on the value before it is appended to the string with the concat operator. This ensures that Symbols are explicitly converted to strings before the concatenation and in addition support existing values, including undefined and null. This will avoid the coercion, which trigger the TypeError in my bug report.

While looking into this, I started to look into all the places where user supplied values are coerced into string values. This, on the other hand is a large change, but not technically hard. Are you interested in a PR to support Symbols in all applicable places, or just a fix to the bug I originally reported?

[fatso83]

I don't see why we would not want that, @mantoni? There is nothing stopping you, of course, from splitting the work in two PRs, if you want to pick the lower hanging fruit first.

[mroderick]

As long as the commits have good comments describing what problem is being solved and how, then I don't see any problem with putting them in the same PR, as they're quite related.

Two PRs are also welcome :)

[mantoni]

Sure. We should support this everywhere as it's a new built-in language type.

[ELLIOTTCABLE]

Are we sure this is fixed by #1014 / in 2.0.0-pre.4? Because I've installed that, and I'm still seeing test failures attributable to Symbols-in-objects.

$ npm install 'sinon@^2.0.0-pre.4'
$ npm run-script test

      1) giraphe ~ a walk function (class/keyer) ~ callbacks see rejected nodes in the `seen` argument:

      AssertionError:   # Tests/giraphe.tests.es6.js:266

  assert(spy.calledWith(bar))
         |   |          |    
         |   false      Node{}
         #function#          

      + expected - actual

Here's a failing example.

[fatso83]

@ELLIOTTCABLE There is nothing in that code that seems to apply to Symbols specifically, AFAI can see. The id attribute with the Symbol is not being used when passing in arguments, so Sinon is not touching it. I suspect you would get the same result using anything else. That is not to say that you are not looking at a Sinon bug, btw ;-) I wonder if this is an issue regarding withArgs that I have seen before.

[ELLIOTTCABLE]

@fatso83 I tracked it down to an unrelated bug — Sinon's inexplicable deepEqual-by-default. Cleaned it up, Symbols and all, by liberally sprinkling sinon.match.same() around my tests.