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: [Multi-domain]: Support multiple multi-domain commands in a single test #20354

Merged
merged 7 commits into from
Feb 24, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -204,3 +204,52 @@ describe('multi-domain', { experimentalSessionSupport: true, experimentalMultiDo
})
})
})

// @ts-ignore
describe('domain validation', { experimentalSessionSupport: true, experimentalMultiDomain: true }, () => {
it('finds the right spec bridge with a subdomain', () => {
cy.visit('/fixtures/auth/index.html') // Establishes Primary Domain
cy.window().then((win) => {
win.location.href = 'http://baz.foobar.com:3500/fixtures/auth/idp.html'
})

cy.switchToDomain('foobar.com', () => {
cy.get('[data-cy="username"]').type('TJohnson')
cy.get('[data-cy="login"]').click()
})

cy.get('[data-cy="welcome"]')
.invoke('text')
.should('equal', 'Welcome TJohnson')
})

it('uses switchToDomain twice', () => {
cy.visit('/fixtures/auth/index.html') // Establishes Primary Domain
cy.get('[data-cy="login-idp"]').click() // Takes you to foobar.com
cy.switchToDomain('idp.com', () => {
cy.get('[data-cy="username"]').type('BJohnson')
cy.get('[data-cy="login"]').click()
}) // Trailing edge wait, waiting to return to the primary domain

// Verify that the user has logged in on /siteA
cy.get('[data-cy="welcome"]')
.invoke('text')
.should('equal', 'Welcome BJohnson')

cy.get('[data-cy="logout"]').click()

cy.window().then((win) => {
win.location.href = 'http://baz.foobar.com:3500/fixtures/auth/idp.html'
})

cy.switchToDomain('foobar.com', () => {
cy.get('[data-cy="username"]').type('TJohnson')
cy.get('[data-cy="login"]').click()
}) // Trailing edge wait, waiting to return to the primary domain

// Verify that the user has logged in on /siteA
cy.get('[data-cy="welcome"]')
.invoke('text')
.should('equal', 'Welcome TJohnson')
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ describe('multi-domain', { experimentalSessionSupport: true, experimentalMultiDo
cy.switchToDomain('0000:0000:0000:0000:0000:0000:0000:0001', () => undefined)
})

it('succeeds on a unicode domain', () => {
// TODO: this was hanging in CI
it.skip('succeeds on a unicode domain', () => {
cy.switchToDomain('はじめよう.みんな', () => undefined)
})
})
Expand Down
91 changes: 46 additions & 45 deletions packages/driver/src/cy/multi-domain/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ export function addCommands (Commands, Cypress: Cypress.Cypress, cy: Cypress.cy,
domain,
})

let onQueueFinished

const cleanup = () => {
communicator.off('queue:finished', onQueueFinished)
}

return new Bluebird((resolve, reject) => {
const _resolve = ({ subject, unserializableSubjectType }) => {
resolve(unserializableSubjectType ? createUnserializableSubjectProxy(unserializableSubjectType) : subject)
Expand All @@ -85,18 +91,14 @@ export function addCommands (Commands, Cypress: Cypress.Cypress, cy: Cypress.cy,
reject(err)
}

const onQueueFinished = ({ err, subject, unserializableSubjectType }) => {
onQueueFinished = ({ err, subject, unserializableSubjectType }) => {
if (err) {
return _reject(err)
}

_resolve({ subject, unserializableSubjectType })
}

const cleanup = () => {
communicator.off('queue:finished', onQueueFinished)
}

communicator.once('sync:config', ({ config, env }) => {
syncConfigToCurrentDomain(config)
syncEnvToCurrentDomain(env)
Expand All @@ -108,16 +110,12 @@ export function addCommands (Commands, Cypress: Cypress.Cypress, cy: Cypress.cy,
sendReadyForDomain()

if (err) {
cleanup()

return _reject(err)
}

// if there are not commands and a synchronous return from the callback,
// this resolves immediately
if (finished || subject || unserializableSubjectType) {
cleanup()

_resolve({ subject, unserializableSubjectType })
}
})
Expand All @@ -135,51 +133,54 @@ export function addCommands (Commands, Cypress: Cypress.Cypress, cy: Cypress.cy,
}

// fired once the spec bridge is set up and ready to receive messages
communicator.once('bridge:ready', () => {
// 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('initialize:cypress', {
config: preprocessConfig(Cypress.config()),
env: preprocessEnv(Cypress.env()),
})

state('readyForMultiDomain', true)

// once the secondary domain page loads, send along the
// user-specified callback to run in that domain
try {
communicator.toSpecBridge('run:domain:fn', {
data,
fn: callbackFn.toString(),
// let the spec bridge version of Cypress know if config read-only values can be overwritten since window.top cannot be accessed in cross-origin iframes
// this should only be used for internal testing. Cast to boolean to guarantee serialization
// @ts-ignore
skipConfigValidation: !!window.top.__cySkipValidateConfig,
state: {
viewportWidth: Cypress.state('viewportWidth'),
viewportHeight: Cypress.state('viewportHeight'),
runnable: serializeRunnable(Cypress.state('runnable')),
duringUserTestExecution: Cypress.state('duringUserTestExecution'),
hookId: state('hookId'),
},
communicator.once('bridge:ready', (_data, bridgeReadyDomain) => {
if (bridgeReadyDomain === domain) {
Copy link
Member Author

Choose a reason for hiding this comment

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

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.

// 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', {
Copy link
Member Author

Choose a reason for hiding this comment

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

Tell the domain we just created to initialize cypress

config: preprocessConfig(Cypress.config()),
env: preprocessEnv(Cypress.env()),
})
} catch (err: any) {
const wrappedErr = $errUtils.errByPath('switchToDomain.run_domain_fn_errored', {
error: err.message,
})

cleanup()
reject(wrappedErr)
} finally {
// @ts-ignore
cy.isAnticipatingMultiDomain(false)
state('readyForMultiDomain', true)

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

Choose a reason for hiding this comment

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

Just run the domain in question

data,
fn: callbackFn.toString(),
// let the spec bridge version of Cypress know if config read-only values can be overwritten since window.top cannot be accessed in cross-origin iframes
// this should only be used for internal testing. Cast to boolean to guarantee serialization
// @ts-ignore
skipConfigValidation: !!window.top.__cySkipValidateConfig,
state: {
viewportWidth: Cypress.state('viewportWidth'),
viewportHeight: Cypress.state('viewportHeight'),
runnable: serializeRunnable(Cypress.state('runnable')),
duringUserTestExecution: Cypress.state('duringUserTestExecution'),
hookId: state('hookId'),
},
config: preprocessConfig(Cypress.config()),
env: preprocessEnv(Cypress.env()),
})
} catch (err: any) {
const wrappedErr = $errUtils.errByPath('switchToDomain.run_domain_fn_errored', {
error: err.message,
})

reject(wrappedErr)
} finally {
// @ts-ignore
cy.isAnticipatingMultiDomain(false)
}
}
})

// this signals to the runner to create the spec bridge for
// the specified domain
communicator.emit('expect:domain', domain)
Cypress.emit('expect:domain', domain)
mjhenkes marked this conversation as resolved.
Show resolved Hide resolved
}).finally(() => {
cleanup()
mjhenkes marked this conversation as resolved.
Show resolved Hide resolved
})
},
})
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cypress/cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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') })
Copy link
Member Author

Choose a reason for hiding this comment

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

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


signalStable()
} catch (err: any) {
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cypress/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1527,7 +1527,7 @@ export default {
}

cy.state('duringUserTestExecution', false)
Cypress.multiDomainCommunicator.toSpecBridge('sync:state', { 'duringUserTestExecution': false })
Cypress.multiDomainCommunicator.toAllSpecBridges('sync:state', { 'duringUserTestExecution': false })
Copy link
Member Author

Choose a reason for hiding this comment

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

Keep all domains in sync


// our runnable is about to run, so let cy know. this enables
// us to always have a correct runnable set even when we are
Expand Down
32 changes: 23 additions & 9 deletions packages/driver/src/multi-domain/communicator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,20 @@ const preprocessErrorForPostMessage = (value) => {
* @extends EventEmitter
*/
export class PrimaryDomainCommunicator extends EventEmitter {
private windowReference
private crossDomainDriverWindow
private windowReference: Window | undefined
private crossDomainDriverWindows: {[key: string]: Window} = {}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is now a map of domains to windows


/**
* Initializes the event handler to receive messages from the spec bridge.
* @param {Window} win - a reference to the window object in the primary domain.
* @returns {Void}
*/
initialize (win) {
initialize (win: Window) {
if (this.windowReference) return

this.windowReference = win

this.windowReference.top.addEventListener('message', ({ data, source }) => {
this.windowReference.top?.addEventListener('message', ({ data, source }) => {
// currently used for tests, can be removed later
if (data?.actual) return

Expand All @@ -87,11 +87,11 @@ export class PrimaryDomainCommunicator extends EventEmitter {
// NOTE: need a special case here for 'bridge:ready'
// where we need to set the crossDomainDriverWindow to source to
// communicate back to the iframe
if (messageName === 'bridge:ready') {
this.crossDomainDriverWindow = source
if (messageName === 'bridge:ready' && source) {
this.crossDomainDriverWindows[data.domain] = source as Window
}

this.emit(messageName, data.data)
this.emit(messageName, data.data, data.domain)
Copy link
Member Author

Choose a reason for hiding this comment

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

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


return
}
Expand All @@ -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) {
Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Works for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could go with some sort of broadcast naming.

Copy link
Member Author

Choose a reason for hiding this comment

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

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

debug('=> to all spec bridges', event, data)
Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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. 🤦‍♂️

// If there is no crossDomainDriverWindow, there is no need to send the message.
Object.values(this.crossDomainDriverWindows).forEach((win: Window) => {
win.postMessage({
event,
data,
}, '*')
})
}

toSpecBridge (domain: string, event: string, data?: any) {
debug('=> to spec bridge', domain, event, data)
// If there is no crossDomainDriverWindow, there is no need to send the message.
this.crossDomainDriverWindow?.postMessage({
this.crossDomainDriverWindows[domain]?.postMessage({
event,
data,
}, '*')
Expand Down Expand Up @@ -180,12 +192,14 @@ export class SpecBridgeDomainCommunicator extends EventEmitter {
* @param {any} data - any meta data to be sent with the event.
*/
toPrimary (event: string, data?: any, options = { syncConfig: false }) {
debug('<= to Primary ', event, data, location.hostname)
if (options.syncConfig) this.syncConfigEnvToPrimary()

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

Choose a reason for hiding this comment

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

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

}, '*')
})
}
Expand Down
13 changes: 7 additions & 6 deletions packages/runner-shared/src/event-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member Author

Choose a reason for hiding this comment

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

all domains should be notified before the test runs

})

Cypress.multiDomainCommunicator.initialize(window)
Expand All @@ -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) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

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

localBus.emit('expect:domain', domain)
})

Cypress.multiDomainCommunicator.on('before:screenshot', (config) => {
Cypress.multiDomainCommunicator.on('before:screenshot', (config, domain) => {
const callback = () => {
Cypress.multiDomainCommunicator.toSpecBridge('before:screenshot:end')
Cypress.multiDomainCommunicator.toSpecBridge(domain, 'before:screenshot:end')
Copy link
Member Author

Choose a reason for hiding this comment

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

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

}

handleBeforeScreenshot(config, callback)
Expand Down Expand Up @@ -640,8 +640,9 @@ export const eventManager = {
ws.emit('spec:changed', specFile)
},

notifyCrossDomainBridgeReady () {
Cypress.multiDomainCommunicator.emit('bridge:ready')
notifyCrossDomainBridgeReady (domain) {
// Any multi-domain event appends the domain as the third parameter and we do the same here for this short circuit
Cypress.multiDomainCommunicator.emit('bridge:ready', undefined, domain)
},

focusTests () {
Expand Down
27 changes: 20 additions & 7 deletions packages/runner/injection/multi-domain.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,29 @@

import { createTimers } from './timers'

// TODO: don't hard-code the index. need it to be predictable or need
// to search for the right one somehow. will need to be fixed when we
// test out visiting a 3rd domain
const cyBridgeFrame = window.parent.frames[2]

const Cypress = cyBridgeFrame.Cypress
let Cypress

for (let index = 0; index < window.parent.frames.length; index++) {
const frame = window.parent.frames[index]

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

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think it's safe to assume.

Cypress = frame.Cypress
}
mjhenkes marked this conversation as resolved.
Show resolved Hide resolved
} catch (error) {
// Catch DOMException: Blocked a frame from accessing a cross-origin frame.
if (error.name !== 'SecurityError') {
Copy link
Member Author

Choose a reason for hiding this comment

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

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

throw error
}
}
}

// TODO: If the spec bridge is not found we should throw some kind of error to main cypress, this should account for redirects so maybe wait a bit before throwing?
// This may not be needed if we defer to the first command
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.')
Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

}

// the timers are wrapped in the injection code similar to the primary domain
Expand Down
2 changes: 1 addition & 1 deletion packages/runner/src/iframe/iframes.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export default class Iframes extends Component {

// if it already exists, don't add another one
if (document.getElementById(id)) {
this.props.eventManager.notifyCrossDomainBridgeReady()
this.props.eventManager.notifyCrossDomainBridgeReady(domain)

return
}
Expand Down