You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/09/24 17:51:29 UTC

[GitHub] [incubator-superset] adam-stasiak opened a new pull request #11049: [WIP] chore:Dashboard cypress refactor

adam-stasiak opened a new pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049


   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   Cypress selectors instead of class selectors in dashboard. It is still WIP- some tests remains to be moved.
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TEST PLAN
   <!--- What steps should be taken to verify the changes -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] rusackas commented on a change in pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#discussion_r497985101



##########
File path: superset-frontend/src/dashboard/components/SaveModal.jsx
##########
@@ -193,7 +193,11 @@ class SaveModal extends React.PureComponent {
         }
         modalFooter={
           <div>
-            <Button buttonStyle="primary" onClick={this.saveDashboard}>
+            <Button
+              data-test="modal-save-button"

Review comment:
       This is a good example... the attribute seems like a sensible name. `modal-save-dashboard-button` would be even more explicit, but this seems nice and easy to search for and reason about.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#issuecomment-700536352


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=h1) Report
   > Merging [#11049](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/8b458ac17297fc2af2f4bba703b097425f4e7414?el=desc) will **increase** coverage by `0.21%`.
   > The diff coverage is `75.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11049/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11049      +/-   ##
   ==========================================
   + Coverage   61.36%   61.57%   +0.21%     
   ==========================================
     Files         383      816     +433     
     Lines       24188    38589   +14401     
     Branches        0     3650    +3650     
   ==========================================
   + Hits        14842    23763    +8921     
   - Misses       9346    14646    +5300     
   - Partials        0      180     +180     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #javascript | `62.18% <75.00%> (?)` | |
   | #python | `61.22% <ø> (-0.15%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/components/DeleteModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGVsZXRlTW9kYWwudHN4) | `100.00% <ø> (ø)` | |
   | [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <ø> (ø)` | |
   | [superset-frontend/src/components/FaveStar.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRmF2ZVN0YXIudHN4) | `80.95% <ø> (ø)` | |
   | [...rset-frontend/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvTGlzdFZpZXcudHN4) | `96.00% <ø> (ø)` | |
   | [...ontend/src/components/ListView/TableCollection.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvVGFibGVDb2xsZWN0aW9uLnRzeA==) | `100.00% <ø> (ø)` | |
   | [...set-frontend/src/components/ListViewCard/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXdDYXJkL2luZGV4LnRzeA==) | `93.47% <ø> (ø)` | |
   | [superset-frontend/src/components/Modal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTW9kYWwudHN4) | `94.11% <ø> (ø)` | |
   | [...frontend/src/dashboard/components/AddSliceCard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0FkZFNsaWNlQ2FyZC5qc3g=) | `50.00% <0.00%> (ø)` | |
   | [.../src/dashboard/components/BuilderComponentPane.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0J1aWxkZXJDb21wb25lbnRQYW5lLmpzeA==) | `33.33% <ø> (ø)` | |
   | [...tend/src/dashboard/components/DashboardBuilder.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZEJ1aWxkZXIuanN4) | `77.27% <ø> (ø)` | |
   | ... and [455 more](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=footer). Last update [8b458ac...92b33c0](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#issuecomment-700536352


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=h1) Report
   > Merging [#11049](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/8b458ac17297fc2af2f4bba703b097425f4e7414?el=desc) will **increase** coverage by `0.22%`.
   > The diff coverage is `75.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11049/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11049      +/-   ##
   ==========================================
   + Coverage   61.36%   61.58%   +0.22%     
   ==========================================
     Files         383      816     +433     
     Lines       24188    38589   +14401     
     Branches        0     3650    +3650     
   ==========================================
   + Hits        14842    23764    +8922     
   - Misses       9346    14645    +5299     
   - Partials        0      180     +180     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #javascript | `62.18% <75.00%> (?)` | |
   | #python | `61.22% <ø> (-0.14%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/components/DeleteModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGVsZXRlTW9kYWwudHN4) | `100.00% <ø> (ø)` | |
   | [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <ø> (ø)` | |
   | [superset-frontend/src/components/FaveStar.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRmF2ZVN0YXIudHN4) | `80.95% <ø> (ø)` | |
   | [...rset-frontend/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvTGlzdFZpZXcudHN4) | `96.00% <ø> (ø)` | |
   | [...ontend/src/components/ListView/TableCollection.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvVGFibGVDb2xsZWN0aW9uLnRzeA==) | `100.00% <ø> (ø)` | |
   | [...set-frontend/src/components/ListViewCard/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXdDYXJkL2luZGV4LnRzeA==) | `93.47% <ø> (ø)` | |
   | [superset-frontend/src/components/Modal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTW9kYWwudHN4) | `94.11% <ø> (ø)` | |
   | [...frontend/src/dashboard/components/AddSliceCard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0FkZFNsaWNlQ2FyZC5qc3g=) | `50.00% <0.00%> (ø)` | |
   | [.../src/dashboard/components/BuilderComponentPane.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0J1aWxkZXJDb21wb25lbnRQYW5lLmpzeA==) | `33.33% <ø> (ø)` | |
   | [...tend/src/dashboard/components/DashboardBuilder.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZEJ1aWxkZXIuanN4) | `77.27% <ø> (ø)` | |
   | ... and [454 more](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=footer). Last update [8b458ac...796aae0](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#issuecomment-700536352


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=h1) Report
   > Merging [#11049](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/8b458ac17297fc2af2f4bba703b097425f4e7414?el=desc) will **increase** coverage by `0.33%`.
   > The diff coverage is `75.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11049/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11049      +/-   ##
   ==========================================
   + Coverage   61.36%   61.69%   +0.33%     
   ==========================================
     Files         383      816     +433     
     Lines       24188    38589   +14401     
     Branches        0     3650    +3650     
   ==========================================
   + Hits        14842    23806    +8964     
   - Misses       9346    14603    +5257     
   - Partials        0      180     +180     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #javascript | `62.18% <75.00%> (?)` | |
   | #python | `61.39% <ø> (+0.03%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/components/DeleteModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGVsZXRlTW9kYWwudHN4) | `100.00% <ø> (ø)` | |
   | [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <ø> (ø)` | |
   | [superset-frontend/src/components/FaveStar.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRmF2ZVN0YXIudHN4) | `80.95% <ø> (ø)` | |
   | [...rset-frontend/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvTGlzdFZpZXcudHN4) | `96.00% <ø> (ø)` | |
   | [...ontend/src/components/ListView/TableCollection.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvVGFibGVDb2xsZWN0aW9uLnRzeA==) | `100.00% <ø> (ø)` | |
   | [...set-frontend/src/components/ListViewCard/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXdDYXJkL2luZGV4LnRzeA==) | `93.47% <ø> (ø)` | |
   | [superset-frontend/src/components/Modal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTW9kYWwudHN4) | `94.11% <ø> (ø)` | |
   | [...frontend/src/dashboard/components/AddSliceCard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0FkZFNsaWNlQ2FyZC5qc3g=) | `50.00% <0.00%> (ø)` | |
   | [.../src/dashboard/components/BuilderComponentPane.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0J1aWxkZXJDb21wb25lbnRQYW5lLmpzeA==) | `33.33% <ø> (ø)` | |
   | [...tend/src/dashboard/components/DashboardBuilder.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZEJ1aWxkZXIuanN4) | `77.27% <ø> (ø)` | |
   | ... and [451 more](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=footer). Last update [8b458ac...796aae0](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] codecov-commenter commented on pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#issuecomment-700536352


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=h1) Report
   > Merging [#11049](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/8b458ac17297fc2af2f4bba703b097425f4e7414?el=desc) will **decrease** coverage by `0.07%`.
   > The diff coverage is `75.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/11049/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #11049      +/-   ##
   ==========================================
   - Coverage   61.36%   61.28%   -0.08%     
   ==========================================
     Files         383      816     +433     
     Lines       24188    38589   +14401     
     Branches        0     3650    +3650     
   ==========================================
   + Hits        14842    23648    +8806     
   - Misses       9346    14761    +5415     
   - Partials        0      180     +180     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #javascript | `62.18% <75.00%> (?)` | |
   | #python | `60.74% <ø> (-0.62%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset-frontend/src/components/DeleteModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGVsZXRlTW9kYWwudHN4) | `100.00% <ø> (ø)` | |
   | [superset-frontend/src/components/EditableTitle.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS50c3g=) | `74.28% <ø> (ø)` | |
   | [superset-frontend/src/components/FaveStar.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRmF2ZVN0YXIudHN4) | `80.95% <ø> (ø)` | |
   | [...rset-frontend/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvTGlzdFZpZXcudHN4) | `96.00% <ø> (ø)` | |
   | [...ontend/src/components/ListView/TableCollection.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvVGFibGVDb2xsZWN0aW9uLnRzeA==) | `100.00% <ø> (ø)` | |
   | [...set-frontend/src/components/ListViewCard/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXdDYXJkL2luZGV4LnRzeA==) | `93.47% <ø> (ø)` | |
   | [superset-frontend/src/components/Modal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTW9kYWwudHN4) | `94.11% <ø> (ø)` | |
   | [...frontend/src/dashboard/components/AddSliceCard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0FkZFNsaWNlQ2FyZC5qc3g=) | `50.00% <0.00%> (ø)` | |
   | [.../src/dashboard/components/BuilderComponentPane.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0J1aWxkZXJDb21wb25lbnRQYW5lLmpzeA==) | `33.33% <ø> (ø)` | |
   | [...tend/src/dashboard/components/DashboardBuilder.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZEJ1aWxkZXIuanN4) | `77.27% <ø> (ø)` | |
   | ... and [457 more](https://codecov.io/gh/apache/incubator-superset/pull/11049/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=footer). Last update [8b458ac...92b33c0](https://codecov.io/gh/apache/incubator-superset/pull/11049?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] rusackas commented on a change in pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#discussion_r497987679



##########
File path: superset-frontend/src/explore/components/PropertiesModal.tsx
##########
@@ -249,10 +250,17 @@ function PropertiesModal({ slice, onHide, onSave }: InternalProps) {
         </Row>
       </Modal.Body>
       <Modal.Footer>
-        <Button type="button" buttonSize="sm" onClick={onHide} cta>
+        <Button
+          data-test="cancel-button"

Review comment:
       With so many cancel buttons in Superset, do you think this may benefit from being more specific, e.g. `properties-modal-cancel-button`? Might not be a big deal with some of these, but I'm thinking it's better to err on the side of specificity, so we know it's the _right_ cancel button (or whatever) in longer, more complex tests.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] rusackas commented on a change in pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#discussion_r497982477



##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js
##########
@@ -23,47 +23,55 @@ describe('Dashboard edit mode', () => {
     cy.server();
     cy.login();
     cy.visit(WORLD_HEALTH_DASHBOARD);
-    cy.get('.dashboard-header [data-test=edit-alt]').click();
+    cy.get('[data-test="dashboard-header"]').find('[data-test=pencil]').click();
   });
 
   it('remove, and add chart flow', () => {
     // wait for box plot to appear
-    cy.get('.grid-container .box_plot');
+    cy.get('[data-test="grid-container"]').find('.box_plot');
 
-    cy.get('.fa.fa-trash')
+    cy.get('[data-test="icon-button"]')
       .last()
       .then($el => {
         cy.wrap($el).invoke('show').click();
         // box plot should be gone
-        cy.get('.grid-container .box_plot').should('not.exist');
+        cy.get('[data-test="grid-container"]')
+          .find('.box_plot')
+          .should('not.exist');
       });
 
-    cy.get('.tabs-components .nav-tabs li a').contains('Charts').click();
+    cy.get('[data-test="tabs-component"]').children().siblings().last().click();

Review comment:
       Curious your thoughts, but to me it makes sense to have very specific data-test values on the tested elements, like the specific 'a' tag here. The idea here is that the `children().siblings().last()` chain is fragile, if we're going through and moving around frontend components.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] adam-stasiak commented on a change in pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
adam-stasiak commented on a change in pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#discussion_r505006582



##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js
##########
@@ -23,47 +23,55 @@ describe('Dashboard edit mode', () => {
     cy.server();
     cy.login();
     cy.visit(WORLD_HEALTH_DASHBOARD);
-    cy.get('.dashboard-header [data-test=edit-alt]').click();
+    cy.get('[data-test="dashboard-header"]').find('[data-test=pencil]').click();
   });
 
   it('remove, and add chart flow', () => {
     // wait for box plot to appear
-    cy.get('.grid-container .box_plot');
+    cy.get('[data-test="grid-container"]').find('.box_plot');
 
-    cy.get('.fa.fa-trash')
+    cy.get('[data-test="icon-button"]')
       .last()
       .then($el => {
         cy.wrap($el).invoke('show').click();
         // box plot should be gone
-        cy.get('.grid-container .box_plot').should('not.exist');
+        cy.get('[data-test="grid-container"]')
+          .find('.box_plot')
+          .should('not.exist');
       });
 
-    cy.get('.tabs-components .nav-tabs li a').contains('Charts').click();
+    cy.get('[data-test="tabs-component"]').children().siblings().last().click();
 
     // wait for tab-switching animation to complete
     cy.wait(1000);
 
     // find box plot is available from list
-    cy.get('.tabs-components')
-      .find('.chart-card-container')
+    cy.get('[data-test="dragdroppable-object"]')
+      .find('[data-test="card-title"]')

Review comment:
       it is for better reflecting components structure. In object dragdroppable-object I expect to find card-title. Just a pattern to better see changes in case of some components breakup




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] rusackas commented on a change in pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#discussion_r497981738



##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js
##########
@@ -23,47 +23,55 @@ describe('Dashboard edit mode', () => {
     cy.server();
     cy.login();
     cy.visit(WORLD_HEALTH_DASHBOARD);
-    cy.get('.dashboard-header [data-test=edit-alt]').click();
+    cy.get('[data-test="dashboard-header"]').find('[data-test=pencil]').click();
   });
 
   it('remove, and add chart flow', () => {
     // wait for box plot to appear
-    cy.get('.grid-container .box_plot');
+    cy.get('[data-test="grid-container"]').find('.box_plot');
 
-    cy.get('.fa.fa-trash')
+    cy.get('[data-test="icon-button"]')

Review comment:
       Wondering if this should be more specific. There might be a _lot_ of icon buttons on the page. If we put the data-test attribute on the exact one we want to test, e.g. `[data-test="delete-grid-container-button"]` then it's easier to reason about, and you don't need to worry about the fragility of using a `.last()`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] rusackas commented on a change in pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#discussion_r497983453



##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/markdown.test.ts
##########
@@ -30,35 +30,44 @@ describe('Dashboard edit markdown', () => {
     cy.get('script').then(nodes => {
       numScripts = nodes.length;
     });
-    cy.get('.dashboard-header [data-test=edit-alt]').click();

Review comment:
       I see there are a lot of these 'edit-alt' ones that got converted to 'pencil' in the changes somehow... maybe an artifact of rebasing?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] rusackas commented on a change in pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#discussion_r497985311



##########
File path: superset-frontend/src/dashboard/components/PropertiesModal.jsx
##########
@@ -338,7 +339,13 @@ class PropertiesModal extends React.PureComponent {
               >
                 {saveLabel}
               </Button>
-              <Button type="button" buttonSize="sm" onClick={onHide} cta>
+              <Button
+                data-test="cancel-button"

Review comment:
       This makes me a little nervous since there must be a hundred cancel buttons in the app. Something like `cancel-properties-modal-button` would be much more explicit and easy to trace.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] rusackas commented on a change in pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#discussion_r497981074



##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js
##########
@@ -23,47 +23,55 @@ describe('Dashboard edit mode', () => {
     cy.server();
     cy.login();
     cy.visit(WORLD_HEALTH_DASHBOARD);
-    cy.get('.dashboard-header [data-test=edit-alt]').click();
+    cy.get('[data-test="dashboard-header"]').find('[data-test=pencil]').click();

Review comment:
       Hmmm... wondering why this is switching back to 'pencil' when the Icon itself was renamed to 'edit-alt'




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] rusackas commented on a change in pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#discussion_r497982714



##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/edit_mode.test.js
##########
@@ -23,47 +23,55 @@ describe('Dashboard edit mode', () => {
     cy.server();
     cy.login();
     cy.visit(WORLD_HEALTH_DASHBOARD);
-    cy.get('.dashboard-header [data-test=edit-alt]').click();
+    cy.get('[data-test="dashboard-header"]').find('[data-test=pencil]').click();
   });
 
   it('remove, and add chart flow', () => {
     // wait for box plot to appear
-    cy.get('.grid-container .box_plot');
+    cy.get('[data-test="grid-container"]').find('.box_plot');
 
-    cy.get('.fa.fa-trash')
+    cy.get('[data-test="icon-button"]')
       .last()
       .then($el => {
         cy.wrap($el).invoke('show').click();
         // box plot should be gone
-        cy.get('.grid-container .box_plot').should('not.exist');
+        cy.get('[data-test="grid-container"]')
+          .find('.box_plot')
+          .should('not.exist');
       });
 
-    cy.get('.tabs-components .nav-tabs li a').contains('Charts').click();
+    cy.get('[data-test="tabs-component"]').children().siblings().last().click();
 
     // wait for tab-switching animation to complete
     cy.wait(1000);
 
     // find box plot is available from list
-    cy.get('.tabs-components')
-      .find('.chart-card-container')
+    cy.get('[data-test="dragdroppable-object"]')
+      .find('[data-test="card-title"]')

Review comment:
       Is the `find()` necessary if the data-test selector is specific enough?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] rusackas commented on a change in pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049#discussion_r497980605



##########
File path: superset-frontend/cypress-base/cypress/integration/chart_list/card_view.test.ts
##########
@@ -71,37 +73,40 @@ describe('chart card view', () => {
     cy.get('.Select__control').last().should('be.visible');
     cy.get('.Select__control').last().click();
     cy.get('.Select__menu').contains('Alphabetical').click();
-    cy.get('.chart-list-view').should('be.visible');
-    cy.get('.ant-card').first().contains('% Rural');
+    cy.get('[data-test="chart-list-view"]').should('be.visible');
+    cy.get('[data-test="styled-card"]').first().contains('% Rural');
 
     // sort Recently Modified
     cy.get('.Select__control').last().should('be.visible');
     cy.get('.Select__control').last().click();
     cy.get('.Select__menu').contains('Recently Modified').click();
-    cy.get('.chart-list-view').should('be.visible');
-    cy.get('.ant-card').first().contains('Unicode Cloud');
-    cy.get('.ant-card').last().contains('Life Expectancy VS Rural %');
+    cy.get('[data-test="chart-list-view"]').should('be.visible');
+    cy.get('[data-test="styled-card"]').first().contains('Unicode Cloud');
+    cy.get('[data-test="styled-card"]')
+      .last()
+      .contains('Life Expectancy VS Rural %');
   });
 
   it('should delete correctly', () => {
     // show delete modal
-    cy.get('.ant-dropdown-trigger').last().trigger('mouseover');
-    cy.get('.ant-dropdown-menu-item').contains('Delete').should('exist');
-    cy.get('.ant-dropdown-menu-item').contains('Delete').click();
-    cy.get('.modal-dialog').should('be.visible');
-    cy.get('.modal-dialog .btn-danger').should('have.attr', 'disabled');
-    cy.get(".modal-dialog input[id='delete']").type('DELETE');
-    cy.get('.modal-dialog .btn-danger').should('not.have.attr', 'disabled');
-    cy.get('.modal-dialog .btn-default').contains('Cancel').click();
+    cy.get('[data-test="more-horiz"]').last().trigger('mouseover');
+    cy.get('[data-test="delete-option"]').contains('Delete').should('exist');
+    cy.get('[data-test="delete-option"]').contains('Delete').click();
+    cy.get('[data-test="modal-footer"]').should('exist');
+    cy.get('[data-test="delete-button"]').should('have.attr', 'disabled');
+    cy.get('[data-test="modal-body"]').should('exist');
+    cy.get("[data-test='delete-input']").type('DELETE');
+    cy.get('[data-test="delete-button"]').should('not.have.attr', 'disabled');
+    cy.get('[data-test="cancel-button"]').click();
   });
 
   it('should edit correctly', () => {
     // show edit modal
-    cy.get('.ant-dropdown-trigger').last().trigger('mouseover');
-    cy.get('.ant-dropdown-menu-item').contains('Edit').should('exist');
-    cy.get('.ant-dropdown-menu-item').contains('Edit').click();
-    cy.get('.modal-dialog').should('be.visible');
-    cy.get('.modal-dialog input[name="name"]').should('not.have.value');
-    cy.get('.modal-dialog .btn-default').contains('Cancel').click();
+    cy.get('[data-test="more-horiz"]').last().trigger('mouseover');
+    cy.get('[data-test="edit-option"]').contains('Edit').should('exist');
+    cy.get('[data-test="edit-option"]').contains('Edit').click();
+    cy.get('[data-test="edit-modal"]').should('exist');

Review comment:
       As @eschutho pointed out, I'm also curious if 'exist' has any advantages here over 'be.visible'




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] adam-stasiak closed pull request #11049: chore: Dashboard cypress refactor

Posted by GitBox <gi...@apache.org>.
adam-stasiak closed pull request #11049:
URL: https://github.com/apache/incubator-superset/pull/11049


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org