Allow attaching a comment before all non-user injected code.

[Izhaki]

Some emitted code results in reduced coverage reports (see #13029, or example below).

To combat this, it would be nice to have a feature similar to the auxiliaryCommentBefore in Babel, which adds a comment of choice before all non-user injected code.

When set, say with Istanbul, to 'istanbul ignore next', such auxiliary transpiled code will not be part of the coverage report.

Example

This:

constructor( x: number, y: number, w: number, h: number ) {
    super(); // <- 'Branch not covered'.
    this.rect = new Rect( x, y, w, h);
}

transpiles to this:

var _this = _super.call(this) || this; // <- || this is not covered. Why would it?

[Izhaki]

I believe that the super() line in this code:

constructor( x: number, y: number, w: number, h: number ) {
    super(); // <- 'Branch not covered'.
    this.rect = new Rect( x, y, w, h);
}

will have to transpile to something like:

var _this = _super.call(this);
/* istanbul ignore next */
_this = _this || this;

rather than:

/* istanbul ignore next */
var _this = _super.call(this) || this;

As with the latter version there will be no coverage for the super constructor.

[Izhaki]

Worth noting that some people solved this by using a replace plugin like this grunt example. Same can be done in babel.

[Izhaki]

Oh, oh , oh, it's magic.

We have recently wrote a new build, with the following coverage setup:

nyc < mocha-webpack <istanbul-instrumenter-loader < awesome-typescript-loader

And it doesn't complain that a branch is not covered. Oddly, even if in tsconfig.json you set target: "es5".

I have not even the slightest idea why this works. Transpiling the files manually to es5 and then running nyc/mocha against them will yield 'branch not covered'. But not through that chain.

Pure magic.

So perhaps this shouldn't be addressed by typescript after all: If your coverage is es6 throughout, the coverage should be sound. In addition, it seems that somehow certain chains get this right even with es5 as target.

[Izhaki]

This issue had very little feedback.

I'm now convinced that instead of this workaround, it is better to simply stick to es6 when producing the coverage report. Here is an example how.

[pelotom]

Hm, it seems unfortunate to give up on this issue. While targeting es6 only in the test environment may be a reasonable workaround for the time being, it's not ideal, because it means you're testing something different than what you get when you build your library "for real".

[Izhaki]

@pelotom But this is only for coverage reports. Do you really need to check coverage for auxiliary code like the || this:

var _this = _super.call(this) || this; // <- || this is not covered. Why would it?

That bit is already covered by typescripts tests.

Anyhow, the request in this issue will effectively do the same - simply tell the coverage tool to ignore certain lines, and unless the emitter is really smart, you'd actually ignore things that you do want to check (see my second comment).

[pelotom]

I suppose if you do one pass to test the code (targeting es5) and another simply to generate coverage information (targeting es6) you can separate the concerns, but I'd rather be able to do them both at once.

[gcnew]

I don't think valuable developer time should be poured in legacy technologies. After all down level emit is more or less byte code. 97% coverage is as good as 100% when you know the reason is actually the generated code. And there are workarounds as well. Either use ES6 or in this case don't use classes at all :)

[Bnaya]

There are other language constructs that should be ignored, like enums

[pelotom]

@gcnew You have an interesting definition of "legacy". I would guess that at present ES5 constitutes the most-executed code in existence.

[Izhaki]

@gcnew

97% coverage is as good as 100%

That depends on how you use code coverage.

It's easy to have 100% code coverage, yet not test all possible scenarios. In more than a few cases, the code coverage was 100%, yet I've found that this or another scenario weren't covered ("How come I don't have a test for this?"). So you add more tests.

So for me - although others may disagree - a code coverage of 100% doesn't mean you're in the clear. But in the way I use coverage reports, anything but 100% immediately teaches me that I've left a stone unturned.

Having a test coverage constantly below 100% kind of defeats its purpose.

[gcnew]

@Izhaki Yes, 100% code coverage does not mean all possible tests are made. It merely means that all code paths have been taken.

One fix would be piping the compiled sources through a script that inserts the comment.

PS: I think the solution that you've found: 'stick to ES6' is a good one, because otherwise you're relying on transpiled code. Transpiled code has completely different use cases than yours and it's optimised for them. Expecting it to behave in a strictly specific way does not seem plausible to me.

[Izhaki]

If you write in C++ you don't run your coverage against the assembly code.

Coverage and tests should be performed using the language being used. Any transformation of such language (transpiled or compiled) is already tested by the transformer.

[Izhaki]

@gcnew

100% code coverage ... merely means that all code paths have been taken.

I bet you I can come up with at least 5 examples where this is incorrect. But the one that springs to mind at this late hour is stubs - you may have a test for a function, but stub it when testing the caller. The stub has broken your path. And even if the stub doesn't match the function behaviour you'd still get 100% coverage.

If processes are graph vertices, and the messages exchanged are the edges, 100% code coverage only teaches you that given your test suite all nodes have been visited at least once. It does not mean the tests are sound; it does not mean all paths have been taken; and it definitely doesn't mean that all the possible data that may flows through the graph (all possible states) have been tested (how do you test array sort?).

Test coverage, when done properly, is Karl Popper: 100% coverage - a theory yet to be falsified - doesn't mean the theory is right; but anything less that 100%, and you've got a falsified theory that is in need of revision.

So here's the paradox:

100% coverage means nothing, but less than that means something.

[kitsonk]

Part of this isn't the "issue" with TypeScript as much as it is the issue with how we remap sources. While I don't disagree that emitting comments in transpiled only code might have some use, it doesn't really solve the problem. In SitePen/remap-istanbul#106 we are going to look at changing the mapping to try to inject some intelligence. Right now it "merges" line coverage, so for example if the transpiled coverage emits 5 lines of code for a single source, that currently equates to if any of those lines get covered, the source line gets reported as covered.

For branches we need to do something similar, to try to determine if the branch is just an artifact of the emit and therefore can be ignored, or if there is a valid branch in the source code.

[gcnew]

@Izhaki It seems we are trying to convey the same information with different words. I actually agreed with you ;).

The best way to test array sort that I've come across is Property-based testing. The library (and company) that made it and popularised it is QuickCheck. The basic idea is that you write your tests in terms of invariants that the tested function has to comply with, and a large number of inputs are generated in an automated manner for you. The hard part is coming up with the invariants.

For array sort one invariant (property) might be:

// the number of elements should not change during sorting
function propSameLength(x) {
    return x.length === sort(x).length;
}

But it's not enough. We also want all elements to be ordered:

function propAscOrder(x) {
    const sorted = sort(x);

    for (var i = sorted.length - 1; i > 0; --i) {
        if (sorted[i - 1] > sorted[i]) {
            return false;
        }
    }

    return true;
}

And finally every element of the original array should be present in the sorted array and vice versa.

function propSameElements(x) {
    const sorted = sort(x);

    return    x.every(e => sorted.indexOf(e) !== -1)
           && sorted.every(e => x.indexOf(e) !== -1);
}

propSameElements has a minor wart. Currently, it does not account for arrays that have the same elements but with different quantities. The following two arrays are considered element same: [1, 2, 2] and [1, 1, 2].

Now, all we have to do is generate a lot of arrays and funnel them through our check function:

function testSort(x) {
    return propSameLength(x) && propAscOrder(x) && propSameElements(x);
}

Property-based testing has a big mindshare in the Haskell community (where it originated from). Practicing it has it's nuisances (e.g. defining how a value should be generated for a specific type), however it is a genuinely novel testing technique. For me, it's the most advanced and thorough approach that we've found so far.

[brunolm]

Workaround

As a workaround I found the special comment /* istanbul ignore next */

If you place after your class, the constructor gets ignored. Has to be exactly after the brace, do not break lines.

Example

export class InternalComponent {
    constructor(private authService: any) { 
    }
} /* istanbul ignore next */

Generates

define(["require", "exports"], function (require, exports) {
    "use strict";
    Object.defineProperty(exports, "__esModule", { value: true });
    var InternalComponent = (function () {
        function InternalComponent(authService) {
            this.authService = authService;
        }
        return InternalComponent;
    }()); /* istanbul ignore next */
    exports.InternalComponent = InternalComponent;
});

Which ignores this line exports.InternalComponent = InternalComponent; that causes the issue.

To ignore super

super() /* istanbul ignore next */;

If you have generics and/or @Inject in your constructor params, try to break lines.

With those hacks I got 100% coverage.

[ben8p]

For the one using webpack and TypeScript configured to output ES5,
I made a little webpack loader:
https://www.npmjs.com/package/ts-es5-istanbul-coverage
Using it make sure you won't get the branch marked as not covered.