Skip to content
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 regex to catch errors without file/line information #3

Merged

Conversation

glunn
Copy link

@glunn glunn commented Feb 24, 2023

We discovered that in some instances, Typescript will return an error that isn't associated with a specific file, line, or col.

For example:
error TS2688: Cannot find type definition file for 'ember__test-helpers'.

In these cases, typescript-xunit-xml was returning a result which looked identical to a passing tsc compile (0 errors), and our builds were passing when they shouldn't.

At least for our use case, it would be really helpful to report out those errors as well. Is that something the broader community might want? Here's one proposal to fix that, which introduces a separate regex to catch the errors.

@glenjamin
Copy link
Owner

Thanks for adding this - could you expand the test script and sample to include this case too please?

Would you be able to provide any more details about what scenario triggers this sort of error?

@glenjamin glenjamin mentioned this pull request Feb 25, 2023
@glunn
Copy link
Author

glunn commented Mar 10, 2023

Absolutely, just pushed up a commit to add those.

What I understand to have happened is that a dependency of ours (ember-test-helpers) made some change that resulted in the removal of their index.d.ts file.

Then, the existence of a subdirectory in a typeRoots directory without an index.d.ts file in our node modules generated the TS2688 error. Similar to the issue documented here: microsoft/TypeScript#27956 (comment)

To make things worse, this sort of issue appears to break typescript's ability to compile and it won't report out any subsequent errors. So once we merged in that (falsely passing) dependency upgrade TS2688 was the only error ever passed to typescript-xunit-xml. Which meant that because that error was swallowed, our build pipeline ended up blind to any new TS violations we added to our own code.

Thanks for being so responsive and open to fixing this, happy to add more to the pr or answer questions if I can : )

somewhatabstract added a commit to Khan/typescript-xunit-xml that referenced this pull request Apr 6, 2023
…pected.txt (#1)

🖍 _This is an audit!_ 🖍

## Summary:
Add regex to catch errors without file/line information

Issue: XXX-XXXX

This merges the fix from glenjamin#3 into our fork.

## Test plan:
`npm test`

Author: somewhatabstract

Auditors: jeresig

Required Reviewers:

Approved By: jeresig

Checks:

Pull Request URL: #1
@glenjamin glenjamin merged commit 661745e into glenjamin:main Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants