fix: only take snapshots if the AUT document is in state #24519
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,107 +2,153 @@ | |||||
| import '../../../support/utils' | ||||||
|
|
||||||
| describe('cy.origin - snapshots', { browser: '!webkit' }, () => { | ||||||
| it('does not create snapshots after the document has unloaded and the AUT has navigated cross-origin', () => { | ||||||
| cy.visit('/fixtures/generic.html') | ||||||
| cy.visit('http://www.foobar.com:3500/fixtures/generic.html') | ||||||
| cy.then(() => { | ||||||
| const snapshot = cy.createSnapshot() | ||||||
|
|
||||||
| expect(snapshot).to.be.null | ||||||
| }) | ||||||
| }) | ||||||
|
|
||||||
| it('takes snapshots from the secondary origin even after the primary AUT has been unloaded from state', () => { | ||||||
| const findLog = (logMap: Map<string, any>, selector) => { | ||||||
| return Array.from(logMap.values()).find((log: any) => { | ||||||
| const props = log.get() | ||||||
|
|
||||||
| return (props?.consoleProps?.Selector === selector) | ||||||
| }) | ||||||
| } | ||||||
| let logs: Map<string, any> = new Map() | ||||||
|
|
||||||
| cy.on('log:changed', (attrs, log) => { | ||||||
| logs.set(attrs.id, log) | ||||||
| }) | ||||||
|
|
||||||
| cy.visit('/fixtures/primary-origin.html') | ||||||
| cy.get('a[data-cy="xhr-fetch-requests-onload"]').click() | ||||||
|
|
||||||
| cy.origin('http://www.foobar.com:3500', () => { | ||||||
| cy.get(`[data-cy="assertion-header"]`) | ||||||
| }) | ||||||
|
|
||||||
| cy.shouldWithTimeout(() => { | ||||||
| const getLogFromSecondaryOrigin = findLog(logs, '[data-cy="assertion-header"]')?.get() | ||||||
|
|
||||||
| expect(getLogFromSecondaryOrigin).to.not.be.undefined | ||||||
|
There was a problem hiding this comment.
Suggested change
There was a problem hiding this comment. I am not sure why but I cannot get this type to show up in a chai assertion. I don't think it is actually valid 🙁 There was a problem hiding this comment. Yeah, doesn't seem to be a bundled chai assertion, but we add on to chai with various libs, so it's possible it ends up as an assertion somehow, since we do use There was a problem hiding this comment. updated to |
||||||
|
|
||||||
| const snapshots = getLogFromSecondaryOrigin.snapshots.map((snapshot) => snapshot.body.get()[0]) | ||||||
|
|
||||||
| expect(snapshots.length).to.equal(1) | ||||||
|
|
||||||
| expect(snapshots[0].querySelector(`[data-cy="assertion-header"]`)).to.have.property('innerText').that.equals('Making XHR and Fetch Requests behind the scenes if fireOnload is true!') | ||||||
| }) | ||||||
| }) | ||||||
|
|
||||||
| describe('e2e log verification', () => { | ||||||
| const findLog = (logMap: Map<string, any>, displayName: string, url: string) => { | ||||||
| return Array.from(logMap.values()).find((log: any) => { | ||||||
| const props = log.get() | ||||||
|
|
||||||
| return props.displayName === displayName && (props?.consoleProps?.URL === url || props?.consoleProps()?.URL === url) | ||||||
| }) | ||||||
| } | ||||||
| let logs: Map<string, any> | ||||||
|
|
||||||
| beforeEach(() => { | ||||||
| logs = new Map() | ||||||
|
|
||||||
| cy.on('log:changed', (attrs, log) => { | ||||||
| logs.set(attrs.id, log) | ||||||
| }) | ||||||
|
|
||||||
| cy.fixture('foo.bar.baz.json').then((fooBarBaz) => { | ||||||
| cy.intercept('GET', '/foo.bar.baz.json', { body: fooBarBaz }).as('fooBarBaz') | ||||||
| }) | ||||||
|
|
||||||
| cy.visit('/fixtures/primary-origin.html') | ||||||
| cy.get('a[data-cy="xhr-fetch-requests-onload"]').click() | ||||||
| }) | ||||||
|
|
||||||
| // TODO: the xhr event is showing up twice in the log, which is wrong and causing flake. skipping until: https://github.com/cypress-io/cypress/issues/23840 is addressed. | ||||||
| it.skip('verifies XHR requests made while a secondary origin is active eventually update with snapshots of the secondary origin', () => { | ||||||
| cy.origin('http://www.foobar.com:3500', () => { | ||||||
| // need to set isInteractive in the spec bridge in order to take xhr snapshots in run mode, similar to how isInteractive is set within support/defaults.js | ||||||
| // @ts-ignore | ||||||
| Cypress.config('isInteractive', true) | ||||||
| cy.visit('http://www.foobar.com:3500/fixtures/xhr-fetch-requests.html') | ||||||
| cy.get(`[data-cy="assertion-header"]`).should('exist') | ||||||
| cy.wait('@fooBarBaz') | ||||||
| }) | ||||||
|
|
||||||
| cy.shouldWithTimeout(() => { | ||||||
| const xhrLogFromSecondaryOrigin = findLog(logs, 'xhr', 'http://localhost:3500/foo.bar.baz.json')?.get() | ||||||
|
|
||||||
| expect(xhrLogFromSecondaryOrigin).to.not.be.undefined | ||||||
|
|
||||||
| const snapshots = xhrLogFromSecondaryOrigin.snapshots.map((snapshot) => snapshot.body.get()[0]) | ||||||
|
|
||||||
| expect(snapshots.length).to.equal(2) | ||||||
|
|
||||||
| // TODO: Since we have two events, one of them does not have a request snapshot | ||||||
|
|
||||||
| expect(snapshots[1].querySelector(`[data-cy="assertion-header"]`)).to.have.property('innerText').that.equals('Making XHR and Fetch Requests behind the scenes if fireOnload is true!') | ||||||
| }) | ||||||
| }) | ||||||
|
|
||||||
| // TODO: fix flaky test https://github.com/cypress-io/cypress/issues/23437 | ||||||
| it.skip('verifies fetch requests made while a secondary origin is active eventually update with snapshots of the secondary origin', () => { | ||||||
|
There was a problem hiding this comment. Are we able to unskip these tests? Maybe add some retries? There was a problem hiding this comment. Let me revisit this and see what the current state is There was a problem hiding this comment. So I can get the tests to pass, but from the flake report the flake is fairly rare, like 2%. I think by default we already retry twice in CI, so I doubt it will fix the problem immediately. @mjhenkes and I were able to reproduce this a few months back, and if I remember correctly there were collisions with the proxy logging on xhr request. I could be completely wrong though. It's been a while 😅 There was a problem hiding this comment. it's possible this gets resolved by removing server/route, but i'm not 100%. I vote we revisit the flake tests after 12.0.0 |
||||||
| cy.origin('http://www.foobar.com:3500', () => { | ||||||
| // need to set isInteractive in the spec bridge in order to take xhr snapshots in run mode, similar to how isInteractive is set within support/defaults.js | ||||||
| // @ts-ignore | ||||||
| Cypress.config('isInteractive', true) | ||||||
| cy.visit('http://www.foobar.com:3500/fixtures/xhr-fetch-requests.html') | ||||||
| cy.get(`[data-cy="assertion-header"]`).should('exist') | ||||||
| cy.wait('@fooBarBaz') | ||||||
| }) | ||||||
|
|
||||||
| cy.shouldWithTimeout(() => { | ||||||
| const xhrLogFromSecondaryOrigin = findLog(logs, 'fetch', 'http://localhost:3500/foo.bar.baz.json')?.get() | ||||||
|
|
||||||
| expect(xhrLogFromSecondaryOrigin).to.not.be.undefined | ||||||
|
|
||||||
| const snapshots = xhrLogFromSecondaryOrigin.snapshots.map((snapshot) => snapshot.body.get()[0]) | ||||||
|
|
||||||
| snapshots.forEach((snapshot) => { | ||||||
| expect(snapshot.querySelector(`[data-cy="assertion-header"]`)).to.have.property('innerText').that.equals('Making XHR and Fetch Requests behind the scenes if fireOnload is true!') | ||||||
| }) | ||||||
| }) | ||||||
| }) | ||||||
|
|
||||||
| it('Does not take snapshots of XHR/fetch requests from secondary origin if the wrong origin is / origin mismatch, but instead the primary origin (existing behavior)', { | ||||||
|
There was a problem hiding this comment. I read this 2x and am a little confused There was a problem hiding this comment. I think this is missing the word There was a problem hiding this comment. addressed in 1d43234 |
||||||
| defaultCommandTimeout: 50, | ||||||
| }, | ||||||
| (done) => { | ||||||
| cy.on('fail', () => { | ||||||
| const xhrLogFromSecondaryOrigin = findLog(logs, 'fetch', 'http://localhost:3500/foo.bar.baz.json')?.get() | ||||||
|
|
||||||
| expect(xhrLogFromSecondaryOrigin).to.not.be.undefined | ||||||
|
|
||||||
| const snapshots = xhrLogFromSecondaryOrigin.snapshots.map((snapshot) => snapshot.body.get()[0]) | ||||||
|
|
||||||
| snapshots.forEach((snapshot) => { | ||||||
| expect(snapshot.querySelector(`[data-cy="assertion-header"]`)).to.be.null | ||||||
| }) | ||||||
|
|
||||||
| done() | ||||||
| }) | ||||||
|
|
||||||
| cy.visit('http://www.foobar.com:3500/fixtures/xhr-fetch-requests.html') | ||||||
|
|
||||||
| cy.origin('http://www.barbaz.com:3500', () => { | ||||||
| // need to set isInteractive in the spec bridge in order to take xhr snapshots in run mode, similar to how isInteractive is set within support/defaults.js | ||||||
| // @ts-ignore | ||||||
| Cypress.config('isInteractive', true) | ||||||
|
|
||||||
| cy.get(`[data-cy="assertion-header"]`).should('exist') | ||||||
| cy.wait('@fooBarBaz') | ||||||
|
There was a problem hiding this comment. Is this failing because the alias does not exist? There was a problem hiding this comment. NVM coming from beforeEach |
||||||
| }) | ||||||
| }) | ||||||
| }) | ||||||
| }) | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -229,6 +229,11 @@ export const create = ($$: $Cy['$$'], state: StateFunc) => { | |
|
|
||
| const createSnapshot = (name, $elToHighlight, preprocessedSnapshot) => { | ||
| Cypress.action('cy:snapshot', name) | ||
| // only take a snapshot if the AUT document is in state to prevent cypress-in-cypress like snapshots | ||
| // or if the snapshot has been taken in a spec bridge and has already been preprocessed | ||
AtofStryker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (!preprocessedSnapshot && !state('document')) { | ||
| return null | ||
| } | ||
|
Comment on lines
+238
to
+240
There was a problem hiding this comment. Does this fix just mean there are more missing screenshots? Can you share a few scenarios this is currently broken? I'd like to test the before & after locally. Does the comment on line 249 no long valid? This is where we are falling back to There was a problem hiding this comment.
I didn't realize the updated tests were specific to origin tests for snapshot. Will be exicted to merge these someday 😄 There was a problem hiding this comment.
That's a good question. Seems like these lines should be updated/removed.
I think if there's no There was a problem hiding this comment. This actually brings up a good point, but I think it needs to stay. Main reason is logs streaming in from the secondary need to be cloned off a document, which in this case the primary document state is unloaded. But the backup isn't the AUT document in this case, rather the actual top document, which is OK since we only using the window to clone the node. If anything, we should probably use I'll play around with this to make sure it works as expect and will add a comment to explain this logic There was a problem hiding this comment.
@emilyrohrbough you can run the snapshot tests from the driver locally, but the impact isn't exactly super noticeable there. Do you have There was a problem hiding this comment. Added comments in 228cef0. I kept the current logic of using loaded document in state first before the specbridge/root document because of how many edge cases exist in snapshot code that could potentially break, but not be caught by this change. Here be 🐉 s. There was a problem hiding this comment. I can easily setup There was a problem hiding this comment. If you run the |
||
|
|
||
| try { | ||
| const { | ||
|
|
||
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.
What log is this associated with?
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.
In this case ,we are just digging out the
[data-cy="assertion-header"]get log to make sure the snapshot is sent back over and not omitted with our new logic changesThere 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 that associated with the origin command log or an xhr log?
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.
origin command log since the get is invoked inside the cy.origin