-
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
misc: update a few cypress types + convert internal driver tests to TypeScript #31055
base: develop
Are you sure you want to change the base?
Conversation
@@ -756,8 +756,10 @@ declare namespace Cypress { | |||
getCoordsByPosition(left: number, top: number, xPosition?: string, yPosition?: string): number | |||
getElementPositioning(element: JQuery | HTMLElement): ElementPositioning | |||
getElementAtPointFromViewport(doc: Document, x: number, y: number): Element | null | |||
getElementCoordinatesByPosition(element: JQuery | HTMLElement, position: string): ElementCoordinates | |||
getElementCoordinatesByPosition(element: JQuery | HTMLElement, position?: string): ElementCoordinates |
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.
position is optional
getHostContenteditable(element: HTMLElement): HTMLElement | ||
getSelectionBounds(element: HTMLElement): { start: number, end: number } |
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.
its pretty strange that we make all of these methods public, but - that's how it's setup, so I added additional types in the public types although they're undocumented.
@@ -167,7 +168,14 @@ describe('src/cy/commands/actions/check', () => { | |||
it('can forcibly click even when being covered by another element', () => { | |||
const checkbox = $('<input type=\'checkbox\' />').attr('id', 'checkbox-covered-in-span').prependTo($('body')) | |||
|
|||
$('<span>span on checkbox</span>').css({ position: 'absolute', left: checkbox.offset().left, top: checkbox.offset().top, padding: 5, display: 'inline-block', backgroundColor: 'yellow' }).prependTo($('body')) | |||
$('<span>span on checkbox</span>') |
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.
It's not valid to pass an object to .css
@@ -282,7 +283,7 @@ describe('src/cy/commands/actions/click', () => { | |||
const button = cy.$$('#button') | |||
|
|||
cy.get('#button').click().then(($button) => { | |||
expect($button).to.match(button) | |||
expect($button[0]).to.eq(button[0]) |
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.
to.match
is only meant to compare via Regex, but we were using this in a lot of places to check equality of elements. It did seem to be passing anyways somehow.
@@ -556,7 +557,7 @@ describe('src/cy/commands/actions/click', () => { | |||
attachMouseHoverListeners({ btn, span }) | |||
|
|||
btn.html('') | |||
btn.attr('disabled', true) | |||
btn.prop('disabled', true) |
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.
disabled with value should be set via prop
@@ -1280,7 +1282,7 @@ describe('src/cy/commands/actions/trigger', () => { | |||
const { lastLog } = this | |||
const { fromElWindow } = Cypress.dom.getElementCoordinatesByPosition($btn) | |||
|
|||
expect(lastLog.get('coords')).to.deep.eq(fromElWindow, 'x', 'y') | |||
expect(lastLog.get('coords')).to.deep.eq(fromElWindow) |
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.
This doesn't accept more than 1 argument and what even were these extra arguments doing - nothing...
@@ -766,6 +772,7 @@ describe('src/cy/commands/actions/type - #type', () => { | |||
.then(() => { | |||
expect(cy.timeout).to.be.calledWith(5 * 8, true, 'type') | |||
|
|||
// @ts-expect-error - TODO: Get this to use the internal Keyboard types |
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 have no idea why some specfiles don't seem to be picking up the internal types I defined for some Cypress definitions.
// @ts-expect-error - TODO: Not sure how to handle this | ||
const spyTableName = cy.spy(top.console, 'group') | ||
// @ts-expect-error - TODO: Not sure how to handle this |
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.
Just complaining that window doesn't have console on it.
/// <reference types="chai-subset" /> | ||
/// <reference types="cypress" /> | ||
|
||
/// <reference path="../../types/internal-types.d.ts" /> |
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 have a feeling there's something extra I need to do to get our internal types in our cypress tests. Sometimes they work and sometimes they don't though.
export const expectCaret = (start: number) => { | ||
return ($el) => { | ||
end = end == null ? start : end | ||
// @ts-ignore | ||
const end = start | ||
|
||
expect(Cypress.dom.getSelectionBounds($el.get(0))).to.deep.eq({ start, end }) | ||
} |
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.
This function is never called with a second argument in our code
_log?: Log | ||
ensure?: object | ||
} | ||
|
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.
Moved to internal types file
…ress-io/cypress into ts-to-js-driver-action-tests
cypress
|
Project |
cypress
|
Branch Review |
ts-to-js-driver-action-tests
|
Run status |
|
Run duration | 27m 59s |
Commit |
|
Committer | Jennifer Shehane |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
9
|
|
1099
|
|
0
|
|
26538
|
View all changes introduced in this branch ↗︎ |
UI Coverage
45.68%
|
|
---|---|
|
190
|
|
164
|
Accessibility
92.54%
|
|
---|---|
|
3 critical
8 serious
2 moderate
2 minor
|
|
890
|
Additional details
Converting some driver tests to TypeScript (more to come!). It's interesting because I'm finding our own types that we expose to users and our internal ones are not ideal to work with ourselves.
Steps to test
Tests should pass
PR Tasks
cypress-documentation
?type definitions
?