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/06/26 07:39:57 UTC

[GitHub] [incubator-superset] ktmud opened a new pull request #10170: build: enable typescript for cypress

ktmud opened a new pull request #10170:
URL: https://github.com/apache/incubator-superset/pull/10170


   ### SUMMARY
   
   Follow up #10161 :
   
   - Enable TypeScript for Cypress
   - Convert support files to TS
   - Convert an example test (dashboard/filter.test.js) to TS
   - Upgrade TypeScript for both `cypress-base` and `superset-frontend`
   - Reenable Prefer TS check for `cypress-base`
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A
   
   ### TEST PLAN
   
   All Cypress tests should still be valid and pass.
   
   
   cc @etr2460 
   


----------------------------------------------------------------
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 #10170: build: enable typescript for cypress

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


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10170?src=pr&el=h1) Report
   > Merging [#10170](https://codecov.io/gh/apache/incubator-superset/pull/10170?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d8314eeb0d8afd4797b5b7ccce1b0c4fb1c16d0d&el=desc) will **decrease** coverage by `0.15%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/10170/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/10170?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #10170      +/-   ##
   ==========================================
   - Coverage   70.55%   70.39%   -0.16%     
   ==========================================
     Files         594      190     -404     
     Lines       31464    18356   -13108     
     Branches     3226        0    -3226     
   ==========================================
   - Hits        22198    12922    -9276     
   + Misses       9150     5434    -3716     
   + Partials      116        0     -116     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #cypress | `?` | |
   | #javascript | `?` | |
   | #python | `70.39% <ø> (+0.03%)` | :arrow_up: |
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10170?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/sql\_parse.py](https://codecov.io/gh/apache/incubator-superset/pull/10170/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `99.30% <0.00%> (-0.01%)` | :arrow_down: |
   | [superset/errors.py](https://codecov.io/gh/apache/incubator-superset/pull/10170/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXJyb3JzLnB5) | `100.00% <0.00%> (ø)` | |
   | [superset/viz\_sip38.py](https://codecov.io/gh/apache/incubator-superset/pull/10170/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6X3NpcDM4LnB5) | `0.00% <0.00%> (ø)` | |
   | [superset/views/database/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10170/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvYXBpLnB5) | `87.50% <0.00%> (ø)` | |
   | [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/10170/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (ø)` | |
   | [superset-frontend/src/components/Pagination.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10170/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvUGFnaW5hdGlvbi50c3g=) | | |
   | [superset-frontend/src/CRUD/utils.js](https://codecov.io/gh/apache/incubator-superset/pull/10170/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvdXRpbHMuanM=) | | |
   | [...shboard/util/charts/getFormDataWithExtraFilters.ts](https://codecov.io/gh/apache/incubator-superset/pull/10170/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2NoYXJ0cy9nZXRGb3JtRGF0YVdpdGhFeHRyYUZpbHRlcnMudHM=) | | |
   | [superset-frontend/src/components/RefreshLabel.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10170/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvUmVmcmVzaExhYmVsLmpzeA==) | | |
   | [...s/ErrorMessage/getErrorMessageComponentRegistry.ts](https://codecov.io/gh/apache/incubator-superset/pull/10170/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRXJyb3JNZXNzYWdlL2dldEVycm9yTWVzc2FnZUNvbXBvbmVudFJlZ2lzdHJ5LnRz) | | |
   | ... and [400 more](https://codecov.io/gh/apache/incubator-superset/pull/10170/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10170?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/10170?src=pr&el=footer). Last update [d8314ee...3ce4fb0](https://codecov.io/gh/apache/incubator-superset/pull/10170?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] ktmud merged pull request #10170: build: enable typescript for cypress

Posted by GitBox <gi...@apache.org>.
ktmud merged pull request #10170:
URL: https://github.com/apache/incubator-superset/pull/10170


   


----------------------------------------------------------------
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] etr2460 commented on pull request #10170: build: enable typescript for cypress

Posted by GitBox <gi...@apache.org>.
etr2460 commented on pull request #10170:
URL: https://github.com/apache/incubator-superset/pull/10170#issuecomment-650265688


   Looks like CI is still breaking a bit, will review once fixed (i think you're missing installing an import)


----------------------------------------------------------------
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] ktmud commented on a change in pull request #10170: build: enable typescript for cypress

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



##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/filter.test.ts
##########
@@ -72,7 +85,7 @@ describe('Dashboard filter', () => {
 
     cy.get('.Select__control input[type=text]')
       .first()
-      .focus({ force: true })

Review comment:
       Type check helped by notifying me this is not a valid API for `focus`.




----------------------------------------------------------------
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] ktmud edited a comment on pull request #10170: build: enable typescript for cypress

Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #10170:
URL: https://github.com/apache/incubator-superset/pull/10170#issuecomment-650340633


   @etr2460 Fixed the tests and this is now ready for re-review.


----------------------------------------------------------------
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] ktmud commented on a change in pull request #10170: build: enable typescript for cypress

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



##########
File path: superset-frontend/cypress-base/cypress/integration/dashboard/filter.test.ts
##########
@@ -32,13 +44,14 @@ describe('Dashboard filter', () => {
 
     cy.visit(WORLD_HEALTH_DASHBOARD);
 
-    cy.get('#app').then(data => {
-      const bootstrapData = JSON.parse(data[0].dataset.bootstrap);
-      const dashboard = bootstrapData.dashboard_data;
+    cy.get('#app').then(app => {
+      const bootstrapData = app.data('bootstrap');
+      const dashboard = bootstrapData.dashboard_data as DashboardData;
       const sliceIds = dashboard.slices.map(slice => slice.slice_id);
-      filterId = dashboard.slices.find(
-        slice => slice.form_data.viz_type === 'filter_box',
-      ).slice_id;
+      filterId =
+        dashboard.slices.find(
+          slice => slice.form_data.viz_type === 'filter_box',
+        )?.slice_id || 0;

Review comment:
       Fallback to 0 is easier than running assertions on a potential `undefined`.




----------------------------------------------------------------
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] ktmud commented on a change in pull request #10170: build: enable typescript for cypress

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



##########
File path: superset-frontend/cypress-base/.eslintrc
##########
@@ -0,0 +1,25 @@
+{
+  "parser": "@typescript-eslint/parser",
+  "plugins": ["cypress", "@typescript-eslint"],
+  "extends": [
+    "plugin:@typescript-eslint/recommended",
+    "plugin:cypress/recommended"
+  ],
+  "rules": {
+    "import/no-unresolved": 0,

Review comment:
       Some Cypress modules might not be available if you run `eslint` in the parent folder before `npm install` in `cypress-base` (which is what our CI does).




----------------------------------------------------------------
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] ktmud commented on pull request #10170: build: enable typescript for cypress

Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #10170:
URL: https://github.com/apache/incubator-superset/pull/10170#issuecomment-650340633


   @etr2460 Fixed the tests and this is now ready for review.


----------------------------------------------------------------
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