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

chore: Implement cross-domain sibling iframe #16708

Merged

Conversation

chrisbreiding
Copy link
Contributor

User facing changelog

N/A - only a partial feature of multidomain

Additional details

Adds a sibling iframe in which to run the cross-domain driver bundle. It's added dynamically when the AUT navigates to a second domain. Much is still hard-coded (domain needs to be localhost:3501), but further work will make it more dynamic.

Also includes a very rudimentary implementation of cy.switchToDomain.

PR Tasks

  • Have tests been added/updated?
  • N/A Has the original issue or this PR been tagged with a release in ZenHub? won't be released - going into a feature branch
  • N/A Has a PR for user-facing changes been opened in cypress-documentation?
  • N/A Have API changes been updated in the type definitions?
  • N/A Have new configuration options been added to the cypress.schema.json?

@chrisbreiding chrisbreiding requested a review from a team as a code owner May 27, 2021 18:12
@chrisbreiding chrisbreiding requested review from flotwig and kuceb and removed request for a team May 27, 2021 18:12
@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 27, 2021

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Jun 4, 2021



Test summary

116 1 0 0Flakiness 1


Run details

Project cypress
Status Errored
Commit 8dd089d
Started Jun 4, 2021 8:14 PM
Ended Jun 4, 2021 8:22 PM
Duration 07:48 💡
OS Linux Debian - 10.8
Browser Electron 89

View run in Cypress Dashboard ➡️


Flakiness

cypress/integration/retries.ui.spec.js Flakiness
1 runner/cypress retries.ui.spec > opens attempt on each attempt failure for the screenshot, and closes after test passes

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

end: true,
})

Cypress.action('cy:cross:domain:message', 'run:in:domain', fn.toString())
Copy link
Contributor

Choose a reason for hiding this comment

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

fn here will have the caveat that it doesn't have access to variables of parent lexical scopes, is this something that will be addressed or is it the plan to keep it that way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it will be addressed in future work. This is a bare-minimum version of this command just to get things rolling.

@@ -5498,6 +5498,8 @@ declare namespace Cypress {
message: any
/** Set to false if you want to control the finishing of the command in the log yourself */
autoEnd: boolean
/** Set to false if you want to control the finishing of the command in the log yourself */
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the difference b/t this and autoEnd? same comment

i think you can also update this in packages/driver/types/internal-types.d.ts if you don't want it to be public

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I can tell, autoEnd is redundant and we don't actually use it at all. The only instance of it is here:

options._log = Cypress.log({
snapshot: true,
autoEnd: false,
timeout: 0,

However, using autoEnd requires calling log.finish():

finish () {
// end our command since our subject
// has been resolved at this point
// unless its already been 'ended'
// or has been specifically told not to auto resolve
if (this._shouldAutoEnd()) {
return this.snapshot().end()
}
},

But I can't find any instances of calling log.finish() and the instance from above that uses autoEnd: true calls log.end(), which is the imperative way to end the snapshot, indicating it was never the intention to "auto-end" the log for cy.pause():

if (options.log) {
options._log.end()
}

The autoEnd option can probably be removed entirely, but that's outside the scope of this PR. I added the end option to the types since it's the one we actually use around the codebase for auto-ending a log. I guess this was the first time it was used in TypeScript though, so we hadn't noticed the omission before.

Comment on lines +291 to +292
// eslint-disable-next-line no-console
console.log('Unknown postMessage:', event.data)
Copy link
Contributor

Choose a reason for hiding this comment

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

the reason to not throw here is because the AUT could emit a postMessage to top, and we shouldn't fail the test in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is another bare-minimum implementation to get the ball rolling. We're planning more work on this in the future to put more thought into it and flesh it out more.

@chrisbreiding chrisbreiding requested a review from flotwig June 7, 2021 13:57
@chrisbreiding chrisbreiding merged commit d028437 into feature-multidomain Jun 7, 2021
@chrisbreiding chrisbreiding deleted the issue-16452-multidomain-sibling-iframe branch April 5, 2022 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants