elwayman02 / ember-sinon-qunit

Sinon sandbox test integration for QUnit
MIT License
58 stars 30 forks source link

sinon.match support #12

Closed Bockit closed 8 years ago

Bockit commented 8 years ago

Hi,

Love the package! Was just wondering if there's a reason sinon.match isn't being added as a property? Happy to make a PR if it's just oversight.

Thanks for your time.

elwayman02 commented 8 years ago

@Bockit yea, there's no specific reason for that, probably an oversight. I think I was just using the example from sinon-qunit when setting up the config. I would more than welcome a PR for this!

elwayman02 commented 8 years ago

@Bockit upon further investigation, I'm not sure if the Sandbox API supports match. Can you please try adding this property and add a test that makes sure it's working? If so, I'm happy to merge that PR.

sukima commented 8 years ago

You can still import sinon from 'sinon' and have sinon.match.any() work just fine. Great for sinon.assert.called() APIs because the sinon qunit uses assert under the hood and provides a message.

// => expected spy to be called with arguments typeOf("string")
sinon.assert.calledWith(this.spy(), sinon.match.string);
// VS.
assert.ok(this.spy().calledWith(sinon.match.string), 'expected the spy to have been called with a string.'); 
elwayman02 commented 8 years ago

:+1: perhaps that's the best solution unless you want to submit a PR to Sinon to add match to sandboxes @Bockit. Closing for now, feel free to re-open if you come up with new ideas on how to implement this.

elwayman02 commented 8 years ago

That's awesome! Do you want to attempt a PR?

On Fri, Feb 12, 2016, 8:10 PM Devin Weaver notifications@github.com wrote:

I just looked into the source https://github.com/sinonjs/sinon/blob/ac12b30f58db1f9014ae7c6f6ca235dcdabaf732/lib/sinon/assert.js#L135-L167 and it seems the sinon.assert.expose() method will graph the assert API onto this and also the match API. Two birds with one stone!

— Reply to this email directly or view it on GitHub https://github.com/elwayman02/ember-sinon-qunit/issues/12#issuecomment-183583716 .

sukima commented 8 years ago

@elwayman02 Tried, didn't work as expected, It added the assert API but not the matchers:

this.assertCalledWith(mySpy, sinon.match(myExpectedObj)); // Worked
this.assertCalledWith(mySpy, this.assertMatch(myExpectedObj)); // Did not work

It may still be worth exposing the assert API. But it seems one still needs the sinon global for using matchers.