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

perf(cy.session): keep page state after validation runs #23503

Merged
merged 8 commits into from
Aug 26, 2022

Conversation

emilyrohrbough
Copy link
Member

User facing changelog

  • The cy.session() command's behavior has been enhanced to no longer clear the dom after a validation function runs. This means a cy.visist() command is no longer required after running cy.session() when a validation function is used; instead the test to continue testing the page visited in the validation function. Addressed #22368.

How has the user experience changed?

This change allow for better reusability of cy.session:

Cypress.addCommand('login', () => {
  cy.session('authenticatedUser', () => {
     cy.visit('/login')
     cy.get('username').type('user', { log: false })
     cy.get('password').type('password', { log: false })
     cy.get('submit').click()
}, {
  validate: () => {
      cy.visit('/home')
      cy.contains('Welcome User')
      cy.window().then((win) => {
        expect(win.localStorage.length).to.eq(2)
     })
  }),
})

it('test', () => {
  cy.login()
-  cy.visit('/home')
-  cy.contains('Welcome User')
  // do something elese
})

PR Tasks

  • Have tests been added/updated?
  • Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • Has a PR for user-facing changes been opened in cypress-documentation?
  • [n/a] Have API changes been updated in the type definitions?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 22, 2022

Thanks for taking the time to open a PR!

@emilyrohrbough emilyrohrbough added the topic: session Issues when using session command label Aug 22, 2022
@cypress
Copy link

cypress bot commented Aug 22, 2022



Test summary

39641 0 3371 0Flakiness 0


Run details

Project cypress
Status Passed
Commit eace0d3
Started Aug 26, 2022 5:08 PM
Ended Aug 26, 2022 5:22 PM
Duration 14:22 💡
OS Linux Debian - 11.3
Browser Multiple

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

Copy link
Contributor

@chrisbreiding chrisbreiding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this change. It's always thrown me off when using cy.session() that the page is reset after the validation and having to visit again. I think this works much more intuitively.

Looks good outside the very minor test comment fix 👍

Comment on lines 819 to 821
.visit('/fixtures/generic.html?foo#bar') // yes (1)
.visit('/fixtures/generic.html?foo#foo') // no (2)
.visit('/fixtures/generic.html?bar#bar') // yes (3)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.visit('/fixtures/generic.html?foo#bar') // yes (1)
.visit('/fixtures/generic.html?foo#foo') // no (2)
.visit('/fixtures/generic.html?bar#bar') // yes (3)
.visit('/fixtures/generic.html?foo#bar') // yes (1)
.visit('/fixtures/generic.html?foo#foo') // no (1)
.visit('/fixtures/generic.html?bar#bar') // yes (2)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oof good catch: 8d0142f (#23503)

'page navigation event (before:load)',
)

expect(emit.thirdCall).to.be.calledWith(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think it reads a bit more consistently with emit.getCall(2) than emit.thirdCall

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emilyrohrbough emilyrohrbough merged commit 9ba71c6 into develop Aug 26, 2022
@emilyrohrbough emilyrohrbough deleted the remove-extra-page-clears branch August 26, 2022 17:22
@stokrattt
Copy link

in this way, we have a problem. When some test from this spec have a redirect or visit another page, the session is broken, and local storage cleared.

@emilyrohrbough
Copy link
Member Author

emilyrohrbough commented Sep 1, 2022

@stokrattt Can you elaborate?

@stokrattt
Copy link

@stokrattt Can you elaborate?

your example at the top of the page implies that in the next it I will not use cy.login?

@emilyrohrbough
Copy link
Member Author

@skvale I don't really understand what you are trying to say. Mind sharing an example of what would be the problem?

@stokrattt
Copy link

stokrattt commented Sep 2, 2022

@skvale I don't really understand what you are trying to say. Mind sharing an example of what would be the problem?
your example at the top of the page (this understand?)

it('test', () => {
  cy.login()
  // do something elese 
})

it('test', () => {
  // do something elese
 click(another page) // and session was cleared. Why?
// do something elese
})

@emilyrohrbough
Copy link
Member Author

emilyrohrbough commented Sep 2, 2022

@stokrattt Thanks for providing an example. The current session is always cleared between tests - this is apart of the test isolation logic implemented in Cypress and ensuring your tests do not rely on one-another. This has been the behavior since the original release of the experiementalSessionAndOrigin, originally called the experimentalSessionSupport , experiment. This PR has not changed this behavior.

@rkhomi
Copy link

rkhomi commented Sep 2, 2022

@stokrattt Thanks for providing an example. The current session is always cleared between tests - this is apart of the test isolation logic implemented in Cypress and ensuring your tests do not rely on one-another. This has been the behavior since the original release of the experiementalSessionAndOrigin, originally called the experimentalSessionSupport , experiment. This PR has not changed this behavior.

Consider this example, we have simple crawler test suite, I just wanna go over sidebar and navigate to a bunch of pages and just check page title

To have this more readable I split it into several it's() to have nice report and know exactly where the page title was changed

Cypress.addCommand('login', () => {
  cy.session('authenticatedUser', () => {
     cy.visit('/login')
     cy.get('username').type('user', { log: false })
     cy.get('password').type('password', { log: false })
     cy.get('submit').click()
}, {
  validate: () => {
      cy.visit('/home')
      cy.contains('Welcome User')
      cy.window().then((win) => {
        expect(win.localStorage.length).to.eq(2)
     })
  }),
})


before(() => {
 cy.login()
})

it('test page 1', () => {
sidebar.goToPage(1)
 cy.contains(Page 1)
})

it('test page 2', () => {
sidebar.goToPage(2)
 cy.contains(Page 2)
})

but this won't work since each sidebar click changes the url
However this test suite is independent as possible,
So cy.session makes me change spec file like this

before(() => {
 cy.login()
})


it('Check pages', () => {
sidebar.goToPage(1)
 cy.contains(Page 1)

sidebar.goToPage(2)
 cy.contains(Page 2)
})

which is less readable

@emilyrohrbough
Copy link
Member Author

emilyrohrbough commented Sep 2, 2022

@rkhomi This isn't the right place for this discussion since you're feedback isn't related to this change. This appears to be a similar concern as shared here: #17887 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: session Issues when using session command
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keep the page state after user logs in when using cy.session()
5 participants