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.95k stars 452 forks source link

Code Coverage: Uncovered lines/statements in file that only defines interfaces. #378

Closed UselessPickles closed 6 years ago

UselessPickles commented 6 years ago

In a .ts file that contains only interface definitions/exports, code coverage reports uncovered lines and statements, even if the interface(s) are imported by unit tests. The mapping of uncovered code also appears to be nonsense.

Example file content:

/**
 * An interface.
 */
export interface MyInterface {
    name: string;
}

Resulting code coverage report: screen shot 2017-11-27 at 11 42 44 am

Resulting code coverage JSON for this file:

{
    "path": "***redacted***/coverage-test.ts",
    "statementMap": {
        "0": {
            "start": {
                "line": 1,
                "column": 13
            },
            "end": {
                "line": 1,
                "column": 42
            }
        },
        "1": {
            "start": {
                "line": 1,
                "column": 42
            },
            "end": {
                "line": 1,
                "column": 55
            }
        },
        "2": {
            "start": {
                "line": 2,
                "column": 0
            },
            "end": {
                "line": 2,
                "column": 62
            }
        }
    },
    "fnMap": {},
    "branchMap": {},
    "s": {
        "0": 0,
        "1": 0,
        "2": 0
    },
    "f": {},
    "b": {}
}

The file has no executable lines/statements, so there should be nothing reported as "not covered". It should report 0/0 (100%) coverage for both lines and statements.

Should be simple enough to reproduce in ANY existing repo.

UselessPickles commented 6 years ago

My prediction is that it has something to do with the resulting JS module that is emitted by the typescript compiler. It is an empty module that does nothing. My guess is that the typescript compiler is then smart enough to NOT emit imports of empty modules, so even though I have unit tests that import the interface, the emitted empty JS module never gets "executed" during the unit tests. The uncovered statements are probably boilerplate module code.

UselessPickles commented 6 years ago

Typescript: 2.6.1 Jest: 21.2.1 ts-jest: 21.2.2 webpack: 3.8.1

Using typescript option for "commonjs" modules.

Let me know if this issue is not easily reproduced in any random existing repo, and I will put effort into creating a minimal repo.

GeeWee commented 6 years ago

Yes it seems to also be mentioned in #377 Not sure what the "right" fix for this is. I don't think we can just not emit anything. Thoughts @kulshekhar ?

UselessPickles commented 6 years ago

377 is specific to .d.ts files, while my example is a .ts file. If I have only types and interfaces in a file, is it more appropriate/correct for me to name the file as a .d.ts file? If that is "the right thing" to do, then my problem is easily solved by renaming such files to .d.ts, and excluding .d.ts files from code coverage. But if .d.ts are supposed to be used for a more specific purpose, then renaming my files would be a hack.

kulshekhar commented 6 years ago

Not sure what the "right" fix for this is. I don't think we can just not emit anything.

It's hard to guess without a minimal repo. @UselessPickles could you please create one?

I won't have time during the week but I plan to set aside a few hours on the weekend to look into the issues here

UselessPickles commented 6 years ago

I should have some time to create a minimal repo tomorrow evening.

GeeWee commented 6 years ago

I think .d.ts files work a little different than regular .ts files. However I don't think a jest preprocessor has the abillity to "not" parse a file. E.g. we can't exclude .ts files with only interfaces, so I'm a little stumped as to what the potential solution might be.

kulshekhar commented 6 years ago

A quick (and imo the correct) solution might be to update the regex so that jest doesn't pick up those files in the first place

edit: the comment was with respect to the .d.ts files

JoshuaKGoldberg commented 6 years ago

@UselessPickles bump?

UselessPickles commented 6 years ago

I haven't forgotten, but this is one of the lowest priority issues I'm dealing with, and creating a minimal repo project is not exactly low effort.

JoshuaKGoldberg commented 6 years ago

Fair. Here's a min repro: https://github.com/JoshuaKGoldberg/ts-jest-wallaby-vscode/tree/ts-jest-coverage-repro

From package.json:

"jest": {
    "collectCoverageFrom": [
      "src/*.ts",
      "!src/excluded.ts"
    ],
    "transform": {
      ".(ts|tsx)": "<rootDir>/node_modules/ts-jest/preprocessor.js"
    },
    "testRegex": "(/__tests__/.*|\\.(test|spec))\\.(ts|tsx|js)$",
    "moduleFileExtensions": [
      "ts",
      "tsx",
      "js"
    ]
  }

If we remove the collectCoverageFrom field, the coverage correctly reports as just being over sum.ts. However, it's common to need to exclude files (such as external API wrappers that can't be tested). Changing it to just the ! excludes means no files are collected from.

----------|----------|----------|----------|----------|----------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
----------|----------|----------|----------|----------|----------------|
All files |       50 |      100 |      100 |       60 |                |
 sum.ts   |      100 |      100 |      100 |      100 |                |
 types.ts |        0 |      100 |      100 |        0 |            1,2 |
----------|----------|----------|----------|----------|----------------|
// types.ts
export type MyNumber = number;
// types.js (what it would look like)
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
//# sourceMappingURL=types.js.map
dmitriid commented 6 years ago

It's worse if decorators are involved :)

A bigger example is here: https://github.com/dmitriid/kalk

Run yarn && yarn coverage

In screenshot below: Yellow files are touched by tests, green files are not, yet reported as 100% coverage (because there's a clock running, and it gets called while tests are running)

screen shot 2017-12-21 at 14 21 22

However, if you look at coverage reports, you'll notice something strange :)

screen shot 2017-12-21 at 14 21 48

With decorators it's definitely core jests' fault as well

kulshekhar commented 6 years ago

@JoshuaKGoldberg I'm trying out the linked repo. Can you tell me what you're seeing and what you expect to see?

JoshuaKGoldberg commented 6 years ago

@kulshekhar types.ts shouldn't count negatively toward the code coverage computation. Either it should count as 0/0=100% coverage (math!) or it shouldn't be a part of the calculations. I prefer the first. That repo should have 100% code coverage; types.ts isn't missing any lines that should be covered.

kulshekhar commented 6 years ago

When I execute yarn test --coverage, I'm getting this:

$ jest --no-cache --coverage
 PASS  tests/sum.test.ts
  ✓ adds 1 + 2 to equal 3 (3ms)

----------|----------|----------|----------|----------|----------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
----------|----------|----------|----------|----------|----------------|
All files |      100 |      100 |      100 |      100 |                |
 sum.ts   |      100 |      100 |      100 |      100 |                |
----------|----------|----------|----------|----------|----------------|
Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        2.104s
Ran all test suites.
JoshuaKGoldberg commented 6 years ago

I don't get that :( are you on the ts-jest-coverage-repro branch?

kulshekhar commented 6 years ago

@JoshuaKGoldberg sorry about that. I think I didn't change the branch. I'll take a look again after work

kulshekhar commented 6 years ago

@JoshuaKGoldberg I was able to reproduce the issue using the correct branch. Taking a closer look at this now

kulshekhar commented 6 years ago

I don't think there's anything that can be done in ts-jest to fix this. I've taken a closer look at this issue and there are two ways to get the desired outcome:

When TypeScript transpiles files, it doesn't convert 'pure type' imports to require statements. This results in Jest not picking and passing those files to ts-jest.

GeeWee commented 6 years ago

I don't think we can exclude files that contain only types. I also think this is a wontfix unless jest adds the capabillity for transformers to opt files out of coverage or something akin to that.

kulshekhar commented 6 years ago

@JoshuaKGoldberg wouldn't http://facebook.github.io/jest/docs/en/configuration.html#coveragepathignorepatterns-array-string suffice?

JoshuaKGoldberg commented 6 years ago

coveragePathIgnorePatterns works but isn't enough.

Normally, we shouldn't have to modify config files to write code outside of project setup. Someone adding code to a project could be very confused why their new interfaces-only file (or existing file that becomes interfaces-only). Also:

GeeWee commented 6 years ago

Let's close this discussion down and see what happens in #5209

vyshkant commented 6 years ago

Let's close this discussion down and see what happens in #5209

@GeeWee Looks like the issue #5209 does not exist on this repo. Could you please explain what issue do you mean?

GeeWee commented 6 years ago

Sorry, meant the jest issue : https://github.com/facebook/jest/issues/5209

Elendev commented 6 years ago

I encounter exactly the same issue as @dmitriid : image

Note that in the same project, with the same configuration, some files handled differently by Jest : image

@dmitriid were you able to find a solution ?

Rush commented 3 years ago

Why is this issue closed if people are still having issues?

ahnpnl commented 3 years ago

Because code coverage functionality belongs to Jest, ts-jest only transforms codes. Coverage is decided by Jest with the help of istanbul.

marceloavf commented 2 years ago

For me, it works when I set this option true in tsconfig.json

...
"sourceMap": true,
...
uvinod commented 1 year ago

For me, it works when I set this option true in tsconfig.json

...
"sourceMap": true,
...

Does it work? It is not working for me.