kulshekhar / ts-jest

A Jest transformer with source map support that lets you use Jest to test projects written in TypeScript.
https://kulshekhar.github.io/ts-jest
MIT License
6.97k stars 453 forks source link

Type error reporting #79

Closed pelotom closed 7 years ago

pelotom commented 7 years ago

Currently it seems that the preprocessor doesn't report type errors. Would it be possible to treat type errors in TS files (whether source or test files) as if they were test failures?

kulshekhar commented 7 years ago

@pelotom can you create a tiny repo that illustrates the issue and your expectations. It would help in quickly getting started on this

gyzerok commented 7 years ago

@kulshekhar there is an example in linked issue. It do not use ts-jest however both this example and ts-jest have same problem. There are expectations defined as well.

kulshekhar commented 7 years ago

@pelotom @gyzerok

It doesn't seem that this can be fixed in ts-jest. When running tsc against example.test.ts from the repo, the corresponding example.test.js file is generated. This is what ts-jest receives and uses. The transpiler returns transpiled code even with noEmitOnError set to true.

If there were a way to get tsc the transpiler to abort and throw an exception on type errors, that would help. Without such an option, the only way to do this would be to launch an external process to transpile the source code and check for the exit code of the process. This would slow down tests and I'm not sure it's worth the returns given that any build step (not to forget the IDEs) will display these errors anyway.

I'm closing this for now but if you have alternative solutions or want to discuss this further, feel free to do so here. If tsc were to get such a feature, it might be worth reopening this issue.

Igmat commented 7 years ago

Really, the main problem is that TS needs project compilation and not file compilation, in order to correctly show type errors. We could try to find a workaround that compiles overall project instead of one file for Jest, but it definitely wouldn't be a preprocessor, because Jest API sends only one file to it.

gyzerok commented 7 years ago

If we can somehow serialize compilation and just restore in subsequent calls to preprocessor. Can we cache compilation results within one run of Jest somehow?

gyzerok commented 7 years ago

I have no time now to dig deeper but this looks like what we need.

Igmat commented 7 years ago

@gyzerok, ok it's a good point. I'll try to dig into it later, but at first look it seems that we will have to inform Jest that it should wait for project compilation and then inform it each time when file changes instead of default Jest watcher. So it could be very difficult to achieve.

pelotom commented 7 years ago

It might be instructive to look at how the ts-loader for webpack works, since it's solving a similar problem... it is using the TS language services to incrementally recompile files and display type errors, and it's adapted to fit the webpack (and also jest) model of servicing compilation requests for individual files.

gyzerok commented 7 years ago

Should we reopen an issue since we might have possible solution?

kulshekhar commented 7 years ago

I haven't dug deep into this but this doesn't look like a workable solution at first sight.

The TS docs linked to and ts-loader both use file watchers to do incremental transpilation.

Since a preprocessor doesn't run in it's own process and is only invoked by jest on a file by file basis, the expectation is that the preprocessor would process the file given to it and return the result. I would personally like to limit the scope of ts-jest to that.

That said, if there's a PR that extends the capability of ts-jest without adding complexity to the core codebase and functionality (something like a plugin/extension), I'm open to it. (Although it would be somewhat amusing to see a plugin, ts-jest, have its own set of plugins)

The only reasonable solution I can see is if tsc starts throwing an exception (or uses some other mechanism to indicate a failure) when there's a type error.

All this aside, I'm also curious about why failing on a type error would be needed in a preprocessor. Shouldn't this be part of the build process where the build fails if there's a type error?

Igmat commented 7 years ago

Ok, after small investigation I have some thoughts on how it could be achieved. It definitely wouldn't be a preprocessor itself, but we already have coverage processor in our package, so IMO it's not a problem to provide such functionality as sub-module of ts-jest. Actually I can't say that it will be implemented (there are a lot of possible caveats), but I'll try.

kulshekhar commented 7 years ago

@Igmat As I mentioned in the earlier comment, I can't see a valid reason for type checking to be the responsibility of a preprocessor. However, if you want to add it, go ahead. Just ensure that this doesn't affect the complexity and performance of the core functionality/code.

Bear in mind that it's hard to remove a feature (even a poorly performing one) once it has been added and other people depend on it.

mikehaas763 commented 6 years ago

All this aside, I'm also curious about why failing on a type error would be needed in a preprocessor. Shouldn't this be part of the build process where the build fails if there's a type error?

test code is still code, if there's type errors in such test code, shouldn't it fail?

hjfreyer commented 6 years ago

As mentioned in #250, it's unclear where else type error reporting could go when using jest --watch. In that case, the build process happens within jest, so somebody has to own it. Relying on IDE support is a real drag since everyone's got their own workflow, and type-aware editors aren't always an option.

kulshekhar commented 6 years ago

@hjfreyer does this help?. It was recently added

hjfreyer commented 6 years ago

Woot! Indeed it does. Thanks!

lyy011lyy commented 6 years ago

@kulshekhar Thanks for the suggestion, but when I enable it, the test is really slow..... say 30sec for a single test... Is there any other way to solve this? Thanks!

kulshekhar commented 6 years ago

@lyy011lyy yes, by keeping type checks out of testing (that's what I do)

Dean177 commented 6 years ago

@lyy011lyy I use tsc --no-emit as one of my CI steps usually just before running any unit tests. That way tests stay fast and you still get typechecking as part of your workflow.

lyy011lyy commented 6 years ago

Hi @Dean177 : Thanks for the suggestion and it works well locally. But I want to add the typer errors info into the jUnit test result report which is used to fail the voter. With this method, if there are type errors, there is no test result report will be generated.

I retried the "enableTsDiagnostics": true and found that seems like this will trigger the type checking for the whole imported dependency tree, which makes it slow. I was wondering that is there a way to make it only check the current test file?

Thanks

lyy011lyy commented 6 years ago

Found the issue at https://github.com/kulshekhar/ts-jest/blob/125d06b9da1bfc635e45f862e8ff35a2cb00b676/src/preprocess.ts#L45 I found that ts-jest is now checking the type errors for source codes and all their dependencies if "enableTsDiagnostics" is turned ON. This makes it very slow and I don't think that's what we want since the src code type errors will be detected during the build time. What we need should be a way to report the type errors inside the test files, because usually test code will be excluded when build.

It should be easy to check whether the filePath is inside one of the root's pathes to achieve that only check the type errors inside test code. I'm creating the pull request.

PiotrOwsiak commented 1 year ago

Hold on, this is actually one of the main features that ts-jest is advertising here https://kulshekhar.github.io/ts-jest/docs/ "It supports all features of TypeScript including type-checking. Read more about Babel7 + preset-typescript vs TypeScript (and ts-jest)."