-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
fix(cypress): Fix failing/flaky E2E tests #22460
Conversation
…lters" and apply their values.'
Codecov Report
@@ Coverage Diff @@
## master #22460 +/- ##
=======================================
Coverage 66.89% 66.89%
=======================================
Files 1850 1850
Lines 70701 70701
Branches 7750 7750
=======================================
Hits 47292 47292
Misses 21393 21393
Partials 2016 2016
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
LGTM, thanks for taking on this challenge, as CI has been pretty painful lately ❤️
@@ -82,7 +82,7 @@ function prepareDashboardFilters( | |||
}).then(res => { | |||
const { body } = res; | |||
const dashboardId = body.result.id; | |||
const allFilters: Record<string, any>[] = []; | |||
const allFilters: Record<string, unknown>[] = []; |
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 believe these are Filters
:
import { Filters } from '@superset-ui/core';
const allFilters: Record<string, Filters>[] = [];
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, I tried this but it gave me a "Module '"@superset-ui/core"' has no exported member 'Filter'." error. I also didn't see anywhere else in superset-frontend/cypress-base
that imports from @superset-ui/core
– do you know if these packages are supposed to be isolated for any reason?
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.
LGTM!
expect($rows).to.contain('Amy'); | ||
}); | ||
// checking the paginated data | ||
cy.get('.ant-pagination-item') | ||
.should('have.length', 6) | ||
.then($pages => { | ||
.should($pages => { |
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.
Cool, I didn't know that should
waits and then
doesn't!
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.
Me either, this PR was a learning experience!
@@ -202,6 +202,7 @@ function openVerticalFilterBar() { | |||
|
|||
function setFilterBarOrientation(orientation: 'vertical' | 'horizontal') { | |||
cy.getBySel('filterbar-orientation-icon').click(); | |||
cy.wait(250); |
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.
Can we add timeout to getBySel
instead of this wait
? Like cy.getBySel('dropdown-selectable-info', { timeout: 250 })
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.
Actually, not sure which method do we prefer in Superset, so feel free to leave it as is 🙂
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.
Nevermind, I didn't notice that it's merged 😄
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.
Haha no worries! The issue seemed to be that the loading of the page caused some scrolling action while the menu was opening, which disrupted Cypress's ability to click on the menu item even though it is able to find it, so this .wait
hopefully will let the scrolling finish in more cases before Cypress tries to click on the menu item.
Hi @codyml! I have problem with |
SUMMARY
This PR attempts to fix some failing/flaky Cypress tests:
The code changes include:
.then
with.should
calls indrilltodetail.test.ts
to allow for retries within the callback.wait
innativeFilters.test.ts
to give the filter bar orientation menu time to change orientation after scrolling into position.type
from theinput
's parent to theinput
itself in horizontal filter bar tests, as theinput
's parent was at times not considered "visible" by CypressTESTING INSTRUCTIONS
dashboard/drilltodetail.test.ts
dashboard/nativeFilters.test.ts
chart_list/list.test.ts
ADDITIONAL INFORMATION