Closed Bug 976935 Opened 8 years ago Closed 8 years ago

Sinon isn't smart enough to spy on WebIDL constructors

Categories

(Firefox OS Graveyard :: Gaia, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bholley, Unassigned)

References

Details

Attachments

(1 file)

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?
Flags: needinfo?(anygregor)
We should use this.sinon.stub(window, 'Notification') because there is no reason to actually call the real Notification constructor in unit tests.
(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.
Flags: needinfo?(anygregor)
Attachment #8383405 - Flags: review?(felash)
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
Attachment #8383405 - Flags: review?(felash) → review+
Got a green travis, merging!

master: 10e1e91d3c9b365a4b14498826263ce11592d924
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.