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

Enable Lifecycle Events to be customizable #686

Open
brian-mann opened this Issue Sep 23, 2017 · 13 comments

Comments

@brian-mann
Member

brian-mann commented Sep 23, 2017

Why this is necessary

Cypress automatically executes code in "between" tests. The idea behind this is that Cypress will try to "reset the state" and enable each test to run in its own pure state without other tests affecting or leaking into one another.

Currently it does the following:

  • clears localStorage
  • clears sessionStorage
  • clears cookies
  • removes aliases you've created
  • resets routes you've defined
  • removes stubs, spies, clocks you've created

Currently what it does not do:

  • clears the actual application under test

Why this needs to change

It really makes no sense the way we currently handle this. This is often a source of confusion we've seen from multiple users.

For instance it is:

  • not clear that this is happening
  • not clear how you can stop lifecycle events from applying
  • not clear why we reset everything above but not the application itself (?)

Because we don't forcibly clear the application state it means that it "appears" nothing has changed between the tests, but under the hood everything that built up the current session was reset.

This often results with users clicking links only to find out they've been magically "logged out".

In addition to sessions being cleared, it is actually technically possible for your application to do things in the tiniest briefest moment that Cypress finishes running one test and "switches" to the new test. Since your application is still active in the window, that means it will continue to run everything it's built to run.

This can cause all kinds of havoc.

Here's a sequence of events that demonstrates the problem:

  • before test 1
    • cypress resets state asynchronously
  • test 1 begins
    • your cypress spec code runs
    • you visit your application
    • you setup cy.route to route XHR's
    • cypress finishes executing all commands
  • test 1 finishes
    • cypress moves onto test 2
  • before test 2
    • cypress resets state asynchronously
      • while this happens, the application you visited in test 1 runs a poll / timer which creates an XHR
      • the state from the routes has been reset, so instead of XHR being routed, it goes out to your server
      • your server throws a 500, and the onerror handler of your XHR bubbles up and changes your application's state
      • your application is now in an invalid state and you have no idea why
  • test 2 begins
    • your cypress spec code runs
    • you don't visit your application again (you already did in test 1)
    • your commands fail because the state of your application in test 1 is now invalid
  • test 2 finishes, displays error

The reason we don't hear every user complain about this - is that for one its incredibly rare for this to happen and represents a "perfect storm" of events.

Additionally when we originally built these lifecycle events, they were with the assumption that most users would be visiting their applications before each test.

When visiting before each test (and thus always recreating / rebuilding the sessions and state), you insulate yourself from all of these problems.

What users really want to do

With that said many of our users really want to do one thing: login once in a before hook during the first test, and then have all subsequent tests run with this session.

While we don't really officially endorse this as the best pattern - it often makes sense in these situations:

  • your app loads slow (albeit you have a much bigger problem than writing tests)
  • you're not actually mutating session state in tests and therefore theres no reason to enforce needing to have it cleared each time

Another example users have suggested is when you apply a "unit testing" pattern to integration tests and write a bunch of "tiny" tests with a single assertion. However, we consider this an anti-pattern.

Irrespective of what we believe - the bottom line is that only logging in once before your tests will without a doubt enable all of your tests to run faster. If they have less to do in each test, they will simply complete faster.

Therefore it's imperative that we adequately solve this use case, and in the documentation and guides we provide resources / suggestions / hints / tips / tricks / philosophy to guide you to making these choices.

BTW in case you're curious we believe most of the need for a single login in a before hook is mitigated completely by using programatic API's like cy.request for things like setting state and logging in. By avoiding your UI you skip most of the reasons tests are slow.

One thing we absolutely do not endorse and will never support is writing tests

Changes that need to be made

Clear the App

Enforce clearing the Application at the end of test if it is not the last test but before the next test runs any of its code.

In other words, unless this is the very last test, the moment test code finishes and before any async code runs we MUST blow the application out of the window so no async events, polls, timers, promises, etc, run.

This will 100% prevent application leakage in "between" the tests and ensure the application is torn down properly.

Update the GUI

Display a new "instrument section" that lists each lifecycle event that was applied (or skipped).

screen shot 2017-09-23 at 8 03 26 pm

Create new commnad

We'll need a new cy.clearApp() command which brings into parity the other cy.clear* commands.

This enables you to issue it within a test as a "one-off" command the same way the others work.

Remove old API's

Deprecate / remove the Cypress.Cookies.preserveOnce API's since that would be superseded by these Lifecycle events.

Other Considerations

Having first class lifecycle events with their own programatic API also means we could turn these completely off when writing unit tests. As it stands Cypress automatically runs the clearing code in between each test. These are asynchronous and involve talking to the automation servers (in the browser and server) and therefore are quite slow (in terms of clock cycles).

Examples of how we could do this

I don't like the idea of making this global configuration in cypress.json. Instead I think we should create API's that enable you to control this in code.

Since Cypress has to know how you intend to apply lifecycle events this code would have to go before any tests are defined or run.

Likely this is what you want - you'd likely want these changes to propagate to an entire spec file.

// users_spec.js

Cypress.lifecycle() // returns an object with the current configuration

Cypress.lifecycle(false) // disable all lifecycle events
Cypress.lifecycle(true) // enable all lifecycle events

Cypress.lifecycle({
  clearApp: true, // leave this on
  clearInternals: true // leave this on
  clearCookies: false // nope
  clearLocalStorage: false // nope
  clearSessionStorage: false // nope
})

describe('my suite', () => {
  it('test 1', () => {
    ...
  })

  it('test 2', () => {
    ...
  })
})

Users wanting to apply Lifecycle events globally to all specs would just add the code into a support file.

Use Cases

At first glance it seems as if lifecycle events would be binary: you'd either want to have them all enabled (by default) or turn them all off.

Not so fast Jack - let's take a deeper look and why there's a bit more complexity than this:

Clear Everything (the default)

Pros

  • Absolutely no state build up
  • Simple, clear, insulated
  • Promotes taking shortcuts and skipping the UI as much as possible

Cons

  • Realistically a bit slower to run all tests

Summary

  • Slowest but flake free. Easy to understand.

Clear Nothing (what many users want)

Pros

  • You visit once in a before hook, usually establishing the session there
  • All subsequent tests use this existing session and don't visit
  • The tests will run faster

Cons

  • All of the tests are loosely coupled to one another unless you're really careful
  • The state from one test will immediately apply to the subsequent test
  • When running a single test vs all you will likely see different behavior that is difficult to debug / understand
  • You're still susceptible to actions in your application leaking into other tests (as described above in detail)

Summary

  • Fastest but susceptible to edge cases, potential flake, and poorly written tests.

Clear Some Things (likely what you should do instead)

If you want the benefit of only establishing a session once, but don't want to problems of loosely coupled tests then you likely want this configuration:

Cypress.lifecycle(false) // disable everything
Cypress.lifecycle{
  clearApp: true // but always clear the app
})

describe('my suite', function () {
  before(function () {
    cy.login() // establish the session only once!
  })

  beforeEach(function () {
    cy.visit('/') // but continue to visit before each test
  })

  it('test 1')

  it('test 2')

  it('test 3')
})

Pros

  • The current state of the app is always blown away before each test
  • The session is preserved between tests avoiding this to be re-created

Cons

  • Takes more explanation and deeper understanding of Cypress to know about this
  • Requires visiting before each test since the app is blown away
  • Keeping the same session around can lead to state issues on the server (if the server isn't stubbed)
  • You're apt not to take shortcuts which means that the gains you receive from running all tests are lost when running and iterating on a single test

Summary

  • Faster than the first open, slower than 2nd. Likely what you want instead.

@brian-mann brian-mann self-assigned this Sep 23, 2017

@brian-mann brian-mann changed the title from Enable Lifecycle Events to be customizable to Proposal: Enable Lifecycle Events to be customizable Sep 23, 2017

@brian-mann

This comment has been minimized.

Member

brian-mann commented Sep 24, 2017

@jennifer-shehane jennifer-shehane changed the title from Proposal: Enable Lifecycle Events to be customizable to Enable Lifecycle Events to be customizable Sep 25, 2017

@BenoitAverty

This comment has been minimized.

BenoitAverty commented Apr 17, 2018

One case where tests need to build up state between them is when each test is a step of a cucumber scenario. In that case, each test depends on the previous steps of the same scenario.

I'm using cypress-cucumber-preprocessor but I'm currently stuck because of this (I have opened TheBrainFamily/cypress-cucumber-preprocessor#43 about this).

With the feaure discussed in this issues, it would be doable for the preprocessor to manage the state clearing in such a way that state would be cleared in between scenarios but not between steps of the same scenario.

I'm guessing this is still a long way down the road, but do you have an estimate on when you may be tackling this ?

@BenoitAverty

This comment has been minimized.

BenoitAverty commented Apr 17, 2018

In the meantime, if I wanted to experiment, can anyone point me to the direction of the function(s) or pieces of code that do the clearing ? I'd like to see if this can't be solved in the preprocessor itself.

@joshacheson

This comment has been minimized.

joshacheson commented Apr 30, 2018

Is there any progress on this issue, or can anyone shed light on how product / engineering at Cypress are viewing this? Resolution (or knowing this is a no-op) on this issue would help me a great deal when it comes to evaluating whether or not Cypress is the best solution for problems I'm trying to solve.

@jdhines

This comment has been minimized.

jdhines commented May 22, 2018

At the very least refer to this issue in the docs. I spent several hours pulling my hair out wondering why my second cy.route() (declared in a before() and used in test number 2) was not actually stubbing. If I had read in the before, route, stubs, and possibly other parts of the docs (which I thought I'd read pretty thoroughly) had this info, I could have easily refactored my test code and gotten my tests passing.

@pacop

This comment has been minimized.

pacop commented Jul 23, 2018

Is some progress or ETA on this proposal? This would be awesome!

@ronlawrence3

This comment has been minimized.

ronlawrence3 commented Jul 25, 2018

Wasted a lot of time trying to figure out why my authentication token was lost during tests seemingly randomly. I'm still not sure there is not an issue with the plugin I'm using in invoking this lifecycle during a test, but it would have helped to have known that this was there.

Also a thought about your "not that this is a good testing practice" wording I see all over your docs... While I tend to agree with most of them I've seen, there are many existing protractor / selenium tests out there where there may not be the best approaches, and if you intend to win over those folks, there needs to be a way to migrate from "bad patterns", not just a scolding from you about the wisdom of doing it that way and no other way of doing it!

@Knaledge

This comment has been minimized.

Knaledge commented Aug 6, 2018

I just want to express appreciation for the typical proposal structure I see from @brian-mann throughout the project.

In using Cypress, it became evident that the state of things was reset between each "it" (test) - which started as a concern at first, which resulted in loosely forming as an observation which then turned into tinkering, spending some time as a suspicion, and then finally becoming apparent/understood.

What's great though is that along that journey, I started to question why I initially reacted with "concern". It gave me something to think about.

And while thinking about it, I realized that all along, I had always believed in "clean test states" - I advocate for this in my own work routinely. Just for whatever reason, it didn't click right away while using Cypress.

What I'm saying is that I appreciate the thought that goes into these requests, the adherence to best practices - and walking the talk. I came around to this understanding of Cypress (and their prescription of "best practice" in this case) organically - and it's just reassuring to see the same thought-journey expressed here in a request from one of the primaries on the project.

All that said, I believe there is a need for this and hopefully maintains the proposed "invocable" / on-demand nature.

Keep up the good work!

@jennifer-shehane

This comment has been minimized.

Member

jennifer-shehane commented Aug 10, 2018

If an issue is labeled as "stage: proposal", then it is within our roadmap to do someday, but typically no work has been put directly into the feature aside from some discussion and determining it to be technically feasible to achieve. Sometimes there are tertiary tasks done to prepare for a proposal's delivery though.

@Phillipe-Bojorquez

This comment has been minimized.

Phillipe-Bojorquez commented Sep 14, 2018

I use cypress as an e2e tool for my UI automation work. Because my companies app is so UI heavy building up state is actually desired. While most of my test suits are discrete units of logic, I also have to evaluate the performance of an app over time from a users built up state to simulate risk conditions. So I really appreciate this new feature request.

@yannicklerestif

This comment has been minimized.

yannicklerestif commented Nov 7, 2018

One of the benefits of having non-independent it() in a spec is that it helps a lot structuring a test report for a longer spec.
For example:

describe('Adding a Student to a Class', () => {

    it('Should Login', () => { //... };

    it('Should Create Class', () => { //... };

    it('Should Create Student', () => { //... };

    it('Should Add Student to the Class', () => { //... };
}

Apparently non-independent it()s inside a describe() are considered a bad practice.

How do you suggest achieving the same result?

@Floriferous

This comment has been minimized.

Floriferous commented Nov 9, 2018

This is completely insane. After months dealing with seemingly obscure logouts, having a horrible cypress experience, and spending hours debugging this (to the best of our knowledge..), we finally kind-of figured this out.

These lifecycle functions should be mentioned within the first 2 minutes of getting started with Cypress in Huge, Bold, italic, underlined letters, like right here: https://docs.cypress.io/guides/getting-started/writing-your-first-test.html#Add-a-test-file.

If at the very least the clearing functions would actually work, and not just completely randomly, it'd be much easier to get around this.

@Bobgy

This comment has been minimized.

Bobgy commented Dec 3, 2018

I've been affected by not clearing the app. My specific use-case is that:
I need to setup some cookies/local storage before visiting the page in each test, so I always put visit commands inside tests.

In one corner case, local storage setup triggered event listeners of the previous app and broke a test. As a workaround, I added a workaround clearApp command in beforeEach hook.

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

This has solved my issue perfectly as of now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment