cypress-io / cypress

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

Cypress doesn't always clean up cookies #781

Closed verheyenkoen closed 2 years ago

verheyenkoen commented 6 years ago

Current behavior:

We have a test that checks if an unauthorized user is redirected to the login page when directly visiting a URL to the "backend" site. This test occasionally fails. From the screenshot it is clear that the user was just sent to the specific backend page without login.

image

I checked (console props of the cy.visit command & cy.getCookies + cy.writeFile) and in some cases the auth cookie from the previous test is not cleared. For that I'm using a custom login command that only fetches cookies once (per username) and stores them in the global scope. Then I use cy.setCookie() to set the cookie again.

In this case the cookie was set in the beforeEach hook of the previous test.

Obviously this never fails if I just run the latter spec.

Desired behavior:

Obviously, by default (unless Cypress.Cookies.preserveOnce() is used), all cookies should be cleared before a test run starts.

How to reproduce:

This is hard. I couldn't narrow it down to one specific always-reproducable scenario but these are things I can confidently say about it:

I'm not sure but it seems to fail easier if I change the focus to other apps (or spaces) while running the tests, but even that isn't deterministic for me.

verheyenkoen commented 6 years ago

I simplified my login command (because it also produced an application specific side effect and didn't really turn out to save much time), just keeping the cy.request statement and not storing cookies anymore and the problem is still there so it's probably not related to cy.setCookie.

brian-mann commented 6 years ago

Do you think this is related to this issue? https://github.com/cypress-io/cypress/issues/408

verheyenkoen commented 6 years ago

@brian-mann I checked that one. It's not a cross-domain login so I guess not. Am I right that I should never manually call clearCookies in an after hook?

brian-mann commented 6 years ago

Correct, Cypress automatically clears cookies prior to each test.

You can use the cookie debug API to print every single modification to every single cookie.

https://docs.cypress.io/api/cypress-api/cookies.html#Debug

verheyenkoen commented 6 years ago

Is there any way to dump the command log of a cypress run (apart from video recording). Cypress.Cookies.debug() may lead me to the problem indeed but this one is very hard to reproduce in the UI, so I'm looking for a way to review these cookie changes.

brian-mann commented 6 years ago

https://github.com/cypress-io/cypress/issues/700 https://github.com/cypress-io/cypress/issues/448

pawelgalazka commented 6 years ago

Can confirm, Cypress does not clear the cookies between tests, it let it leaks. This not happens all the time though.

webdevian commented 6 years ago

We are having this problem intermittently on Mac OS X Sierra w/ Chrome 63 and Electron 53 on the first test in our spec.

Printing out cy.getCookies() appears to show the cookies are cleared between tests but when we print out the request cookies in php there are some cookies there.

It has been hard to replicate this, there doesn't seem to be a pattern to force it to happen.

verheyenkoen commented 6 years ago

On my end also the first test in the spec that fails. Obviously only if other specs have been run before.

antonfisher commented 6 years ago

In my case it was silly, I had whitelist configuration to keep session id between different cy.location() calls and I forgot about it:

Cypress.Cookies.defaults({whitelist: 'sid'});

So I need to clean up this particular cookie before run whole test spec: before(() => cy.clearCookie('sid'));. Just in case.

verheyenkoen commented 6 years ago

I'm not using that feature so that's not it.

webdevian commented 6 years ago

Any luck with v2?

verheyenkoen commented 6 years ago

Didn't see this fail anymore. Were there any changes on this part?

webdevian commented 6 years ago

I have noticed it happen still

verheyenkoen commented 6 years ago

Ok now I have too.

lukemadera commented 6 years ago

I'm having this issue as well.

djalmaoliveira commented 6 years ago

I have some problem with cookies/session, my workaround was first select "electron" and run tests that will not work properly, after that select "Chrome" and run tests that work.

Cypress package version: 2.1.0 Cypress binary version: 2.1.0 OS: Ubuntu 17.10

anurbol commented 6 years ago

I have the problem too, cookies persist. That means I am always logged in and my login test fails. Problem is on both Windows 7 and Linux (Debian 9.4), in Chrome, Cypress 2.1.0.

Can this be an issue of Chrome?

I noticed some interesting pattern: cookies are cleared as expected, when I: 1) cypress open 2) run all tests 3) (old cookies are there, user is logged in) stop tests by clicking stop button 4) F12 (open DevTools) 5) restart tests -> everything is OK

scrat98 commented 6 years ago

the same problem

kc-beard commented 6 years ago

same problem. cookies are persisting.

anurbol commented 6 years ago

My temporary workaround:

describe('Auth', function () {

    before(() => {

        // remove this when issue will be solved
        // https://github.com/cypress-io/cypress/issues/781
        cy.visit('/')
        cy.clearCookies()
        cy.reload()
    })

...

Place this code at the very beginning of your tests (I guess they are ordered alphabetically).

kc-beard commented 6 years ago

@anurbol we went with something similar, but we have several describe blocks across different files so it's a pain to have to do this everywhere.

verheyenkoen commented 6 years ago

@kc-beard You can also define a beforeEach routine in your support/index.js file, which would be executed before each test in any other file, but that's probably overkill...

kc-beard commented 6 years ago

@verheyenkoen thanks for the tip! I didn't know that. Seems like that's covered in the docs too but i missed it (https://docs.cypress.io/guides/core-concepts/writing-and-organizing-tests.html#Support-file)

anurbol commented 6 years ago

@kc-beard in my experience, placing this in one file (which is starting point of your tests) is enough. verheyenkoen's answer is 2nd option, but as said, overkill (slows down tests execution significally).

KrisBraun commented 6 years ago

This is definitely a significant issue! Feel like something that requires warnings in the docs until it's resolved, as it took us a while to figure out why tests were failing when the documentation claims that cookies are cleared between tests.

GrayedFox commented 5 years ago

I can consistently reproduce this. After experimenting with persisting cookies using the whitelist feature and trying various setups, here is a summary of information about my current setup which consistently reproduces this issue:

If you run all specs with:

cypress run  // or in my case, yarn cypress run

The second set of integration tests will pass.

If you run the tests separately with:

yarn cypress run --spec 'cypress/integration/auth/**/*'
yarn cypress run --spec 'cypress/integration/secondSet/**/*'

All tests will pass in the first run, and the second set will fail, due to being unauthenticated.

GrayedFox commented 5 years ago

I understand this bug was opened in October 2017, but could we get some confirmation that this is indeed a high priority issue? I'm trying to convince a rather large Ruby team using Rspec to move over to Cypress and with inconsistent cookie handling, there's simply no way we can start integrating this into our CI and start replacing the old (flaky) Ruby tests.

Love the work and philosophy behind Cypress but it is sometimes a bit hard to see what is actively being worked on and what is a priority. If this really is something which still "needs investigating" after the amount of user reproductions, I'm not sure I can convince my team this really is the right choice for us at the moment :confused: (I may be assuming too much of that label!)

KrisBraun commented 5 years ago

We're in exactly the same position. Ready to make a big investment in Cyprus (including becoming a paying customer), but this is the biggest blocker for us. The length of time and lack of communication is giving us second thoughts.

nirpeled commented 5 years ago

Was this fixed in v3?

jennifer-shehane commented 5 years ago

I believe this may be a duplicate of https://github.com/cypress-io/cypress/issues/781 https://github.com/cypress-io/cypress/issues/408, but this is difficult to tell as there is not a clear reproducible example to run the code with here.

Can anyone verify that the cookies that are not being cleared are on a separate domain (even subdomain)?

kc-beard commented 5 years ago

@jennifer-shehane this is #781, assuming there's another issue you meant to link. In our case, they are on a separate domain.

@nirpeled no, we are seeing the same issue in v3

jennifer-shehane commented 5 years ago

Sorry, I meant duplicate of https://github.com/cypress-io/cypress/issues/408

verheyenkoen commented 5 years ago

@jennifer-shehane This was already suggested by @brian-mann last year (see earlier in this issue).

Do you think this is related to this issue? #408

My reply back then was:

@brian-mann I checked that one. It's not a cross-domain login so I guess not. Am I right that I should never manually call clearCookies in an after hook?

dwelle commented 5 years ago

In my case the cookie is not only not cleared on a cold run, but also the cookie with new value is being overriden by the old value (i.e. the cookies are set 2x on a cold run -- 1st the new value, 2nd the old value from previous test). The test must be restarted for the cookie to be set properly.

Is this the same issue and can somebody reproduce this, or should I create a new one?

Edit: I've narrowed it down --- the cy.visit() sets its own cookies for some reason, and it uses the previous cookie (which was supposed to have been cleared), so that's why it overrides the cy.setCookie() that I issue before the cy.visit(). This happens only on a cold run, as stated above.

dwelle commented 5 years ago

Ok, 4 hours later of trying to write my own cookie layer on top of Cypress, it's now obvious where's the flake from cold run coming from:

  1. cypress on cold run opens a browser and visits http://localhost:53814 (or something like that) which is its own domain.
  2. you run cy.clearCookies() (well you don't have to since Cypress does it before eachh test) in a before() hook in your index.js, and it clears the cookie from that domain, not your own.
  3. you then run cy.visit('your.domain/'), and it has the cookies from your previous run --- provided you didn't run cy.clearCookies() in an after() hook - use of which is usually discouraged because after() isn't guaranteed to run (which is why we didn't do that).

Thus, until https://github.com/cypress-io/cypress/issues/408 is implemented, the only way to solve this (without hacks such as recovering from flake and running cy.visit() twice) is to put cy.clearCookies() in the after() hook in index.js.

EDIT (18-11-18):

The main issue is this: on a cold run, setting an initial cookie before the first cy.visit will set a cookie with domain of the cypress runner's empty page (localhost on a random port), and that cookie won't be sent alongside the cy.visit request to your server. This can be fixed and automated away using this code (adapt for your purposes):

// cypress/support/index.js

Cypress.Commands.overwrite( 'setCookie', (origFn, name, value, opts) => {
    return origFn(name, value, {
        domain: '.' + new URL(Cypress.config('baseUrl')).hostname,
        ...opts
    });
});

before(() => {
    Cypress.config('baseUrl', 'http://your-app.test');
});
jennifer-shehane commented 5 years ago

Can anyone else confirm @dwelle's workaround solves your issue?

verheyenkoen commented 5 years ago

I think I get the reasoning of @dwelle but I don't understand why this bug is not consistently reproducible then.

Also, the proposed solution will not work in projects where you visit pages on multiple domains I guess.

It could be a solution for Cypress to not do the "localhost + random port" thing at all when a baseUrl is configured and instead use that baseUrl from the start of each session, so the cookie problem cannot occur anyhow (given that @dwelle's findings are correct and you're working in a "single-baseUrl-project").

dwelle commented 5 years ago

There's more to it than that. From what I remember, when the cold run encounters cy.visit it restarts the test so that any commands before it (such as cy.setCookie) are re-run on the correct domain and in AUT env --- which in itself should fix the problem my code above is aiming to solve --- but it doesn't. There's some bug which makes the cy.setCookie not override the cookie from previous run (at least cy.visit doesn't use the new cookie). I'm probably not making a lot of sense right now -- I'll investigate more, later, and prob create a new issue since it seems somewhat different from this one.

verheyenkoen commented 5 years ago

@dwelle Did you check #408? From the description that one seems different but the underlying problem (and fix) could be the same.

dwelle commented 5 years ago

@verheyenkoen yep, I first thought #408 would fix this (and it still might), but I wanted to investigate further if there aren't deeper problems.

jennifer-shehane commented 5 years ago

It could be a solution for Cypress to not do the "localhost + random port" thing at all when a baseUrl is configured and instead use that baseUrl from the start of each session, so the cookie problem cannot occur anyhow (given that @dwelle's findings are correct and you're working in a "single-baseUrl-project").

This is the behavior of Cypress today. If you have a baseUrl set, we do not visit the localhost + random port and instead visit the configured baseUrl.

verheyenkoen commented 5 years ago

@jennifer-shehane I know but does cy.setCookie() also use the same domain from the baseUrl (before the first cy.visit() command is called)?

dwelle commented 5 years ago

@jennifer-shehane This is the behavior of Cypress today. If you have a baseUrl set, we do not visit the localhost + random port and instead visit the configured baseUrl.

Huh... didn't think of that --- note that it only behaves like that if the baseUrl is set in cypress.json, and it seems to solve the issue I was describing above.

But, doesn't work if the baseUrl is set in support/index.js's before() which is what I've been doing all the time for some reason.

Edit: which makes sense, reading Cypress.config and Setting a global baseUrl.

benmardana commented 5 years ago

We have set the baseurl in cypress.json and still see this behaviour erratically. The behaviour only occurs when doing a login with cy.request, it doesn't appear to properly set the cookies and the previous cookies are used? See screenshots for reproduction. Note original cookie used in second test only after visiting a new address.

First user test screenshot

Second user test screenshot

Example done with something similar to below:

// log cookies here
cy.request({
  method: 'POST',
  url: '/account/login',
  followRedirect: false,
  form: true,
  body: {
    email: user.email,
    password: user.password
  }
}).then((resp) => {
  cy.visit(resp.redirectedToUrl);
});

// log cookies here
cy.visit('/home')

// log cookies here
Bobgy commented 5 years ago

Hi, I've been experiencing intermittent test failures related to cookie, when I run them sequentially in either cypress open or cypress run. I haven't yet to find any way to stably reproduce them.

However, later I met another issue with webapp from previous test interacting with setting local storage in the next test. So I applied this hack to clear previous webapp: https://github.com/cypress-io/cypress/issues/686#issuecomment-443691052

beforeEach(() => cy.visit('/some-blank-page-that-has-nothing'))

Surprisingly, it not only fixed the local storage event handler issue, but also fixed intermittent failures related to cookies. Now I'm guessing this may be related to previous app's lingering network requests interfering with our approach to clear all cookies.

verheyenkoen commented 5 years ago

@Bobgy That approach forces the baseUrl to be used from the very start of the test run which adds to the theory that Cypress may still use the "localhost:{random port}" base url for working with cookies (or local storage) until the first cy.visit or cy.request is recorded, which is clearly wrong.

I guess doing cy.request('/some-blank-page-that-has-nothing') would yield the same results. Can you try as that may be better in terms of performance?

Bobgy commented 5 years ago

@verheyenkoen The primary reason I do that is because of needing to clear the app from previous test. So using a request couldn't fit my need.

Secondly, I have a correct global baseUrl setup which is the same as the domain I test, so I don't think that's how it went wrong.

Thirdly, the intermittent failure never happen in the first test, so it seems to be related to previous test, not initial state

DennisLoska commented 5 years ago

too bad this issue still exist, just stumbled upon it today. The workaround to visit a page, clear the cookies and then reload again works though.

andreasmihm commented 5 years ago

same issue here with cypress 3.1.5. our setup is:

This is an absolutely essential issue, don't understand, why the bug is still open