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
Reduce false negative cases of typescript parser tests #10979
Reduce false negative cases of typescript parser tests #10979
Conversation
@@ -1,7 +1,8 @@ | |||
const path = require("path"); | |||
const fs = require("fs").promises; | |||
const ts = require("typescript"); | |||
const ts = require("../../../build/typescript"); |
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.
Use the bundled typescript bin since the dev dependency could be out-of-dated.
|
||
module.exports = [ | ||
// "TS1005", // '{0}' expected. | ||
"TS1009", // Trailing comma not allowed. |
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.
TypeScript does not push parseDiagnostics
for all and not limited to these errors. 🤷
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.
Thanks!
For details please refer to the comment section of new files.
I have commented out the error codes which would introduce new false positive cases, so that this PR is only removing the false negative cases.
Before
After
When reading the TypeScript tests, I realize that some false negative cases comes from that we are not running test properly. i.e.
declarationMapsOutFile.ts
uses@filename
annotation to specify multiple typescript files inside a single testcase. But we are parsing the whole testcase a single file, this would introduce FN especially if the case is testing import/exports. That's why we have many import/export related case in the whitelists. I would address this issue in a separate PR.