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

Setting stubs/spies in before hooks and automatic restoring #583

Open
pawelgalazka opened this issue Aug 4, 2017 · 17 comments
Open

Setting stubs/spies in before hooks and automatic restoring #583

pawelgalazka opened this issue Aug 4, 2017 · 17 comments
Labels
E2E Issue related to end-to-end testing stage: proposal 💡 No work has been done of this issue topic: hooks ↪ type: enhancement Requested enhancement of existing feature type: unexpected behavior User expected result, but got another

Comments

@pawelgalazka
Copy link

pawelgalazka commented Aug 4, 2017

Context

From Cypress docs:

Automatic reset/restore between tests

cy.stub() creates stubs in a sandbox, so all stubs created are automatically reset/restored between tests without you having to explicitly reset/restore them.

How this relates to stubs/spies which were set in before hooks ? I would expect that stub/spy which was in before won't be reset/restored for each test within the context. This seems to currently happening actually, but is that intended ?

@bahmutov
Copy link
Contributor

bahmutov commented Aug 4, 2017

@pawelgalazka could you give an example of a stub/spy that you set multiple stubs - some in before hooks, some in the test?

@pawelgalazka
Copy link
Author

pawelgalazka commented Aug 4, 2017

@bahmutov

describe('Google Analytics', () => {
  let gaStub

  before(() => {
    gaStub = cy.stub()
    cy.visit('http://localhost:8080', {
      onBeforeLoad: (contentWindow) => {
        Object.defineProperties(contentWindow, {
          'ga': {
            value: gaStub,
            writable: false
          }
        })
      }
    })
  })

  it('should perform logging on first page load', () => {
    cy.wrap(gaStub).should('have.been.called')
  })

  it('should report pageview', () => {
    cy.wrap(gaStub).should('have.been.called.with', 'send', 'pageview')
  })
})

This actually works, but following documentation, second test should fail, since stubs should be automatically restored/reset between tests. Is that correct ?

BTW. On subject page call window.ga('send', 'pageview') should be performed

@chrisbreiding
Copy link
Contributor

chrisbreiding commented Aug 4, 2017

The stub should be reset between tests regardless of where it's created, so this looks like a bug.

I was able to reproduce with this code, where the second test fails but should pass:

describe('stub not restored', function () {
  let stub
  before(function () {
    stub = cy.stub()
  })

  it('should be called', function () {
    stub()
    expect(stub).to.have.been.called
  })

  it('should not be called', function () {
    expect(stub).not.to.have.been.called
  })
})

The driver has undergone (and is still undergoing) a lot of refactoring for 0.20.0, so it's possible this has been fixed. I'll make sure to verify and add a test once the refactoring is finished.

@pawelgalazka
Copy link
Author

pawelgalazka commented Aug 4, 2017

I'm wondering if this should be treated as a bug, because if I set a stub in before hook I would actually expect that stub should not be reset between tests (at least for tests in the closest describe).

If stub is set in beforeEach then I would expect stub restoring between tests.

@brian-mann
Copy link
Member

brian-mann commented Sep 6, 2017

This brings up a lot of potential discussion points, but I think we are conflating many things which are actually separate and distinct from one another.

Point 1.

It is true that Cypress will reset stubs, aliases, routes, local storage, cookies, and virtually all state from the browser in between tests. We do not recommend sharing state, and we need to come up with fully fledged lifecycle API's to control this. It is not obvious nor easy to do this, and that's why it doesn't exist yet.

Point 2.

In your examples above, I'm see the correct behavior. Resetting a stub simply means to replace the existing method that was stubbed with the original.

In your example you're pulling out the stub reference to make to accessible in the entire closure. This reference is just that - a reference to the stub. You can continue to make assertions on it because its a legitimate stub. When it was 'reset', it was simply removed from being the function defined on windodw.ga. So the difference is that when your page calls window.ga it will no longer call the stub - but that's a different problem altogether.

Point 3.

I think our team kind of understands what you're doing. You're wanting to visit once, and then write a bunch of assertions in different tests about the thing you performed once. This conflicts with how we reset state between tests. What you're essentially writing is one giant test. There's not really any benefit splitting things out, because each test is already coupled to the others.

@pawelgalazka
Copy link
Author

pawelgalazka commented Sep 10, 2017

I’ve just want to address point 3 to explain the benefit of before hook. As you describe what I do here are kind of “passive asserts”, so I visit page once and then do bunch of asserts. Same can be for other example:

describe('when logged as admin', () => {
  before(() => {
	  // some login commands
    cy.visit('http://localhost:8080/some/page')
  })

  it('should display panel bar', () => {
    cy.get('.panel-bar').should('be.visible')
  })

  it('should should show additional item', () => {
    cy.get('.admin-items').should('be.visible')
  })
})

There could be a case that additional item could be shown but admin panel bar not at this state (because of some weird error with permissions checks in template), so still there is a benefit of splitting this up to have a better feedback, where we fail exactly.

Then there is a question, why not to use beforeEach then ? Well if you have 30 passive tests like that and you visit staging site env, every visit is time expensive. Suddenly test execution jumps from 10s to 5 minutes. True tests share state, but each test does not perform any action which could potentially change it. So we gain nothing with beforeEach in this case, except longer tests execution.

Because of this it would be great if it would be possible to disable automatic alias/stubs clearing for certain tests/test files to be able to use them with before hooks.

Hope this explains situation better.

@pacop
Copy link

pacop commented Jul 23, 2018

Somes news right here?

Because of this it would be great if it would be possible to disable automatic alias/stubs clearing for certain tests/test files to be able to use them with before hooks.

This idea rocks!

@jennifer-shehane jennifer-shehane added type: unexpected behavior User expected result, but got another stage: proposal 💡 No work has been done of this issue and removed type: question labels Aug 2, 2018
@moroshko
Copy link

moroshko commented Oct 9, 2019

Any news here?
@pawelgalazka Did you find a reasonable workaround?

@pawelgalazka
Copy link
Author

@moroshko unfortunately I haven't found any workarounds this

@Sumanth1703
Copy link

Any update or possible work around for this ?

@jennifer-shehane jennifer-shehane added the type: enhancement Requested enhancement of existing feature label Aug 21, 2020
@jonyw4
Copy link

jonyw4 commented Oct 7, 2020

Sorry to bother you guys, but I think that it's a very unexpected behavior, do you have some idea when a fix will come?

@TheGooner93
Copy link

+1 . this would be a welcome feature!

@melibe23
Copy link

I will definitely enjoy this enhancement.
I use an alias to save the Google Analytics events therefore I must use a beforeEach and this makes the script last longer. Which is a very very very disadvantage on E2E testing.

I tried in every situation to assert several things on one single test. But IMO this approach applies very well for assertions that do not require interaction, for example, SEO checks.
onClick events, by nature, need an interaction. So, to properly cover each of them I have to create different tests, one per interaction.

Imagine a component with 3 onClick links, I lose testing coverage if I try to use several assertions on one test so I can use a before.
image

@mamagruder
Copy link

Throwing in a +1 for the "have a way to not reset stubs / spies between tests" option.

@aleksey-ilin
Copy link

@jennifer-shehane Will you plan to add this feature?

@zaren-s
Copy link

zaren-s commented May 11, 2023

Will there be any update on this?

@nagash77 nagash77 added the E2E Issue related to end-to-end testing label May 12, 2023
@lucbellavance
Copy link

Any update on this? We can't call window.open twice inside a single test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E2E Issue related to end-to-end testing stage: proposal 💡 No work has been done of this issue topic: hooks ↪ type: enhancement Requested enhancement of existing feature type: unexpected behavior User expected result, but got another
Projects
None yet
Development

No branches or pull requests