gotwarlost / istanbul

Yet another JS code coverage tool that computes statement, line, function and branch coverage with module loader hooks to transparently add coverage when running tests. Supports all JS coverage use cases including unit tests, server side functional tests and browser tests. Built for scale.
Other
8.7k stars 787 forks source link

Branch Coverage - else statement location is always the same as if statement location #712

Open shaicantor opened 7 years ago

shaicantor commented 7 years ago

Hi there,

In the instrumenter code while walking on an if statement node and building the branch map I've noticed that the else statement locations is always the same as the if statement locations. Why is that?

The code that does this is located here - https://github.com/gotwarlost/istanbul/blob/master/lib/instrumenter.js#L978

ifBranchInjector: function (node, walker) {
            var alreadyIgnoring = !!this.currentState.ignoring,
                hint = this.currentState.currentHint,
                ignoreThen = !alreadyIgnoring && hint && hint.type === 'if',
                ignoreElse = !alreadyIgnoring && hint && hint.type === 'else',
                line = node.loc.start.line,
                col = node.loc.start.column,
                makeLoc = function () { return  { line: line, column: col }; },
                bName = this.branchName('if', walker.startLineForNode(node), [
                    { start: makeLoc(), end: makeLoc(), skip: ignoreThen || undefined },
                    { start: makeLoc(), end: makeLoc(), skip: ignoreElse || undefined }
                ]),
                thenBody = node.consequent.body,
                elseBody = node.alternate.body,
                child;
            thenBody.unshift(astgen.statement(this.branchIncrementExprAst(bName, 0)));
            elseBody.unshift(astgen.statement(this.branchIncrementExprAst(bName, 1)));
            if (ignoreThen) { child = node.consequent; child.preprocessor = this.startIgnore; child.postprocessor = this.endIgnore; }
            if (ignoreElse) { child = node.alternate; child.preprocessor = this.startIgnore; child.postprocessor = this.endIgnore; }
        }
seanpoulter commented 6 years ago

Did you ever follow-up on this issue in the new monorepo @shaicantor? This is the only relevant Issue I could find ...

shaicantor commented 6 years ago

@seanpoulter, no. We moved to nyc and istanbul-lib-instrument. If I'm not mistaken, it has the same issue there as well.

https://github.com/istanbuljs/istanbuljs/blob/master/packages/istanbul-lib-instrument/src/visitor.js#L362

seanpoulter commented 6 years ago

Yea, istanbul-lib-instrument has the same behavior. I've opened istanbuljs/istanbuljs#130 with the same name. Thanks @shaicantor.