-
Notifications
You must be signed in to change notification settings - Fork 3.4k
chore: develop (including cy.origin) merge into 10.0 #21237
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
Conversation
…nto feature-multidomain
…nto feature-multidomain
…nto feature-multidomain
…e between domains (#17942)
…nto feature-multidomain
…nto feature-multidomain
| @@ -0,0 +1,14 @@ | |||
| declare namespace Cypress { | |||
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.
likely the changes here need to be overwritten to satisfy is comment
| @@ -176,13 +178,13 @@ const ffToStandardResourceTypeMap: { [ff: string]: ResourceType } = { | |||
| } | |||
|
|
|||
| export class CdpAutomation { | |||
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.
original in 9.x
| let frameTree | ||
| let gettingFrameTree | ||
|
|
||
| const onReconnect = (client: CRIWrapper.Client) => { |
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.
not exactly a 1:1 here, but this callback handler is defined here and passed into browserCRIClient static constructor which is ultimately referenced in the generic criClient static contructor. original in 9.x
| @@ -147,6 +147,10 @@ export const create = async (target: string, onAsynchronousError: Function, host | |||
| }) | |||
|
|
|||
| enqueuedCommands = [] | |||
|
|
|||
| if (onReconnect) { | |||
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.
see original in 9.x
| } | ||
|
|
||
| // eslint-disable-next-line @cypress/dev/arrow-body-multiline-braces | ||
| const _updateFrameTree = (client: CRIWrapper.Client, eventName) => 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.
see original in 9.x
| @@ -450,16 +570,21 @@ export = { | |||
| ]) | |||
|
|
|||
| await this._navigateUsingCRI(pageCriClient, options.url) | |||
|
|
|||
| if (options.experimentalSessionAndOrigin) { | |||
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.
Since we have a bit of a different process now for attaching to target and getting the browser CRI client, we need to make sure that the appropriate even listeners are applied every time we attach to a new target, as well as turn on any necessary CDP events.
| // the frame tree on reconnect in cases there were changes while | ||
| // the client was disconnected | ||
| _updateFrameTree(client)() | ||
| return _updateFrameTree(client)() |
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.
Is this asynchronous? Do we need to await it in criClient.create
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.
it is asynchronous, but we do not need to await it. The return statement here is to appease the linter. The waiting gets handled by the _isAUTFrame function itself here.
| } | ||
|
|
||
| const shouldWithTimeout = (cb, timeout = 250) => { | ||
| // FIXME: currently increasing the timeout here from 250ms to 1 second |
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 we log an issue for this?
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.
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.
issue logged in #21375
flotwig
left a comment
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.
Looks good, nothing jumped out at me as being a mismerge.
|
I tried to write some tests using I also noticed we can't use I tried writing some tests in I tried in develop, and I get the same problem, so I guess it's not a problem introduced by this back merge, so I'll ✅ . I am curious if I'm doing something wrong - this should work, correct? |
lmiller1990
left a comment
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 tried it out and compared between develop and this branch and it seems good.
I had some problems with using cy.origin that seem unrelated, but I posted a comment in case they are: https://cypressio.slack.com/archives/C01TQQCQLJY/p1652071156301199
…ating tests in an attempt to keep strict comparisons in the test
@lmiller1990 we talked about this standup. It looks like the dashboard code tries to reference |
…mpleted (#21373) Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>


DO NOT SQUASH!!!!
Closes #19887
User facing changelog (as seen in #18075)
experimentalSessionSupportFlagexperimentalSessionAndOriginFlag containing:originwhich allows users to run commands within multiple origins from a single test.pageLoadTimeout.Additional details
The goal of this PR is to merge
developcontaining thecy.origincommand and a few other updates, into10.0. For original PR intodevelop, please see #18075. To recap important changes in the development environment:experimentalSessionAndOriginoff (with the tests restructured to use a visit between tests to be compatible with sessions). Thecy.origintests are not run in this jobexperimentalSessionAndOriginon withcy.origintests run in this job. When this feature becomes GA, the goal is to consolidate back into 1 integration job per browser.For merging with 10.0, main conflicts/changes from the
developbranch into10.0:Driver
integrationtoe2eand following*.cy.jsparadigm over*.spec.js) and various snapshot updates. Also updating tests that look forcypress.jsonto look for newcypress.config.jsparadigm.createSpecIframeto genericaddIframeas we need to now add multiple iframes to create the spec bridges forcy.origin.Runner / App
develop, we were able to referenceCypressto remove/create the iframes and inject them into the page. Since it doesn't look to be instantiated yet, the same functionality has been replaced with native DOM methods.runner->appthat is AUT/iframe/event driven:event-manager(new websocket/communicator methods, screenshot methods (NEEDS TO BE VERIFIED)aut-iframechanges for cross origin snapshotting behavior and removal of snapshot responsibilities (NEEDS TO BE VERIFIED)iframe-modelchanges for cross origin snapshotting behaviorServer / Data-Context
remoteStatesconcept needs to be consumed by thedata-contextpackage when serving the app.runner.serveis no longer used in10.0andmakeServeConfigis the method whereremoteStateslikely needs to be reset. When the config is served,remoteshould otherwise be set tocurrent.X-Cypress-Is-AUT-Frameheader to inform the server to inject the cross origin driver, as well as passing the feature flag into the CDP automation static constructor.runnerdirectory in e2e mode and therunner-ctdirectory otherwise to make sure the runner routes can serve the cross origin bundle. (THIS NEEDS TO BE VERIFIED IF THIS CHANGE IS CORRECT)How has the user experience changed?
PR Tasks
Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)N/AHas a PR for user-facing changes been opened inN/A: Docs forcypress-documentation?cy.originare merged intocypress-documentationand shipped with9.6.0type definitions?Have new configuration options been added to theApplicable item changes fromcypress.schema.json?cypress.schema.jsonwere migrated tofrontend-sharedlocales to be referenced on the settings page (see screenshot below)