build: upgrade Cypress and re-enable visualization tests #10158
Conversation
ktmud
changed the title
|
I'm a big fan of this change, but probably the wrong person to review it. Thanks for taking this on. |
| @@ -0,0 +1,122 @@ | |||
| /** | |||
There was a problem hiding this comment.
Are these changes only file renames? It's tough to tell from the diff
There was a problem hiding this comment.
For anything other than visualizations tests, it's just file renames. Had to fix a couple of visualization tests, will add notes shortly.
|
Close and reopen to see if Prefer TypeScript fail is a glitch. |
|
is file name change necessary? I prefer keep change history. |
| verify(HISTOGRAM_FORM_DATA); | ||
| cy.get('.chart-container svg .vx-bar').should( | ||
| 'have.length', | ||
| HISTOGRAM_FORM_DATA.link_length, |
There was a problem hiding this comment.
Changed number of expected bin size.
| }); | ||
|
|
||
| it('should work with filter and update num bins', () => { | ||
| const numBins = 2; |
There was a problem hiding this comment.
When distribution is very long tail, VX histogram sometimes will generate bars < numBins. I use to very small numBins to pass the test.
| verify(PIVOT_TABLE_FORM_DATA); | ||
| cy.get('.chart-container tr:eq(0) th:eq(1)').contains('sum__num'); | ||
| cy.get('.chart-container tr:eq(1) th:eq(0)').contains('state'); | ||
| cy.get('.chart-container tr:eq(2) th:eq(0)').contains('name'); |
There was a problem hiding this comment.
Refactored how most pivot table assertions work. Previously it was something like
cy.get('.chart-container tr th').then(ths => {
expect(ths.find(th => th.innerText.trim() === 'name')).to.not.equal(
undefined,
);
});| ...SUNBURST_FORM_DATA, | ||
| groupby: ['region', 'country_name'], | ||
| }); | ||
| cy.get('.chart-container svg g#arcs path').should('have.length', 117); |
There was a problem hiding this comment.
Had to adjust some of the expected numbers because of library upgrades.
| @@ -18,118 +18,115 @@ | |||
| */ | |||
| import { FORM_DATA_DEFAULTS, NUM_METRIC } from './shared.helper'; | |||
There was a problem hiding this comment.
The Time Table chart in testing is somehow empty. Leaving it as it for future persons who work on time table to debug.
|
|
||
| const level0 = '.chart-container rect[style="fill: rgb(255, 90, 95);"]'; | ||
| const level1 = '.chart-container rect[style="fill: rgb(123, 0, 81);"]'; | ||
| const level2 = '.chart-container rect[style="fill: rgb(0, 122, 135);"]'; |
There was a problem hiding this comment.
Use color to detect lookup different levels of treemap rects. The tests might break if default color palettes updates, which is not ideal, but this is the only way I could find to find rect of different levels. The classnames used in previous tests have stopped working:
cy.get('.chart-container svg rect.parent').should('have.length', 7);
cy.get('.chart-container svg rect.child').should('have.length', 214);
Yes, it's necessary. It's hard to retain the history even if I split this to multiple commits with rename + change because the history will be lost during squash merge anyway.
|
mistercrunch
added
🏷️ bot
SUMMARY
This PR made several changes to Cypress tests:
Upgrade Cypress to 4.7.0
Some E2E tests has been flaky, especially with the "should apply filter" test in dashboard (never ran into an issue while debugging locally, but fails from time to time in CI). I'm reluctant to disable/skip it as it does feel important. Let's see if upgrading Cypress fixes it.
Re-enable visualization tests
End-to-end tests for editing visualizations have been disabled for some time (most likely unintentionally). Let's reenable it to ensure the safety of charting experience.
Also fixed a couple of broken tests.
Split tests into independent
.test.jsfilesThe original intention of combining all tests in a
index.test.jsfile was to speed things up because of overhead of starting Chrome for each file. But it has been verified that latest Cypress has no such overhead.This change will make debugging a specific test easier.
TEST PLAN
CI should pass
ADDITIONAL INFORMATION