Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Chrome execution/calling scope differs to Electron, and when running Chrome tests over CLI compared to the Cypress GUI test runner #2616

Closed
GrayedFox opened this issue Oct 16, 2018 · 3 comments

Comments

@GrayedFox
Copy link

GrayedFox commented Oct 16, 2018

Current behavior:

The Electron and Chrome execution context are different, resulting in inconsistent test output. You can easily show this by promisifying a few functions and using arrow functions to ensure they are lexical in scope, and observe how the expect() function behaves inside your test files.

I've also now confirmed and can consistently reproduce differing execution/calling scope from tests initiated with yarn cypress run --browser chrome --spec 'cypress/integration/app/**/*' as opposed to running it manually using yarn cypress open.

Edit: I've confirmed this bug also occurs when installing and running tests using NPM.

Desired behaviour:

I would expect both the Electron and Chrome browsers to behave the same way, and execute functions from within the same execution context. I would also expect Chrome to behave the same way no matter if it is run using the CLI or the native Cypress GUI test runner.

Steps to reproduce:

// commands.js
const { formatUrl } = require('./helpers.js')

const testEnv = `${Cypress.env('TEST_ENV')}`
const env = Cypress.env(testEnv)

const loginTypes = {
  installer: {
    body: {
      auth: {
        email: env.installer.email,
        password: env.installer.password
      }
    },
    url: `${env.apiUrl}/user_token`,
    form: false
  },
  manager: {
    body: {
      manager: {
        email: env.manager.email,
        password: env.manager.password
      }
    },
    url: `${env.baseUrl}/managers/sign_in`,
    form: true
  }
}

// unexposed login method (not available as a Cypress custom command)
const logIn = (client) => {
  return new Promise( (resolve) => {
    cy.request({
      method: 'POST',
      url: client.url,
      form: client.form,
      body: client.body
    }).then( (response) => resolve(response) )
  })
}

Cypress.Commands.add('login', (userType) => {
  const client = loginTypes[userType]
  logIn(client)
})

Cypress.Commands.add('api', (userType, method, resource, id, options, payload) => {
  // allow skipping the id by shifting options and payload down
  if (typeof id === 'object' && !payload) {
    payload = options
    options = id
    id = ''
  }

  logIn(loginTypes[userType]).then( (response) => {
    const body = payload

    let params = ''
    let headers = {
      accept: 'application/vnd.api+json',
      origin: env.baseUrl
    }

    if (id) {
      id = '/' + id
    }

    if (options) {
      params = '?' + formatUrl(options)
    }

    if (userType === 'installer') {
      headers.authorization = response.body.jwt
    }

    const url = `${env.apiUrl}/${resource}${id}${params}`

    cy.request({
      method,
      url,
      headers,
      body,
      failOnStatusCode: false
    })
  })
})
// fixtures/payloads/post-meterable-bookings.json
{
  "data": {
  "type": "possible-meterable-bookings",
  "attributes": {
    "identifier": "12346578",
    "meter-model-key": "heizkostenverteiler_qundis_whe460_wmbus",
    "validate-only": true
  },
  "relationships": {
    "user": {
      "data": null
      }
    }
  },
  "meta": {}
}
// integration/api/possible-meterable-bookings.js
const fixtures = 'cypress/fixtures'

// all of these tests relate to the /possible-meterable-bookings resource
describe('/possible-meterable-bookings', () => {
  it('books a meter to no installer (device pool) as a manager', () => {
    const options = { include: 'user,meterable' }

    cy.readFile(`${fixtures}/payloads/post-meterable-bookings.json`).then( (payload) => {
      cy.api('manager', 'POST', 'possible-meterable-bookings', options, payload)
        .then( (response) => {
          expect(response.status).to.eq(201)
          cy.writeFile(`${fixtures}/get-possible-meterable-bookings.json`, response.body)
        })
    })
  })
})

First: create a backend that returns a 200 when logging in, and that returns a 201 after a successful POST request

  1. Execute the test using Chrome (yarn cypress open) to see the test pass: expect method is working with the response of the API call, not the login request, and so a 201 is expected

  2. Execute the test using Electron to see the test fail: expect method is working with the response of the logon request (return of logIn(client).then( response => resolve(response))), and so the test fails since the success response of the logon request is a 200, but a 201 is expected.

  3. Execute the test using the CLI, by doing yarn cypress run --browser chrome --spec 'cypress/integration/api/possible-meterable-bookings.js' to see the test fail for the same reason: the expect method is working with the response of the logon request which is a 200, but a 201 is expected.

The attached GIF shows the above running code against our backend with the only change being switching from Chrome to Electron.

different-execution-context-chrome-electron

Versions

Cypress package version: 3.1.0
Cypress binary version: 3.1.0
Electron 59
Chrome 69

@jennifer-shehane jennifer-shehane added the stage: needs investigating Someone from Cypress needs to look at this label Oct 17, 2018
@GrayedFox GrayedFox changed the title Unobvious and inconsistent binding of execution context when running in Chrome compared to Electron [Bug] Chrome function execution/calling scope differs to Electron and when running from CLI Feb 18, 2019
@GrayedFox GrayedFox changed the title [Bug] Chrome function execution/calling scope differs to Electron and when running from CLI [Bug] Chrome execution/calling scope differs to Electron, and when running Chrome tests over CLI compared to the Cypress GUI test runner Feb 18, 2019
@GrayedFox
Copy link
Author

GrayedFox commented Feb 18, 2019

I just wanted to confirm that this bug is still happening using the following setup:

Cypress package version: 3.1.5
Cypress binary version: 3.1.5
Electron 59
Chrome 72

I have also now confirmed and can trigger this bug even without using Electron, but simply from launching Chrome via yarn cypress run --browser chrome --spec 'cypress/integration/api/possible-meterable-bookings.js. I have updated the original post to reflect this.

This strongly suggests that this bug resides in the way Cypress handles callbacks and promises, and not with Chrome or Electron itself.

Due to how we want to test our apps (doing API tests which generate mocks which we use later in all integration specs, something that also fits in with Cypress.io's own best practise advice) this bug severely impacts us: we are forced to run all tests inside Chrome (which we cannot run headlessly) instead of Electron and we cannot generate all the mocks we need based on API calls.

Due to our heavy reliance on client-side rendering, a small change to the JSON API response (or request params) can completely change the GUI. The best way to keep our tests accurate and reliable but still use mocks is to generate mocks with each run, which we want to achieve by hitting a real backend with cy.request() first.

Please consider looking into this bug, it has been 100% reproducible for some time, one need only use the above code against a bare-bones backend to see this is true.

@GrayedFox
Copy link
Author

GrayedFox commented Feb 18, 2019

Oh.... my god. I have finally resolved this.

The problem was that I didn't quite realise (or somehow forgot - thanks brain!) that I could call custom Cypress commands form within another Cypress custom command. So what I did was create a private function (logIn(client)) which was used by multiple Cypress custom commands (cy.api and cy.login).

This tricks the Cypress test runner and gets around the whole "don't try and wrap a return around promises in our already promisified custom commands error" thing, and so tests are run, but under the hood things obviously get messy.

For anyone else struggling with this, here is the fixed code, now working in both Electron and Chrome, headlessly, and from the Cypress GUI test runner:

// annoying ass superfluous broken code: calling cy.login from inside cy.api works just fine, no need to wrap result in a promise either.
// const logIn = (client) => {
//   return new Promise( (resolve) => {
//     cy.request({
//       method: 'POST',
//       url: client.url,
//       form: client.form,
//       body: client.body
//     }).then( (response) => resolve(response) )
//   })
// }

// am now calling this from within the Cypress.Commands.add('api') custom command with no issue
Cypress.Commands.add('login', (userType) => {
  const client = loginTypes[userType]
  cy.request({
    method: 'POST',
    url: client.url,
    form: client.form,
    body: client.body
  })
})

Cypress.Commands.add('api', (userType, method, resource, id, options, payload) => {
  // allow skipping the id by shifting options and payload down
  if (typeof id === 'object' && !payload) {
    payload = options
    options = id
    id = ''
  }

  cy.login(userType).then( (response) => {
    ...

@GrayedFox
Copy link
Author

@jennifer-shehane @Ivan-Cristian @Strajk see above 🤦‍♂️

@jennifer-shehane jennifer-shehane removed the stage: needs investigating Someone from Cypress needs to look at this label Feb 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants