cypress-io / cypress

Fast, easy and reliable testing for anything that runs in a browser.
https://cypress.io
MIT License
47.02k stars 3.18k forks source link

Sinon-Chai Thrown assertion not working as expected #24120

Closed Sporradik closed 2 years ago

Sporradik commented 2 years ago

Current behavior

I have worked a bunch with spies and calledOnce, etc. I tried to use expect(spy).to.have.thrown and it does not seem to be working as I would expect based on the documentation. Am I using this correctly? It essentially gives me the same message if I pass in undefined or a string.

Screen Shot 2022-10-03 at 6 02 05 PM
describe.only('test throw', function() {
    it('should throw', function () {
        const obj = {
            method: () => {
                throw new Error('error')
            }
        }
        const spy = cy.spy(obj, 'method')
        try {
            obj.method()
        } catch (e) {
            console.error(e)
        }
        expect(spy).to.have.thrown(Error)
    })
})
Screen Shot 2022-10-03 at 6 01 53 PM

Desired behavior

I would expect the above code to pass the expect assertion. (However if this is wrong, I would like to understand the proper method)

Test code to reproduce

describe('test throw', function() {
    it('should throw', function () {
        const obj = {
            method: () => {
                throw new Error('error')
            }
        }
        const spy = cy.spy(obj, 'method')
        try {
            obj.method()
        } catch (e) {
            console.error(e)
        }
        expect(spy).to.have.thrown(Error)
    })
})

Cypress Version

10.7.0

Node version

12.22.12

Operating System

macOs 12.2.1

Debug Logs

cypress:server:util:socket_allowed allowed socket closed, removing { localPort: 59086 } +10s
GET /__cypress/iframes//Users/dace/momentum/extension/test/unit/dataSync/requestQueueSpec.js 304 2.748 ms - -
  cypress:server:routes-ct proxying to /__cypress/src, originalUrl /__cypress/src/main.js +10s
GET /__cypress/src/main.js 304 2.506 ms - -
  cypress:server:server-base Got CONNECT request from 127.0.0.1:8080 +11s
  cypress:https-proxy Writing browserSocket connection headers { url: '127.0.0.1:8080', headLength: 0, headers: { host: '127.0.0.1:8080', 'proxy-connection': 'keep-alive', 'user-agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/105.0.0.0 Safari/537.36' } } +11s
  cypress:https-proxy Got first head bytes { url: '127.0.0.1:8080', head: 'GET /ws HTTP/1.1\r\nHost: 127.0.0.1:8080\r\nConnection: Upgrade\r\nPra' } +1ms
  cypress:https-proxy Making intercepted connection to 58940 +0ms
  cypress:network:connect successfully connected { opts: { port: 58940, host: 'localhost', getDelayMsForRetry: [Function: getDelayForRetry] }, iteration: 0 } +11s
  cypress:https-proxy received upstreamSocket callback for request { port: 58940, hostname: 'localhost', err: undefined } +0ms
  cypress:server:util:socket_allowed allowing socket { localPort: 59108 } +279ms
  cypress:server:server-base Got UPGRADE request from /ws +1ms
  cypress:network:cors Parsed URL { port: '8080', tld: '127.0.0.1', domain: '' } +11s
  cypress:network:agent addRequest called { isHttps: false, href: 'http://127.0.0.1:8080/ws' } +11s
  cypress:network:agent got family { family: 4, href: 'http://127.0.0.1:8080/ws' } +0ms
  cypress:server:routes-ct proxying to /__cypress/src, originalUrl /__cypress/src/vendors-node_modules_cypress_code-coverage_support_js.js +27ms
  cypress:server:routes-ct proxying to /__cypress/src, originalUrl /__cypress/src/test_support_index_js.js +0ms
GET /__cypress/src/test_support_index_js.js 304 1.929 ms - -
GET /__cypress/src/vendors-node_modules_cypress_code-coverage_support_js.js 304 2.857 ms - -
  cypress:server:routes-ct proxying to /__cypress/src, originalUrl /__cypress/src/vendors-node_modules_dexie_dist_modern_dexie_mjs.js +51ms
  cypress:server:routes-ct proxying to /__cypress/src, originalUrl /__cypress/src/spec-1.js +0ms
GET /__cypress/src/spec-1.js 304 1.813 ms - -
GET /__cypress/src/vendors-node_modules_dexie_dist_modern_dexie_mjs.js 304 3.049 ms - -
  cypress:server:socket-base automation:request get:cookies { domain: 'localhost' } +10s
  cypress:server:automation:cookies getting:cookies { domain: 'localhost' } +10s
  cypress:server:automation:cookies received get:cookies [] +0ms
  cypress:server:socket-base backend:request { eventName: 'reset:server:state', args: [] } +9ms
  cypress:proxy:http:util:buffers resetting buffers +10s
  cypress:server:remote-states resetting remote state +10s


### Other

If this is an issue with sinon-chai and not cypress related, my apologies, I can follow up with them
astone123 commented 2 years ago

@Sporradik I think the issue here is timing. What we can do is use cy.wrap to wrap the call to obj.method() so that we can assert after this call finishes, like this:

    try {
      cy.wrap(obj.method()).then(() => {
        expect(spy).to.have.thrown(Error)
      })
    } catch (e) {
      console.error(e)
    }

This should work for asserting that method throws an error when we call it.

Sporradik commented 2 years ago

@astone123, Does this work for you? If I do this, then the .then callback does not run because obj.method() throws an error.

This example I posted is an overly simplified version of what I am trying to do in my real test. In the real test the method I am spying is called within the method I am calling in the test.

Both in my simplified example and in my real test, the assertion does run after the method was called, as you can see by the message in the error screenshot I posted above. It says "The following calls were made..."

warrensplayer commented 2 years ago

@Sporradik The parameter for the expect(...).to.have.thrown(<here>) can be undefined, an object, or a string. For your original test above, passing in Error is asking the assertion to check that it threw that specific object instance, which it is not. Instead, you can pass in the string 'Error' to assert that it throws an error of type Error. Below are a few other examples to look at.

describe.only('test throw', function () {
  it('should throw', function () {
    const obj = {
      method: () => {
        throw new Error('error')
      }
    }
    const spy = cy.spy(obj, 'method')
    try {
      obj.method()
    } catch (e) {
      console.error(e)
    }
    expect(spy).to.have.thrown('Error')
  })
  it('should throw anything', function () {
    const obj = {
      method: () => {
        throw new Error('error')
      }
    }
    const spy = cy.spy(obj, 'method')
    try {
      obj.method()
    } catch (e) {
      console.error(e)
    }
    expect(spy).to.have.thrown()
  })
  it('should throw specific error', function () {
    const error = new Error('my error')
    const obj = {
      method: () => {
        throw error
      }
    }
    const spy = cy.spy(obj, 'method')
    try {
      obj.method()
    } catch (e) {
      console.error(e)
    }
    expect(spy).to.have.thrown(error)
  })

  class ValidationError extends Error {
    constructor(message) {
      super(message);
      this.name = "ValidationError";
    }
  }

  it('should throw', function () {
    const obj = {
      method: () => {
        throw new ValidationError('not valid')
      }
    }
    const spy = cy.spy(obj, 'method')
    try {
      obj.method()
    } catch (e) {
      console.error(e)
    }
    expect(spy).to.have.thrown('ValidationError')
  })
})
Sporradik commented 2 years ago

Thank you so much @warrensplayer, this is exactly what i was looking for!