chore: [Multi-domain]: Support multiple multi-domain commands in a single test #20354

mjhenkes · 2022-02-24T16:25:25Z

Closes [Multi-domain]: Support multiple switchToDomain calls in a single test #20199

User facing changelog

n/a

Additional details

This PR introduces the concept of running multiple multi-domain command in a single test. To accomplish this, I have added logic to:

Find an appropriate bridge when a multi-domain AUT is loaded
Send bridge messages to a specific domain
Send bridge messages to all domains
Identify which domain sent a message to primary

What this does not fix:

Setting readyForMultiDomain to false
Anticipating a domain that never loads

How has the user experience changed?

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?
Have API changes been updated in the type definitions?
Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2022-02-24T16:25:27Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

packages/driver/src/cy/multi-domain/index.ts

mjhenkes · 2022-02-24T16:27:13Z

packages/driver/src/cy/multi-domain/index.ts

- hookId: state('hookId'),
- },
+ communicator.once('bridge:ready', (_data, bridgeReadyDomain) => {
+ if (bridgeReadyDomain === domain) {


We can verify the bridge is ready for the correct domain now, not sure if this is ever not the same or if we should do something more drastic, but it's theoretically possible.

mjhenkes · 2022-02-24T16:28:30Z

packages/driver/src/cy/multi-domain/index.ts

+ communicator.once('bridge:ready', (_data, bridgeReadyDomain) => {
+ if (bridgeReadyDomain === domain) {
+ // now that the spec bridge is ready, instantiate Cypress with the current app config and environment variables for initial sync when creating the instance
+ communicator.toSpecBridge(domain, 'initialize:cypress', {


Tell the domain we just created to initialize cypress

mjhenkes · 2022-02-24T16:28:43Z

packages/driver/src/cy/multi-domain/index.ts

+ // once the secondary domain page loads, send along the
+ // user-specified callback to run in that domain
+ try {
+ communicator.toSpecBridge(domain, 'run:domain:fn', {


Just run the domain in question

mjhenkes · 2022-02-24T16:29:09Z

packages/driver/src/cypress/cy.ts

@@ -534,7 +534,7 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert
 // not utilized
 try {
 this.Cypress.action('app:window:load', this.state('window'))
- this.Cypress.multiDomainCommunicator.toSpecBridge('window:load', { url: this.getRemoteLocation('href') })
+ this.Cypress.multiDomainCommunicator.toAllSpecBridges('window:load', { url: this.getRemoteLocation('href') })


Any domain could be anticipating a window load, so tell them all

mjhenkes · 2022-02-24T16:29:21Z

packages/driver/src/cypress/runner.ts

@@ -1527,7 +1527,7 @@ export default {
 }

 cy.state('duringUserTestExecution', false)
- Cypress.multiDomainCommunicator.toSpecBridge('sync:state', { 'duringUserTestExecution': false })
+ Cypress.multiDomainCommunicator.toAllSpecBridges('sync:state', { 'duringUserTestExecution': false })


Keep all domains in sync

mjhenkes · 2022-02-24T16:30:06Z

packages/driver/src/multi-domain/communicator.ts

- private windowReference
- private crossDomainDriverWindow
+ private windowReference: Window | undefined
+ private crossDomainDriverWindows: {[key: string]: Window} = {}


This is now a map of domains to windows

mjhenkes · 2022-02-24T16:31:20Z

packages/driver/src/multi-domain/communicator.ts

@@ -105,9 +105,21 @@ export class PrimaryDomainCommunicator extends EventEmitter {
 * @param {string} event - the name of the event to be sent.
 * @param {any} data - any meta data to be sent with the event.
 */
- toSpecBridge (event: string, data?: any) {
+ toAllSpecBridges (event: string, data?: any) {


Opinions on function naming? this is kinda awkward, but also clear

Works for me.

Could go with some sort of broadcast naming.

I was thinking some sort of broadcast, but wasn't sure if that was more clear or just a more enjoyable word to say.

packages/driver/src/cy/multi-domain/index.ts

mjhenkes · 2022-02-24T16:32:05Z

packages/runner-shared/src/event-manager.js

@@ -522,13 +522,13 @@ export const eventManager = {
 Cypress.emit('internal:window:load', { type: 'cross:domain', url })
 })

- Cypress.multiDomainCommunicator.on('expect:domain', (domain) => {
+ Cypress.on('expect:domain', (domain) => {


I changed this because it made more sense to me that cypress is sending the message than the multi-domain communicator

mjhenkes · 2022-02-24T16:32:35Z

packages/runner-shared/src/event-manager.js

@@ -513,7 +513,7 @@ export const eventManager = {
 })

 Cypress.on('test:before:run', (...args) => {
- Cypress.multiDomainCommunicator.toSpecBridge('test:before:run', ...args)
+ Cypress.multiDomainCommunicator.toAllSpecBridges('test:before:run', ...args)


all domains should be notified before the test runs

mjhenkes · 2022-02-24T16:33:00Z

packages/driver/src/multi-domain/communicator.ts

 if (options.syncConfig) this.syncConfigEnvToPrimary()

 this.handleSubjectAndErr(data, (data: any) => {
 this.windowReference.top.postMessage({
 event: `${CROSS_DOMAIN_PREFIX}${event}`,
 data,
+ domain: location.hostname,


I am now appending the domain to ALL messages sent to the primary.

mjhenkes · 2022-02-24T16:33:35Z

packages/driver/src/multi-domain/communicator.ts

 }

- this.emit(messageName, data.data)
+ this.emit(messageName, data.data, data.domain)


the Domain the message originated from is now attached to all emitted messages

mjhenkes · 2022-02-24T16:34:43Z

packages/driver/src/multi-domain/communicator.ts

@@ -105,9 +105,21 @@ export class PrimaryDomainCommunicator extends EventEmitter {
 * @param {string} event - the name of the event to be sent.
 * @param {any} data - any meta data to be sent with the event.
 */
- toSpecBridge (event: string, data?: any) {
+ toAllSpecBridges (event: string, data?: any) {
+ debug('=> to all spec bridges', event, data)


I added these debug messages on eventing, I find that when i'm debugging multi-domain I'm always setting these to see what communication is happening.

Funny story: to enable these debug messages you have to add a local storage variable localStorage.debug = 'cypress:*', but with session enabled, we clear local storage after each test. 🤦‍♂️

mjhenkes · 2022-02-24T16:36:03Z

packages/runner-shared/src/event-manager.js

 const callback = () => {
- Cypress.multiDomainCommunicator.toSpecBridge('before:screenshot:end')
+ Cypress.multiDomainCommunicator.toSpecBridge(domain, 'before:screenshot:end')


Since we get the originating domain we handily also know where to send the message back

mjhenkes · 2022-02-24T16:36:57Z

packages/runner/injection/multi-domain.js

 if (!Cypress) {
 throw new Error('Something went terribly wrong and we cannot proceed. We expected to find the global \
- Cypress in the spec bridge window but it is missing. This should never happen and likely is a bug. Please open an issue.')
+ Cypress in the spec bridge window but it is missing.')


This could legitimately happen if a click causes several redirects and finally lands on the specified domain. That shouldn't be an error.

So do we even want this error? Seems odd to say something went terribly wrong for an expected scenario.

I think we will want to remove this error in the long run. Not finding cypress may or may not be a problem depending if this is the last domain loaded and I don't think we can know that. Even if it is the last domain loaded this error isn't bubbled up to the primary domain to display and the test times out.

I think the onus lies on the primary domain and ideally, the command that interacts with the dom to timeout if the correct domain hasn't loaded in an appropriate amount of time.

+1, I somehow missed the comment you added above the line

mjhenkes · 2022-02-24T16:37:37Z

packages/runner/injection/multi-domain.js

+
+ try {
+ // If Cypress is defined and we haven't gotten a cross domain error we have found the correct bridge.
+ if (frame.Cypress) {


Is this enough, I don't think there will be other frames on the same domain that also have cypress defined, it should just be the bridge.

Yeah I think it's safe to assume.

mjhenkes · 2022-02-24T16:38:05Z

packages/runner/injection/multi-domain.js

+ }
+ } catch (error) {
+ // Catch DOMException: Blocked a frame from accessing a cross-origin frame.
+ if (error.name !== 'SecurityError') {


Narrowing the exception we're catching as much as possible.

cypress · 2022-02-24T19:05:26Z

Test summary

4611 • 0 • 79 • 1 • 2

Run details


Project	cypress
Status	Passed
Commit	`8207496`
Started	Feb 24, 2022 10:25 PM
Ended	Feb 24, 2022 10:37 PM
Duration	11:48 💡
OS	Linux Debian - 10.10
Browser	Electron 94

View run in Cypress Dashboard ➡️

Flakiness

	commands/net_stubbing_spec.ts	1
1	network stubbing > intercepting response > can 'delay' a proxy response using Promise.delay
	cypress/proxy-logging_spec.ts	1
1	Proxy Logging > request logging > xhr log has response body/status code

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

chrisbreiding

Would you mind updating the FIXME tests here, now that multiple switchToDomains are supported.

packages/driver/src/cy/multi-domain/index.ts

chrisbreiding · 2022-02-24T19:03:01Z

packages/runner/injection/multi-domain.js

+
+ try {
+ // If Cypress is defined and we haven't gotten a cross domain error we have found the correct bridge.
+ if (frame.Cypress) {


Yeah I think it's safe to assume.

packages/runner/injection/multi-domain.js

chrisbreiding · 2022-02-24T19:04:47Z

packages/driver/src/multi-domain/communicator.ts

@@ -105,9 +105,21 @@ export class PrimaryDomainCommunicator extends EventEmitter {
 * @param {string} event - the name of the event to be sent.
 * @param {any} data - any meta data to be sent with the event.
 */
- toSpecBridge (event: string, data?: any) {
+ toAllSpecBridges (event: string, data?: any) {


Works for me.

mschile · 2022-02-24T21:03:25Z

packages/runner/injection/multi-domain.js

 if (!Cypress) {
 throw new Error('Something went terribly wrong and we cannot proceed. We expected to find the global \
- Cypress in the spec bridge window but it is missing. This should never happen and likely is a bug. Please open an issue.')
+ Cypress in the spec bridge window but it is missing.')


So do we even want this error? Seems odd to say something went terribly wrong for an expected scenario.

mjhenkes · 2022-02-24T22:09:10Z

packages/driver/src/multi-domain/communicator.ts

 if (options.syncConfig) this.syncConfigEnvToPrimary()

 this.handleSubjectAndErr(data, (data: any) => {
 this.windowReference.top.postMessage({
 event: `${CROSS_DOMAIN_PREFIX}${event}`,
 data,
- domain: location.hostname,


The host name is converted into punycode, which will not match the domain specified in the command. To circumvent this and avoid adding a dependency to convert punycode to unicode I have attached the unicode domain name to the window via the server.

mjhenkes · 2022-02-24T22:10:07Z

packages/server/lib/html/multi-domain-iframe.html

@@ -7,6 +7,7 @@
 <body>
 <script type="text/javascript">
 document.domain = '{{domain}}';


the browser converts document.domain to punycode by default, how helpful, but doesn't provide any API's to convert it back to unicode. So I attach the domain to window below.

mjhenkes added 4 commits February 24, 2022 09:42

multi-multi-domain changes

39f6784

tests

9834f70

line it up!

b8705bc

trim tests

48768cf

mjhenkes added the topic: cy.origin Problems or enhancements related to cy.origin command label Feb 24, 2022

mjhenkes requested review from chrisbreiding, mschile and AtofStryker February 24, 2022 16:25

mjhenkes requested a review from a team as a code owner February 24, 2022 16:25

mjhenkes self-assigned this Feb 24, 2022

mjhenkes commented Feb 24, 2022

View reviewed changes

packages/driver/src/cy/multi-domain/index.ts Outdated Show resolved Hide resolved

mjhenkes commented Feb 24, 2022

View reviewed changes

packages/driver/src/cy/multi-domain/index.ts Outdated Show resolved Hide resolved

mjhenkes commented Feb 24, 2022

View reviewed changes

try skipping unicode test

737cb8d

chrisbreiding requested changes Feb 24, 2022

View reviewed changes

Pr Updates

a8a08a3

chrisbreiding approved these changes Feb 24, 2022

View reviewed changes

mschile requested changes Feb 24, 2022

View reviewed changes

mschile approved these changes Feb 24, 2022

View reviewed changes

support unicode domain names

8207496

mjhenkes commented Feb 24, 2022

View reviewed changes

chrisbreiding approved these changes Feb 24, 2022

View reviewed changes

mjhenkes merged commit a0c6f00 into feature-multidomain Feb 24, 2022

mjhenkes deleted the md-multi-multi-domains branch February 24, 2022 22:41

cypress-bot bot mentioned this pull request Feb 24, 2022

[Multi-domain]: Support multiple switchToDomain calls in a single test #20199

Closed

chrisbreiding mentioned this pull request Apr 22, 2022

feat: Cross Origin Testing Support #18075

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: [Multi-domain]: Support multiple multi-domain commands in a single test #20354

chore: [Multi-domain]: Support multiple multi-domain commands in a single test #20354

mjhenkes commented Feb 24, 2022

cypress-bot bot commented Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

chrisbreiding Feb 24, 2022

mschile Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

mschile Feb 24, 2022

mjhenkes Feb 24, 2022

mschile Feb 24, 2022

mjhenkes Feb 24, 2022

chrisbreiding Feb 24, 2022

mjhenkes Feb 24, 2022

cypress bot commented Feb 24, 2022 •

edited

Loading

chrisbreiding left a comment

chrisbreiding Feb 24, 2022

chrisbreiding Feb 24, 2022

mschile Feb 24, 2022

mjhenkes Feb 24, 2022

mjhenkes Feb 24, 2022

chore: [Multi-domain]: Support multiple multi-domain commands in a single test #20354

chore: [Multi-domain]: Support multiple multi-domain commands in a single test #20354

Conversation

mjhenkes commented Feb 24, 2022

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

cypress-bot bot commented Feb 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress bot commented Feb 24, 2022 • edited Loading

Test summary

Run details

Flakiness

chrisbreiding left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress bot commented Feb 24, 2022 •

edited

Loading