test: [M3-7139] - Enable TypeScript typechecking in Cypress directory #10086

jdamore-linode · 2024-01-19T21:03:55Z

Description 📝

This updates the Cypress tsconfig.json file to allow typechecking against files in the Cypress directory, and fixes some TypeScript errors.

Changes 🔄

List any change relevant to the reviewer.

Extend Cypress's tsconfig from Manager's tsconfig, remove a lot of unneeded config
Fix various TypeScript issues
- Removed lots of unused imports/consts/etc.
- Added missing function param annotations
- Fixed a couple issues where incorrect types or properties were specified
Update API-v4 CreateLinodeRequest interfaces property to accept InterfacePayload objects instead of Interface objects

How to test 🧪

We can lean on the automated tests to make sure the e2e tests themselves aren't broken. I did make a really minor change to vite.config.ts, and any issues there should also manifest during the tests, but it might be good to build Cloud and give it a quick look just to make sure there are no unexpected issues.

To run typechecking against the Cypress code, you can run this command from within packages/manager:

yarn tsc -p cypress --noEmit

As an Author I have considered 🤔

Check all that apply

abailly-akamai · 2024-01-22T15:43:31Z

Thanks @jdamore-linode! This is exciting. Do you still need help to fix types in here? Happy to jump in in my leisurely time to help with the PR

… unrestricted users

…` and `active` params are not required

jdamore-linode · 2024-01-22T19:23:34Z

Thanks @jdamore-linode! This is exciting. Do you still need help to fix types in here? Happy to jump in in my leisurely time to help with the PR

Thanks @abailly-akamai!

Just pushed changes that get the typecheck passing, although some of the fixes are pretty ugly and I'd love a second set of eyes on those in case there are opportunities to make things better/safer/etc. (And if you do see anything to improve, you're absolutely welcome to push improvements here)

Some examples of questionable fixes:

Casting booleans as any
Casting regular strings as string literal types for mocks
Lots of any param annotations
Lots of ! non-null assertions for mock data that's technically optional but known to exist

(Also CCing @bnussman-akamai since he normally advocates for increased type safety and I'd like to hear his opinion)

jdamore-linode · 2024-01-22T19:38:53Z

packages/api-v4/src/linodes/types.ts

@@ -354,7 +354,7 @@ export interface CreateLinodeRequest {
  tags?: string[];
  private_ip?: boolean;
  authorized_users?: string[];
-  interfaces?: Interface[];
+  interfaces?: InterfacePayload[];


I want to draw attention to this change in case it's incorrect. Without this change, I encountered type errors in the config test because TypeScript expected id and active properties to be specified when creating a Linode with Interfaces via the API SDK's createLinode function.

Linode API docs show active in the request payload (after VPC beta), but I have a feeling that it's supposed to be a read-only property? Anyways I think InterfacePayload is correct and id definitely shouldn't be in there.

github-actions · 2024-01-22T19:52:02Z

Coverage Report: ✅
Base Coverage: 80.33%
Current Coverage: 80.33%

hkhalil-akamai · 2024-01-22T23:27:13Z

Wonder if we should add tsc -p cypress --noEmit to packages/manager/package.json:86, which currently looks like:

    "start": "concurrently --raw \"vite\" \"tsc --watch --preserveWatchOutput\"",

This might bring up any new type errors as team members are working on the cloud instead of having to run typechecks manually or relying on test failures.

hkhalil-akamai

Nice work on getting types working here, I'm confident this can help prevent some testing bugs in the future.

bnussman-akamai · 2024-01-23T01:53:01Z

This is great! 🎉 You may have been use to it @jdamore-linode, but for me it was very frustrating working on the e2es without proper language server help! This is gonna be super nice and help me move faster 🚀

Before	After
Screen.Recording.2024-01-22.at.8.43.59.PM.mov	Screen.Recording.2024-01-22.at.8.44.50.PM.mov

I'm not too stressed about any of the "questionable fixes". I think we can iron those out over time.

Regarding what @hkhalil-akamai said about adding tsc to the start command, I'm personally fine without it, but I think I'd be okay with whatever. I think most of the value here for me is the proper language server auto-completion and whatnot as shown in the videos.

I'll continue reviewing and thinking over the changes!

abailly-akamai

Thanks @jdamore-linode - this is a great improvement.

Also agree we don't need the --watch as much here, but we can always change our mind

I pushed a few type improvements around any and type casting which I hope won't break anything!

abailly-akamai · 2024-01-23T15:53:16Z

packages/manager/cypress/e2e/core/volumes/attach-volume.spec.ts

+//   });
+//   const volume = await createVolume(volumeRequest);
+//   return [linode, volume];
+// };


can we add a comment as to commenting this out?

jdamore-linode · 2024-01-23T17:16:38Z

This is great! 🎉 You may have been use to it @jdamore-linode, but for me it was very frustrating working on the e2es without proper language server help!

@bnussman-akamai Interestingly, using Sublime Text with the lsp-typescript plugin, I always had (some) editor annotations which led me to assume that our setup was working as expected -- I'd get imports added automatically when I'd refer to api-v4 types, could see inline type docs when applicable, and so on. I was really surprised by some of the type mistakes that were revealed by this change!

bnussman-akamai · 2024-01-23T17:44:01Z

@jdamore-linode Ahh, very interesting. In VSCode it seemed that things were partially working in general.

bnussman-akamai

Awesome stuff 🚀

cpathipa

Thank you @jdamore-linode Nice work!

Joe D'Amore added 13 commits January 19, 2024 15:01

Use ESM-friendly method to get directory name in Vite configs

3eb113f

Extend Cypress tsconfig from Cloud Manager tsconfig

27c1162

Fix Cypress tsc issues caused by api-v4/types imports

51c6671

Fix tsc issue related to Service Transfer status types

b2c2f6e

Fix billing tests mismatched types and missing param types

344ffaa

Fix mismatched types in Cypress plugins dir

ad8e688

Fix missing param annotations

67d3d9f

Fix DBaaS test type issues

bf74e67

Fix Domains test type issues

42cc2d9

Fix domain ID mismatched type

7fcb369

Fix type errors in defer command

79df5d5

Fix missing import, remove commented line

9da5039

Fix Firewall update type errors

d70bee4

jdamore-linode added the Testing label Jan 19, 2024

jdamore-linode requested a review from abailly-akamai January 19, 2024 21:03

jdamore-linode self-assigned this Jan 19, 2024

jdamore-linode added the Work in Progress label Jan 19, 2024

Joe D'Amore added 5 commits January 19, 2024 16:39

Fix type errors

5a5714a

Fix mockGetClusters param annotation

2f15e9e

Various fixes

def56a7

Add types for chai-string

d217f0c

WIP

95f7c31

Joe D'Amore added 6 commits January 22, 2024 13:47

Fix TypeScript error stemming from misunderstanding of grants API for…

04579ea

… unrestricted users

Fix OBJ enable test TS errors by casting mock cluster IDs

fb20b23

Replace Linode type with CreateLinodeRequest to fix TS error

4f1a3af

Use InterfacePayload type in Linode create request type so that `id…

6d253f3

…` and `active` params are not required

Update yarn.lock

4b70f02

Typecheck Cypress during Git precommit hook

5701dc4

jdamore-linode added Ready for Review and removed Work in Progress labels Jan 22, 2024

jdamore-linode marked this pull request as ready for review January 22, 2024 19:24

jdamore-linode requested review from a team as code owners January 22, 2024 19:24

jdamore-linode requested review from cliu-akamai, hana-akamai and cpathipa and removed request for a team January 22, 2024 19:24

Joe D'Amore added 2 commits January 22, 2024 14:24

Merge branch 'develop' into exp-cypress-typecheck

c669e8b

Add changesets

265f4da

jdamore-linode commented Jan 22, 2024

View reviewed changes

Fix domain record test by using domain creation payload factory

8761081

hkhalil-akamai approved these changes Jan 22, 2024

View reviewed changes

Improve leftover types

a91272e

abailly-akamai approved these changes Jan 23, 2024

View reviewed changes

bnussman-akamai approved these changes Jan 23, 2024

View reviewed changes

Add TODO note for commented function

89fc6a7

cpathipa approved these changes Jan 24, 2024

View reviewed changes

Merge branch 'develop' into exp-cypress-typecheck

b4560de

jdamore-linode merged commit a8e51e7 into linode:develop Jan 26, 2024
18 checks passed

cpathipa mentioned this pull request Feb 1, 2024

Release v1.111.0 - release → staging #10136

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: [M3-7139] - Enable TypeScript typechecking in Cypress directory #10086

test: [M3-7139] - Enable TypeScript typechecking in Cypress directory #10086

jdamore-linode commented Jan 19, 2024 •

edited

Loading

abailly-akamai commented Jan 22, 2024

jdamore-linode commented Jan 22, 2024 •

edited

Loading

jdamore-linode Jan 22, 2024

hkhalil-akamai Jan 22, 2024 •

edited

Loading

github-actions bot commented Jan 22, 2024 •

edited

Loading

hkhalil-akamai commented Jan 22, 2024 •

edited

Loading

hkhalil-akamai left a comment

bnussman-akamai commented Jan 23, 2024

abailly-akamai left a comment

abailly-akamai Jan 23, 2024

jdamore-linode commented Jan 23, 2024 •

edited

Loading

bnussman-akamai commented Jan 23, 2024

bnussman-akamai left a comment

cpathipa left a comment

test: [M3-7139] - Enable TypeScript typechecking in Cypress directory #10086

test: [M3-7139] - Enable TypeScript typechecking in Cypress directory #10086

Conversation

jdamore-linode commented Jan 19, 2024 • edited Loading

Description 📝

Changes 🔄

How to test 🧪

As an Author I have considered 🤔

abailly-akamai commented Jan 22, 2024

jdamore-linode commented Jan 22, 2024 • edited Loading

jdamore-linode Jan 22, 2024

Choose a reason for hiding this comment

hkhalil-akamai Jan 22, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jan 22, 2024 • edited Loading

hkhalil-akamai commented Jan 22, 2024 • edited Loading

hkhalil-akamai left a comment

Choose a reason for hiding this comment

bnussman-akamai commented Jan 23, 2024

abailly-akamai left a comment

Choose a reason for hiding this comment

abailly-akamai Jan 23, 2024

Choose a reason for hiding this comment

jdamore-linode commented Jan 23, 2024 • edited Loading

bnussman-akamai commented Jan 23, 2024

bnussman-akamai left a comment

Choose a reason for hiding this comment

cpathipa left a comment

Choose a reason for hiding this comment

jdamore-linode commented Jan 19, 2024 •

edited

Loading

jdamore-linode commented Jan 22, 2024 •

edited

Loading

hkhalil-akamai Jan 22, 2024 •

edited

Loading

github-actions bot commented Jan 22, 2024 •

edited

Loading

hkhalil-akamai commented Jan 22, 2024 •

edited

Loading

jdamore-linode commented Jan 23, 2024 •

edited

Loading