-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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: Reapply state refactor #23092
Changes from all commits
1f07e9f
dfda706
d1b0fa5
bd04057
988f382
4f07301
930b4da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,11 @@ describe('src/cy/commands/aliasing', () => { | |
}) | ||
}) | ||
|
||
it('stores the lookup as an alias when .then() is an intermediate', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was the minimal repro of the issue causing the todomvc failure, included in the monorepo to prevent any future regression. Explanation about what was going wrong and how it was fixed in another comment. |
||
cy.get('body').then(() => {}).as('body') | ||
cy.get('@body') | ||
}) | ||
|
||
it('stores the resulting subject as the alias', () => { | ||
const $body = cy.$$('body') | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,7 +58,8 @@ describe('src/cy/commands/commands', () => { | |
cy | ||
.get('input:first') | ||
.parent() | ||
.command('login', '[email protected]').then(($input) => { | ||
.command('login', '[email protected]') | ||
.then(($input) => { | ||
expect($input.get(0)).to.eq(input.get(0)) | ||
}) | ||
}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -218,6 +218,24 @@ class $Cypress { | |
_.extend(this, browserInfo(config)) | ||
|
||
this.state = $SetterGetter.create({}) as unknown as StateFunc | ||
|
||
/* | ||
* As part of the Detached DOM effort, we're changing the way subjects are determined in Cypress. | ||
* While we usually consider cy.state() to be internal, in the case of cy.state('subject'), | ||
* cypress-testing-library, one of our most popular plugins, relies on it. | ||
* https://github.com/testing-library/cypress-testing-library/blob/1af9f2f28b2ca62936da8a8acca81fc87e2192f7/src/utils.js#L9 | ||
* | ||
* Therefore, we've added this shim to continue to support them. The library is actively maintained, so this | ||
* shouldn't need to stick around too long (written 07/22). | ||
Comment on lines
+223
to
+229
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice work finding usage and shiming to support cypress-testing-library |
||
*/ | ||
Object.defineProperty(this.state(), 'subject', { | ||
get: () => { | ||
$errUtils.warnByPath('subject.state_subject_deprecated') | ||
|
||
return cy.currentSubject() | ||
}, | ||
}) | ||
|
||
this.originalConfig = _.cloneDeep(config) | ||
this.config = $SetterGetter.create(config, (config) => { | ||
if (this.isCrossOriginSpecBridge ? !window.__cySkipValidateConfig : !window.top!.__cySkipValidateConfig) { | ||
|
@@ -640,9 +658,6 @@ class $Cypress { | |
case 'cy:url:changed': | ||
return this.emit('url:changed', args[0]) | ||
|
||
case 'cy:next:subject:prepared': | ||
return this.emit('next:subject:prepared', ...args) | ||
|
||
case 'cy:collect:run:state': | ||
return this.emitThen('collect:run:state') | ||
|
||
|
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've updated this branch to run the complete set of linux tasks, including the test-binary-against-todomvc-firefox stage which caused to be be reverted previously.