build: dont prefer ts for cypress tests #10161
Conversation
There was a problem hiding this comment.
I feel like we should still prefer TS for all files (tests or not) but not force tests to be TS right away. What do you think? Note that this workflow doesn't block merging, this is an optional check
On a side note, I think the action that makes comments is broken by github changing their API. I need to figure out a fix...
| } | ||
| echo ::set-output name=js_files_added::$(js_files_added) | ||
|
|
||
| - if: steps.check.outputs.js_files_added |
There was a problem hiding this comment.
i think this might need to still have the string comparison? how did you test this?
There was a problem hiding this comment.
oh, i see your link to your fork. nevermind
There was a problem hiding this comment.
Yeah, fortunately empty string is also considered falsy in GitHub actions.
I think it's OK to not require TypeScript for Cypress tests, as they seldom need typing. It does block merging, see: #10158 (because of the broken comment action, I guess). |
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`
mistercrunch
added
🏷️ bot
SUMMARY
Cypress tests don't need to be TypeScript
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TEST PLAN
Check CI.
Tested in ktmud#153