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

calling method on super: branch not covered #690

Open
dcrockwell opened this issue Aug 28, 2016 · 40 comments
Open

calling method on super: branch not covered #690

dcrockwell opened this issue Aug 28, 2016 · 40 comments

Comments

@dcrockwell
Copy link

export default class MyComponent extends Component {
    link(name, Constructor) {
        super.link(name, Constructor);
    }
}
class AnotherComponent extends Component {}

describe("MyComponent", () => {
    it("should be covered", () => {
        const myComponent = new MyComponent();
        myComponent.link("anotherComponent", AnotherComponent);
    });
});

This results in:
screen shot 2016-08-28 at 1 09 32 pm

@dcrockwell
Copy link
Author

I can workaround to get 100% coverage by doing the following:

export default class MyComponent extends Component {
    constructor() {
        super();
        this.superLink = super.link();
    }
    link(name, Constructor) {
        this.superLink(name, Constructor);
    }
}
class AnotherComponent extends Component {}

describe("MyComponent", () => {
    it("should be covered", () => {
        const myComponent = new MyComponent();
        myComponent.link("anotherComponent", AnotherComponent);
    });
});

@yanneves
Copy link

this is reproducible for me on any call to super or one of its properties with at least one argument

@dobryanskyy
Copy link

is reproducible for me as well v 1.1.0-alpha.1. Marks opening bracket of each constructor where i call super as not covered branch

@noamokman
Copy link

Just called super in a constructor and got an uncovered branch 😞

@brandonsturgeon
Copy link

Having this problem too. I have to use the /* istanbul ignore next */ hack way more than I'd like to.

@bitttttten
Copy link

bitttttten commented Oct 25, 2016

I've just got

import React, { Component } from 'react'
export default class Main extends Component {
    constructor(props) {
        super(props)
        this.state = {
            counter: 0
        }
    }
    render() {
        {this.state.counter}
    }
}

And I get a notice that my constructor branch is not being covered and so then a less than 100% score on my branches.

Is this related?

@brandonsturgeon
Copy link

@bitttttten

That is the issue we're all complaining about, I believe.

@tconroy
Copy link

tconroy commented Nov 7, 2016

any news on a fix for this? I'm running into the same problem. Can't get my coverage to 100% as a result. :/

@bitttttten
Copy link

bitttttten commented Nov 7, 2016

not as far as I know, just put down your expected coverage to what you get with this issue. that's what I am doing.

@dobryanskyy
Copy link

I also did not find workaround for this. Thus I moved to nyc(which uses istanbul under cover) with babel and now everything is calculated correctly.

@brandonsturgeon
Copy link

@dobryanskyy @tconroy

Another option is to put /* istanbul ignore next */ right above your constructor and istanbul won't bug you about coverage for your constructor

@Drasky-Vanderhoff
Copy link

Found the same issue, what is exactly generating this problem ? Some ES6 compatibility ? I'm working with React and that's a very common airbnb pattern.

@owenDods
Copy link

owenDods commented Dec 8, 2016

Also came up against this and found that declaring static properties above the constructor causes the /* istanbul ignore next */ statement to not work:

screen shot 2016-12-08 at 10 45 44

But seems fine with:

screen shot 2016-12-08 at 10 46 13

@owenDods
Copy link

owenDods commented Dec 8, 2016

Found a workaround for the above problem I posted that doesn't anger the AirBnB lint gods (i.e. the react/sort-comp rule):

screen shot 2016-12-08 at 11 18 44

It seems that adding the /* istanbul ignore next */ to before the statement that immediately follows the call to super fixes it:

screen shot 2016-12-08 at 11 17 32

@brandonsturgeon
Copy link

Nice find @owenDods !

I remember also running into that issue and just scrapping the static method entirely.

@owenDods
Copy link

owenDods commented Dec 8, 2016

Yeah @brandonsturgeon, I almost did the same. But I didn't want to alter my code because of a coverage bug! It was a matter of principle!

@Izhaki
Copy link

Izhaki commented Dec 12, 2016

I've been using 1.1.0-alpha.1 for a while, and saw this for the first time this morning after fresh npm install.

On another machine, where 1.1.0-alpha.1 was installed quite some time ago, this issue doesn't occur.

@Izhaki
Copy link

Izhaki commented Dec 18, 2016

OK, some more details:

This doesn't happen with [email protected], but it does with [email protected]

For this code:

    constructor( x: number, y: number, w: number, h: number ) {
        super(); // Branch not covered.
        this.rect = new Rect( arguments[ 0 ], arguments[ 1 ], arguments[ 2 ], arguments[ 3 ] );
    }

In 2.1.4 super() emits (clearly this will throw Branch not covered):

        var _this = _super.call(this) || this;

Whereas 2.0.3 emits:

    _super.call(this);

So it seems that this is caused by a change to how typescript emits.

@Izhaki
Copy link

Izhaki commented Mar 16, 2017

I believe that this one is for typescript to fix.

Cast your vote and opinions: microsoft/TypeScript#13455

pelotom added a commit to runtypes/runtypes that referenced this issue Mar 20, 2017
@Izhaki
Copy link

Izhaki commented Mar 20, 2017

I'm now convinced that it is best to simply stick to es6 when producing the coverage report - the emitted auxiliary code only reduces coverage with an es5 target. Here is an example how.

Related:
microsoft/TypeScript#13455

@brunolm
Copy link

brunolm commented Aug 23, 2017

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.

@rtskmr
Copy link

rtskmr commented Oct 23, 2017

I got the same issue as

{this.state.isDeleteVisible ?
<ImageButton name="search-bar-icon-delete" source={{ uri: SEARCH_ICON_DELETE_IPHONE }} style={styles.searchBarDeleteIcon} pressedStyle={styles.closeIconPressed} action={alert}/> : null}

@ben8p
Copy link

ben8p commented Dec 13, 2017

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.

@Dji75
Copy link

Dji75 commented Jan 5, 2018

Got the same issue. Workaround works.
@davinkevin, @Neonox31

@jfrej
Copy link

jfrej commented Jan 25, 2018

I ran into the same problem running React with babel, webpack and karma. What solved it was switching from istanbul-instrumenter-loader to babel-plugin-istanbul

@rpresb
Copy link

rpresb commented Mar 2, 2018

By installing babel-plugin-istanbul solved for me

@srihari-sridharan
Copy link

Using /* istanbul ignore next */ worked for me.

@FrancescoBorzi
Copy link

The bug is still there in version 0.4.5. Are there any news? @gotwarlost can you please have a look?

@Mischala
Copy link

Mischala commented Nov 20, 2018

Ignoring it works, but that doesn't really solve the problem.
Still getting this bug when using ts-node with mocha and targeting ES5

BelfordZ added a commit to open-rpc/schema-utils-js that referenced this issue Apr 12, 2019
@FrancescoBorzi
Copy link

Any news about this?

FrancescoBorzi added a commit to azerothcore/Keira3 that referenced this issue Jul 13, 2019
@melgish
Copy link

melgish commented Sep 30, 2019

Just ran into this and found that removing the semicolon after
super(); // <-- this one

raised coverage back to 100% ... at least until prettier put it back :/

@simlevesque
Copy link

You can replace this: super();
With this: super()/* istanbul ignore next */;

@FrancescoBorzi
Copy link

I can confirm this is still an issue

FrancescoBorzi added a commit to azerothcore/Keira3 that referenced this issue Mar 27, 2020
@maddy619
Copy link

This is still an issue..!

@jeancatarina
Copy link

I need a workaround

image

@smitdesai16
Copy link

I can workaround to get 100% coverage by doing the following:

export default class MyComponent extends Component {
    constructor() {
        super();
        this.superLink = super.link();
    }
    link(name, Constructor) {
        this.superLink(name, Constructor);
    }
}
class AnotherComponent extends Component {}

describe("MyComponent", () => {
    it("should be covered", () => {
        const myComponent = new MyComponent();
        myComponent.link("anotherComponent", AnotherComponent);
    });
});

you are moving problem from that line to constructor

@maddy619
Copy link

I can workaround to get 100% coverage by doing the following:

export default class MyComponent extends Component {
    constructor() {
        super();
        this.superLink = super.link();
    }
    link(name, Constructor) {
        this.superLink(name, Constructor);
    }
}
class AnotherComponent extends Component {}

describe("MyComponent", () => {
    it("should be covered", () => {
        const myComponent = new MyComponent();
        myComponent.link("anotherComponent", AnotherComponent);
    });
});

you are moving problem from that line to constructor

thats good but what if I don`t want to write superlink, or just imagine if I have 50 components

@royalpinto
Copy link

I can confirm this is still an issue

@whyboris
Copy link

I'm missing branch coverage because of the ; :trollface:
image

frederikprijck added a commit to auth0/auth0-spa-js that referenced this issue Sep 6, 2021
frederikprijck added a commit to auth0/auth0-spa-js that referenced this issue Sep 8, 2021
…new tokens (#789)

* Expose mfa_token from the mfa_required error when getting new tokens

* Ignore super calls from istanbul

See: gotwarlost/istanbul#690

* Export MfaRequiredError

Co-authored-by: Steve Hobbs <[email protected]>
@nbilyk
Copy link

nbilyk commented Nov 9, 2021

The way I worked around this issue (sorry, I can't share the code) is using a mock (in my case, jasmine).
The es5 generated code has a branch:
var _this = _super.call(this) || this;
So _super.call needs to be mocked.

// Code that needs coverage
  class MyError extends Error {
    constructor(message: string) {
       super(message) // Generated creates a branch
    }
  }

In my test:

  new MyError() // Normal coverage
  spyOn(Error, 'call').and.returns(null)
  new MyError() // Coverage where Error class doesn't exist
   

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

No branches or pull requests