-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
build: enable typescript for cypress #10170
Conversation
@@ -72,7 +85,7 @@ describe('Dashboard filter', () => { | |||
|
|||
cy.get('.Select__control input[type=text]') | |||
.first() | |||
.focus({ force: true }) |
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.
Type check helped by notifying me this is not a valid API for focus
.
filterId = | ||
dashboard.slices.find( | ||
slice => slice.form_data.viz_type === 'filter_box', | ||
)?.slice_id || 0; |
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.
Fallback to 0 is easier than running assertions on a potential undefined
.
227baec
to
35d524d
Compare
Looks like CI is still breaking a bit, will review once fixed (i think you're missing installing an import) |
a7f0708
to
3ce4fb0
Compare
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
"plugin:cypress/recommended" | ||
], | ||
"rules": { | ||
"import/no-unresolved": 0, |
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.
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).
@etr2460 Fixed the tests and this is now ready for re-review. |
Follow up apache#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`
Some modules might not be available when `eslint` in the parent folder before running `npm install` in the cypress folder.
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, thanks for taking the time to do this!
Prefer TypeScript has stopped working since apache#10170 because of a [typo][1] in `jq` script. [1] https://github.com/apache/incubator-superset/pull/10170/files#diff-0f1a9bbd1316e385c51f8aba1583bd99R24
Prefer TypeScript has stopped working since #10170 because of a [typo](https://github.com/apache/incubator-superset/pull/10170/files#diff-0f1a9bbd1316e385c51f8aba1583bd99R24) in `jq` script. Also fix the `trilom/file-changes-action` GH Action version so the process can be more predictable.
Prefer TypeScript has stopped working since apache#10170 because of a [typo](https://github.com/apache/incubator-superset/pull/10170/files#diff-0f1a9bbd1316e385c51f8aba1583bd99R24) in `jq` script. Also fix the `trilom/file-changes-action` GH Action version so the process can be more predictable.
SUMMARY
Follow up #10161 :
cypress-base
Currentlycypress-base
is now using a newer version of TypeScript thansuperset-frontend
. Will file another PR to upgrade later.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TEST PLAN
All Cypress tests should still be valid and pass.
cc @etr2460