Skip to content
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(frontend): uplift Cypress to v13 #29265

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

hainenber
Copy link
Contributor

chore(frontend): uplift Cypress to v13

SUMMARY

Uplift Cypress to currency (v13). Acceptance threshold is all Cypress tests in CI pass with flying colors :D

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@rusackas
Copy link
Member

Hey, thanks for getting into this! This'll enable some cool stuff, including test playback on the Cypress dashboard, which is pretty nifty!

The coordinates to drill the BarChartV2 are much harder due to slender visualization so we have to increase the `y` coordinate to get more chance clicking into correct areas.

This in turns forces the test assertion to be more laxed to handle many states the drill-by can have.

Signed-off-by: hainenber <[email protected]>
[70, 94],
[362, 68],
]);
[362, 474],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's due to Cypress upgrade or else but the previous coordinates were flaky. I have to manually tinker around to pick out the least "flaky" coordinates to pass local CI

@@ -588,7 +719,7 @@ describe('Drill by modal', () => {
it('Gauge Chart', () => {
testEchart('gauge_chart', 'Gauge Chart', [
[151, 95],
[300, 143],
[300, NaN], // this fixes the test after upgrading to Cypress 13
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is completely voodoo magic to me. Previous coordinates didn't work and frustratingly I entered NaN and it worked


// reload dashboard
cy.reload();
cy.get(dataTestChartName(testItems.topTenChart.name)).within(() => {
cy.contains(testItems.filterDefaultValue).should('be.visible');
cy.contains(testItems.filterOtherCountry).should('not.exist');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cy.reload() in Cypress 13 seems to completely remove local cache for stored filter inputs. Lmk if my change here is a fix or a band-aid :D

@@ -37,30 +36,10 @@ export default eyesPlugin(
openMode: 0,
},
e2e: {
experimentalMemoryManagement: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see we are doing { testIsolation: false } in the describe blocks, is there a reason to not just add that to the config instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's due to default test isolation in Cypress 12. While there are many tests sharing context, as evidenced by the amount of { testIsolation: false }, the remaining are isolated and should be encouraged instead.

We'll probably need to refactor these context-sharing tests later to be isolated in future PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants