-
Notifications
You must be signed in to change notification settings - Fork 13
Add @typescript-eslint/parser and typescript-eslint to user tests #119
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
Add @typescript-eslint/parser and typescript-eslint to user tests #119
Conversation
@@ -0,0 +1,4 @@ | |||
{ | |||
"cloneUrl": "https://github.com/typescript-eslint/typescript-eslint.git", |
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.
Theoretically, I would think that this is enough to capture everything, right?
(Also, could you to ts-eslint add something that can install the latest TS and check it so you get early warnings too?)
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.
Theoretically, I would think that this is enough to capture everything, right?
Our git clone
-> yarn install
& yarn build
isn't what's failing in CI though. It's the yarn test
. Does the automation also run tests? (sorry if I missed this, it would explain what you've been trying to convey to me)
(Also, could you to ts-eslint add something that can install the latest TS and check it so you get early warnings too?)
Absolutely: typescript-eslint/typescript-eslint#7093
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.
The automation only installs without scripts and runs tsc
on each tsconfig.json
. It never executes code from within the repo, unless you write a script-based test which would do that. If yarn test
takes a large amount of time, I don't think we would add it here, but I would me more interested in why things fail only in test
and not otherwise... You're still referring to an OOM, right? Not trying to catch errors in the typescript API?
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.
still referring to an OOM, right?
Exactly. I don't know why it's OOMing yet, but at the very least I'm hoping that running a similar bit of code to the offending tests would catch similar issues.
Going to merge this to test it; if it breaks I'll send a revert. |
Unfortunately it looks like this infra isn't able to reproduce the problem, but given it finds type errors it's not super clear to me whether or not it's actually successfully building it or not. |
|
Adds a usage of
@typescript-eslint/parser
'sparseAndGenerateServices
API with relatively more intensive options (especiallymoduleResolver
). It's based on https://github.com/typescript-eslint/typescript-eslint/blob/a2b6b2e0a1ccdfbd5c76ba3fbcffd94d29f5d2b1/packages/typescript-estree/tests/lib/parse.moduleResolver.placeholder-success.test.ts. Runningyarn test
-in which tests call to TypeScript APIs- is what's running out of memory.typescript-eslint/typescript-eslint#7088Also adds
typescript-eslint
as a more typical repo to clone & type check.cc @jakebailey - we'd chatted about this a bit.