-
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: remove command type todos #20601
Changes from all commits
bff7f0b
93f2b2b
a0fbd97
e9120ad
9216026
edb7d39
0c62852
ce302e8
585fbe8
b650488
9801c7c
f4adb3a
b17da05
0dfffc6
6674167
9cf79c8
0659e3b
f03249a
95cc205
c8face7
021774c
5493561
6b8c4be
2d4191c
d2b2dd8
136d989
8fb7463
33f0885
ea2126b
b43ca6a
7fcf88a
c8e33fd
35c9445
fc8e114
4bd4536
01de010
cf50abb
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 | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -761,9 +761,9 @@ describe('src/cy/commands/screenshot', () => { | |||||||||||||||||||||||||||||||||
cy.get('.short-element').within(() => { | ||||||||||||||||||||||||||||||||||
cy.screenshot({ capture: 'runner' }) | ||||||||||||||||||||||||||||||||||
}).then(() => { | ||||||||||||||||||||||||||||||||||
// the runner was captured | ||||||||||||||||||||||||||||||||||
expect(Cypress.action.withArgs('cy:before:screenshot').args[0][1].appOnly).to.be.true | ||||||||||||||||||||||||||||||||||
expect(Cypress.automation.withArgs('take:screenshot').args[0][1].capture).to.equal('viewport') | ||||||||||||||||||||||||||||||||||
// the runner was captured ("appOnly === true" means to hide the runner UI) | ||||||||||||||||||||||||||||||||||
expect(Cypress.action.withArgs('cy:before:screenshot').args[0][1].appOnly).to.be.false | ||||||||||||||||||||||||||||||||||
expect(Cypress.automation.withArgs('take:screenshot').args[0][1].capture).to.equal('runner') | ||||||||||||||||||||||||||||||||||
Comment on lines
-764
to
+766
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. According to the definition of cypress/packages/driver/src/cy/commands/screenshot.ts Lines 269 to 272 in b0fe8bf
it means to hide the runner UI. In other words, it should be This weird test failure happened because
cypress/packages/driver/src/cy/commands/screenshot.ts Lines 444 to 455 in b0fe8bf
The code above, when |
||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
|
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.
cy.blur
supportstimeout
but it was missing in types. I added it.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.
hmm, what does
.timeout
actually do incy.blur
? it gets passed to Cypress.log, but shouldn't this be a synchronous operation? maybe we need to remove this if it's a no-op.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 also think it's a synchronous operation and harder to finish blur() over 4 seconds.
But it might be here because we need it for
retry
andverifyAssertions
.And it's also in the documentation. Removing it is a breaking change.
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.
IDK, I feel like if it's a no-op currently, removing the option isn't really breaking. But it's ok, don't need to worry about it here.