976935 - Sinon isn't smart enough to spy on WebIDL constructors

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Bobby Holley (:bholley)]

In gaia-unit, we have a test that creates a spy for the Notification constructor:

http://mxr.mozilla.org/gaia/source/apps/system/test/unit/voicemail_test.js#26

This doesn't work, because spies are just functions (and not proxies), and they can't distinguish between invoked and construct (i.e. whether the |new| keyword was used). So when the test does:

this.sinon.spy(window, 'Notification');

Future invocations of |new Notification(...)| will be converted by sinon.js into bareword (non-constructing) invocations of the constructor, which will cause us to throw post bug 916644.

I can think of a number of workarounds, but I'm no Gaia hacker. Gregor, how do you want to proceed here?

Comment 1

[Julien Wajsberg [:julienw]]

We should use this.sinon.stub(window, 'Notification') because there is no reason to actually call the real Notification constructor in unit tests.

Comment 2

[Bobby Holley (:bholley)]

(In reply to Julien Wajsberg [:julienw] from comment #1)
> We should use this.sinon.stub(window, 'Notification') because there is no
> reason to actually call the real Notification constructor in unit tests.

Sounds good. I'll give that a shot.

Comment 3

[Bobby Holley (:bholley)]

Attachment: Bug 976935 - Use a stub rather than a spy for the Notification constructor. v2

Comment 4

[Julien Wajsberg [:julienw]]

Comment on attachment 8383405 [details] [review]
Bug 976935 - Use a stub rather than a spy for the Notification constructor. v2

r=me but let's wait for a green travis :)

I'll merge it when it's ready

Comment 5

[Julien Wajsberg [:julienw]]

Got a green travis, merging!

master: 10e1e91d3c9b365a4b14498826263ce11592d924