-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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 tests reliability improvements #19800
fix: Cypress tests reliability improvements #19800
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19800 +/- ##
==========================================
+ Coverage 66.40% 66.55% +0.14%
==========================================
Files 1690 1692 +2
Lines 64732 64801 +69
Branches 6656 6657 +1
==========================================
+ Hits 42987 43128 +141
+ Misses 20046 19973 -73
- Partials 1699 1700 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
53b094c
to
3157c1e
Compare
d7c2583
to
1ba4e09
Compare
1ba4e09
to
bb12964
Compare
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.
THANK YOU!
SUMMARY
This PR aims to solve the intermittent issues we're having with our integration tests, to make them deterministic.
Exploration & findings
The issue we're having, seemingly randomly, is with one of the following:
AdhocFilters
AdhocMetrics
Advanced analytics
Annotations
Some examples of these tests failing in the past:
Looking at several of these errors, the snapshots show something very similar to each other:
The error
The URL is missing the dataset_id or slice_id parameters.
occurs when these params are not found.Back to the tests, we're trying to explore the same chart,
Num Births Trend
, by performing the following action:In turn,
visitChartByName
, does the following:So, we try to fetch the slice information by name, to get the id and route to the explore.
The
/chart/api/read
works (aka, returns 200) for the failed tests, but, it returns 0 results.So, that's why we're seeing the error, and why the test is failing.
Root cause
The chart we're trying to access exists, and the test is correct.
If it's not deterministic then, it must be that other test is messing with the data these tests are using.
I can think of two potential causes:
Investigating, I found a test inside
chart list view
suite that performs a bulk delete, which seems to be the culprit.This test bulk deletes the first two charts.
This explains the issue and why the tests are non-deterministic:
This happens because, when we load the chart list view, the charts are ordered by modification date, which means, if other test modified the
Num Births Trend
chart before the bulk delete, then that chart is getting deleted.Solution
If we sort the chart list view by name, then we ensure this test always deletes the same charts, and it does not impact the charts other tests rely on.
Note
Our integration tests have dependencies (whether expected or not) and we need to consider/restructure the execution to ensure we get the same deterministic result, always.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION