-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
chore(sessions): break out sessions manager code and add unit tests #21268
Conversation
Thanks for taking the time to open a PR!
|
# Conflicts: # packages/driver/src/cy/commands/sessions/index.ts
Test summaryRun details
View run in Cypress Dashboard ➡️ Flakiness
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 |
@@ -0,0 +1,295 @@ | |||
import _ from 'lodash' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you change any logic in this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made comments on the changes.
packages/driver/cypress/integration/commands/sessions/manager_spec.ts
Outdated
Show resolved
Hide resolved
packages/driver/cypress/integration/commands/sessions/manager_spec.ts
Outdated
Show resolved
Hide resolved
packages/driver/cypress/integration/commands/sessions/manager_spec.ts
Outdated
Show resolved
Hide resolved
|
||
describe('._setStorageOnOrigins()', () => {}) | ||
|
||
it('.getAllHtmlOrigins()', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a describe
with an it
that describes the method's behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it matters. I can change it if you feel strongly.
Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>
const getOrigins = this.Cypress.Promise.map( | ||
([] as string[]).concat(origins), async (v) => { | ||
if (v === '*') { | ||
return await this.getAllHtmlOrigins() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to use existing function that handled this logic
- return _.keys(await Cypress.backend('get:rendered:html:origins')).concat([currentOrigin])
+ return await this.getAllHtmlOrigins()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙏
} | ||
|
||
if (_.isEmpty(origins)) { | ||
return results |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updates
- return getResults()
+ return results
pushValue(val[0], val[1]) | ||
}) | ||
|
||
return results |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updates
- return getResults()
+ return results
const currentOrigin = $Location.create(window.location.href).origin | ||
|
||
const origins: Array<string> = await this.mapOrigins(opts.origin) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
- const getResults = () => {
- return results
- }
}) | ||
}) | ||
|
||
describe('._setStorageOnOrigins()', () => {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume you meant for this describe to wrap the test below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, I missed adding a // TO DO
comment.
packages/driver/cypress/integration/commands/sessions/manager_spec.ts
Outdated
Show resolved
Hide resolved
const getOrigins = this.Cypress.Promise.map( | ||
([] as string[]).concat(origins), async (v) => { | ||
if (v === '*') { | ||
return await this.getAllHtmlOrigins() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙏
packages/driver/cypress/integration/commands/sessions/manager_spec.ts
Outdated
Show resolved
Hide resolved
packages/driver/cypress/integration/commands/sessions/manager_spec.ts
Outdated
Show resolved
Hide resolved
packages/driver/cypress/integration/commands/sessions/manager_spec.ts
Outdated
Show resolved
Hide resolved
packages/driver/cypress/integration/commands/sessions/manager_spec.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Bill Glesias <bglesias@gmail.com>
* 10.0-release: (22 commits) fix: migrate multiples projects when in global mode (#21458) test: fix flaky cy-in-cy selector validity test (#21360) chore: remove unused codeGenGlobs (#21438) fix: use correct path for scaffolding spec on CT (#21411) fix: remove breaking options from testing type on migration (#21437) fix: test-recording instructions in Component Test mode (#21422) feat: distinguish app vs launchpad utm_source when using utm params (#21424) chore: update stubbed cloud types (#21451) chore: change to yarn registry fix(sessions): refactor flows, fix grouping bugs and align validation fail text (#21379) chore(sessions): more driver tests (#21378) chore: rename domain_fn to origin_fn (#21413) chore: release 9.6.1 (#21404) fix: ensure that proxy logs are updated after the xhr has actually completed (#21373) chore: Re-organize tests in assertions_spec.js (#21283) chore: Distribute tests to desktop-gui containers. Make `desktop-gui` tests faster! (#21305) chore(sessions): add additional tests (#21338) fix: Allow submit button to be outside of the form for implicit submission (#21279) fix(launcher): support Firefox as a snap (#21328) chore(sessions): break out sessions manager code and add unit tests (#21268) ...
Apart of #20977
This refactor is prepping for updating changes with cy.session to use the new command group API and adding test coverage for GA support.
This PR breaks out the cy.session session manager functions into its own file and adds some missing unit tests -- there are a few that are still need to be added that I will update at a later time.
User facing changelog
n/a
PR Tasks
cypress-documentation
?type definitions
?cypress.schema.json
?