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

Assertion on getCookie() is called twice in the Cypress runner #6672

Closed
abhidp opened this issue Mar 7, 2020 · 6 comments · Fixed by #8276
Closed

Assertion on getCookie() is called twice in the Cypress runner #6672

abhidp opened this issue Mar 7, 2020 · 6 comments · Fixed by #8276
Assignees
Labels
good first issue Good for newcomers pkg/driver This is due to an issue in the packages/driver directory type: regression A bug that didn't appear until a specific Cy version release v4.0.0 🐛 Issue present since 4.0.0

Comments

@abhidp
Copy link

abhidp commented Mar 7, 2020

Current behavior:

I am asserting cy.getCookie() only once in my code, but the Cypress runner shows the assertion happened twice. Though it is a very minor and low priority issue, it is not the most optimum as the logic for validating the cookie from cypress-runner.js is being called twice and adds to the execution time.

Desired behavior:

Cypress runner should show the ASSERT log only once for cy.getCookie('auth').should('exist')

Test code to reproduce

// cypress/integration/login.spec.js
it('should login successfully with correct credentials and get auth cookie', () => {
    cy.get('#loginForm').within(() => {
      cy.get('#email')
        .clear()
        .type('cypress@crate.com')
        .get('#password')
        .clear()
        .type('Pass1234');
      cy.contains('Login').click();
    });
    cy.title().should('eq', 'Crates for everyone! - Crate');
    cy.url().should('eq', `${Cypress.config().baseUrl}/crates`);
    cy.getCookie('auth').should('exist');
  });

image

Both the ASSERTs print exactly the same message from the same line number of the Cypress runner source code cypress-runner.js:168422 to the DevTools console

image

image

Versions

Cypress:

"devDependencies": {
    "cypress": "^4.1.0"
 }

OS:

ProductName:	Mac OS X
ProductVersion:	10.14.6

Browser:

Google Chrome is up to date
Version 80.0.3987.132 (Official Build) (64-bit)
@jennifer-shehane jennifer-shehane added the v4.0.0 🐛 Issue present since 4.0.0 label Mar 9, 2020
@jennifer-shehane
Copy link
Member

jennifer-shehane commented Mar 9, 2020

Yeah...not sure why it's doing that.

This began happening in Cypress 4.0.0. There's a lot of changes in 4.0.0, but my complete guess is maybe it involves the Chai upgrade #2862.

spec.js

it('cookie verification', () => {
  cy.visit('https://example.cypress.io/commands/cookies')
  cy.get('#getCookie .set-a-cookie').click()
  cy.getCookie('token').should('exist')
});

3.8.1

Screen Shot 2020-07-09 at 2 38 02 PM

4.0.0+

Screen Shot 2020-03-09 at 9 27 14 PM

Screen Shot 2020-03-09 at 9 41 14 PM

The below code does not log the assertion twice.

it('cookie verification', () => {
  cy.visit('https://example.cypress.io/commands/cookies')
  cy.get('#getCookie .set-a-cookie').click()
  cy.getCookie('token').then((cookie) => {
    expect(cookie).to.exist
  })
});

@cypress-bot cypress-bot bot added the stage: needs investigating Someone from Cypress needs to look at this label Mar 9, 2020
@jennifer-shehane jennifer-shehane added priority: low 🎗 pkg/driver This is due to an issue in the packages/driver directory type: unexpected behavior User expected result, but got another and removed stage: needs investigating Someone from Cypress needs to look at this labels Mar 9, 2020
@jennifer-shehane jennifer-shehane added type: regression A bug that didn't appear until a specific Cy version release and removed priority: low 🎗 type: unexpected behavior User expected result, but got another labels Jul 2, 2020
@jennifer-shehane jennifer-shehane added the good first issue Good for newcomers label Jul 30, 2020
@Tomastomaslol
Copy link
Contributor

Tomastomaslol commented Aug 11, 2020

HI @jennifer-shehane,

I had a look at this and it looks like the change in #2862 that is causing the assertion to run twice is:

https://github.com/cypress-io/cypress/pull/2862/files#diff-462c090a4a1cf87c581bae15f3af0d6aR111

It looks the same behaviour has been ported from the Coffescript to the Javascript version of asserting.

Example spec

it('cookie verification', () => {
  cy.visit('https://example.cypress.io/commands/cookies')
  cy.get('#getCookie .set-a-cookie').click()
  cy.getCookie('token').should('exist')
});

Running the spec against latest develop

Screen Shot 2020-08-11 at 1 02 48 pm

Running the spec against #8246

Screen Shot 2020-08-11 at 12 57 51 pm

I don't really understand why the code was introduced in the first place but it looks like most checks pass against the draft pull request I opened. Is there anything else you would like me to do or look at before requesting a review for the pull request?

@jennifer-shehane
Copy link
Member

@Tomastomaslol The PR will require tests to ensure that this is no longer logging twice in the UI. There's some idea of what these tests look like around command logs here: https://github.com/cypress-io/cypress/blob/develop/packages/driver/cypress/integration/commands/cookies_spec.js#L174:L174

@Tomastomaslol
Copy link
Contributor

@jennifer-shehane Thanks for your pointer. I opened a pull request that should fix the described issue. If you have a spare minute please let me know what you think of my proposed solution.

I really enjoyed working with the codebase and hopefully there will be other opportunities for me to contribute in the future!

Have a good Friday! 🙂

@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: ready for work The issue is reproducible and in scope labels Aug 18, 2020
@cypress-bot cypress-bot bot added stage: work in progress stage: needs review The PR code is done & tested, needs review and removed stage: needs review The PR code is done & tested, needs review stage: work in progress labels Sep 11, 2020
@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Sep 14, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Sep 14, 2020

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

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Sep 15, 2020

Released in 5.2.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v5.2.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Sep 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good for newcomers pkg/driver This is due to an issue in the packages/driver directory type: regression A bug that didn't appear until a specific Cy version release v4.0.0 🐛 Issue present since 4.0.0
Projects
None yet
5 participants