Cannot spy on browser Notification constructor function

[bahmutov]

Cypress v3.8.1
Reproducible example in https://github.com/cypress-io/cypress-test-tiny/tree/spy-on-constructor

This test breaks with the error below

it('spies on Notification', () => {
  cy.spy(window, 'Notification').as('Notification')
  const f = new Notification('Hello')
})

Note that spying on "regular" constructor functions works

function Foo () {
  if (!new.target) {
    console.log('Foo was called without new!')
    return
  }
  console.log('inside Foo constructor')
}
window.Foo = Foo

it('stubs the constructor', () => {
  cy.stub(window, 'Foo')
  const f = new window.Foo()
  // new automatically converts whatever stub returns into an empty object
  expect(f).to.deep.equal({})
})

and if we wrap Notification with our function that calls new then notifications are working

// workaround - spy on our wrapper of Notification
it('spies on Notification wrapper', () => {
  window.BrowserNotification = (text) => {
    return new Notification(text)
  }
  cy.spy(window, 'BrowserNotification').as('Notification')
  // actual notification will pop up (if Cypress notifications are enabled)
  const f = new BrowserNotification('Hello')
  cy.get('@Notification').should('have.been.calledOnce')
    .and('calledWithNew')
})

So this seems like sinon.js problem in this case, but still very weird behavior

[bahmutov]

The place in Sinon.js in our code that calls the function it spies on

[jennifer-shehane]

This is fixed in our 4.0 Cypress branch, which includes a Sinon upgrade. #4226

@bahmutov Is it safe to close this as part of that release?

[bahmutov]

can this test be one of e2e tests in 4.0 branch?

it('spies on Notification', () => {
  cy.spy(window, 'Notification').as('Notification')
  const f = new Notification('Hello')
  cy.get('@Notification').should('have.been.calledWith', 'Hello')
})

Then I will feel safe :)

[jennifer-shehane]

Hmm, I'm a bit unsure of where a test like this should go. @chrisbreiding Do we have any similar tests like this in our repo? Any idea where a test like this would go?

[chrisbreiding]

I think it could go along with the other cy.spy() tests in the driver.

[flotwig]

Found the original Sinon issue that was fixed in 4.1.3: sinonjs/sinon#1265

[flotwig]

So... the spying works, but when Cypress goes to serialize the spy for printing, Sinon has a bug that causes it to throw an error. 😄

I opened a bug report here: sinonjs/sinon#2215 Might open a PR, since it looks like a simple fix.

[flotwig]

Opened a PR over there to address the issue: sinonjs/sinon#2216

Once that gets merged and we update sinon, spying on ALL constructors using cy.spy will be gravy. For now, certain constructors (like Notification) will only work via Cypress.sinon.spy, since that prevents the toString call.

[cypress-bot]

The code for this is done in cypress-io/cypress#6310, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

[flotwig]

Sinon will release the fix in 9.0.0. Not sure why this issue has been left open... D: