-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
[cypress] Add integration test for area, pie, pivot_table, world_map, dual_line, sunburst, sankey, big_number, bubble, box_plot, treemap #5924
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5924 +/- ##
=========================================
+ Coverage 63.61% 63.72% +0.1%
=========================================
Files 386 386
Lines 23532 23532
Branches 2621 2621
=========================================
+ Hits 14971 14996 +25
+ Misses 8548 8523 -25
Partials 13 13
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #5924 +/- ##
=========================================
+ Coverage 63.61% 63.72% +0.1%
=========================================
Files 386 386
Lines 23532 23532
Branches 2621 2621
=========================================
+ Hits 14971 14996 +25
+ Misses 8548 8523 -25
Partials 13 13
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #5924 +/- ##
==========================================
+ Coverage 63.61% 63.66% +0.04%
==========================================
Files 386 386
Lines 23532 23532
Branches 2621 2621
==========================================
+ Hits 14971 14981 +10
+ Misses 8548 8538 -10
Partials 13 13
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #5924 +/- ##
=========================================
+ Coverage 63.61% 63.72% +0.1%
=========================================
Files 386 386
Lines 23532 23532
Branches 2621 2621
=========================================
+ Hits 14971 14996 +25
+ Misses 8548 8523 -25
Partials 13 13
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #5924 +/- ##
=========================================
+ Coverage 63.61% 63.72% +0.1%
=========================================
Files 386 386
Lines 23532 23532
Branches 2621 2621
=========================================
+ Hits 14971 14996 +25
+ Misses 8548 8523 -25
Partials 13 13
Continue to review full report at Codecov.
|
resample_how: null, | ||
resample_rule: null, | ||
resample_fillmethod: null, | ||
annotation_layers: [], |
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've been trying to reduce the number for form_data params that are hardcoded, just because we'll need to change them when they change. What do you think about reducing this list to the form_data fields that are necessary for the chart to load (removing those that are null).
sqlExpression: null, | ||
fromFormData: true, | ||
filterOptionName: 'filter_tqx1en70hh_7nksse7nqic', | ||
}], |
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.
If you rebase on this change you can import and use the SIMPLE_FILTER
here.
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 decided to keep this one as it is clearer that by selecting gender=boy
it will ensure there is only one pie slice.
f08a0e7
to
4513053
Compare
Codecov Report
@@ Coverage Diff @@
## master #5924 +/- ##
=========================================
+ Coverage 63.61% 63.72% +0.1%
=========================================
Files 386 386
Lines 23532 23532
Branches 2621 2621
=========================================
+ Hits 14971 14996 +25
+ Misses 8548 8523 -25
Partials 13 13
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #5924 +/- ##
=========================================
+ Coverage 63.61% 63.72% +0.1%
=========================================
Files 386 386
Lines 23532 23532
Branches 2621 2621
=========================================
+ Hits 14971 14996 +25
+ Misses 8548 8523 -25
Partials 13 13
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #5924 +/- ##
==========================================
+ Coverage 64.6% 64.65% +0.04%
==========================================
Files 446 446
Lines 23814 23814
Branches 2639 2639
==========================================
+ Hits 15386 15396 +10
+ Misses 8415 8405 -10
Partials 13 13
Continue to review full report at Codecov.
|
7e13b10
to
2374f08
Compare
|
||
it('should work', () => { | ||
verify(TREEMAP_FORM_DATA); | ||
cy.get('.chart-container svg rect.child').should('have.length', 214); |
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.
is this one robust to minor screen size changes? I thought I remembered treemap creating a variable # of rects based on available space, so wanted to make sure this wouldn't start failing if e.g., someone increased or decreased the size of the side panel.
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 checked the code and did not see any logic that reduce number of nodes. One component that does weird stuffs with the children is icicle
, which only display top 5 children, but I don't see that with this treemap.
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 with one question!
f1056b6
to
bd10395
Compare
bd10395
to
2cde5d4
Compare
Much better time than before |
… dual_line, sunburst, sankey, big_number, bubble, box_plot, treemap (apache#5924) * Add integration test for world map * add pie chart * add area * use should for assertion * update area test * update it message * remove null params * add pivot tests * remove urlparams * add dual_line * add sunburst test * add big number * add sankey * add bubble * add box plot * add treemap tests * combine all vis under single test
@michellethomas @williaster @conglei