'istanbul ignore else' inbetween a 'else' and 'if' is ignored

[jwalton]

See also #230.

You can work around it with some extra braces:

[nmarra]

I'm having this same issue, it seems like a problem with the branch coverage though because as the coverage shows the 'else if' case is covered by a unit test. It's very confusing to get the 'else path not taken' marking when the 'else if' is clearly covered as shown by the 11x marking on the right side.

[woutervanvliet]

Just found this issue when running into the same problem. I solved it by adding the ignore line above the first if, like so:

/* istanbul ignore else */
if (element) {
	element.addEventListener('animationend', this.handleAnimationEnd)
} else if (this.contentElement) {
	this.contentElement.removeEventListener('animationend', this.handleAnimationEnd)
}

[AlexisWilke]

@woutervanvliet Yeah but then the first else is likely ignored too when we want to know whether that first else was taken! We only want to ignore the last one.

[getify]

Bump.

This is annoying. I have a list of if..else if.. statements that does not need an else clause, because it's impossible to get anything else other than what's accounted for. Similar problem if I use a switch statement.

I've confirmed that if I put the /* istanbul ignore else */ above the first if, it ignores all the else if clauses, not just the final if.

My annoyingly bad choices are:

1. use the /* istanbul ignore else */ above the first if, and lose the benefit of Istanbul tracking my coverage of all my else if statements.

2. live with less than 100% coverage

3. forcibly pass a wasted piece of bad data into this function to get it to hit that phantom else condition

4. refactor the if...else if... series like this:

    if (one) { .. }
    else if (two) { .. }
    else if (three) { .. }
    else {
       /* istanbul ignore else */ 
       if (four) { .. }
    }
   

I picked option (4), but it's extra annoying because not only do I have to contort the code in that weirder shape, but then my linter complains about the "lonely if", so I have to put another comment to get the linter to hush. :/

[alasdairhurst]

Something like /* instanbul ignore-final-else */ would be pretty useful for stuff like this. I'd actually simplify the choices we have to the following:

• Lose coverage tracking
• Live with less coverage
• Decrease code simplicity/performance

[a4xrbj1]

So is there any solution to this? I agree @getify that the workaround isn't working for all situations.

[livingston]

this works

if (condition1) {
} else /* istanbul ignore next */
if (condition) {

Didn't mean that we should use this, just because it works.

[arleyyap]

Can someone explain to me why the solution of @livingston should not be done?. it worked for me.

[getify]

@arleyyap this "suggested solution" doesn't fit the form of the problem I brought up (which differs from the OP)... consider:

if (condition1) {
  // ..
} /* istanbul ignore next */
else {
  // ..
}

vs

if (condition1) {
  // ..
} else /* istanbul ignore next */
if (whatShouldGoHere) {
  // ..
}

In the first snippet, we're trying to ignore an else... which doesn't work -- that's my complaint. In the second snippet, the ignoring is of an if that's part of an else... but what condition should go there for an else that isn't ever planned to fire?

This hack could "work", but IMO is as janky (or worse) than some other work-arounds discussed earlier in the thread:

if (condition1) {
  // ..
} else /* istanbul ignore next */
if (false) {
  // ..
}

[pampananagasatish]

I am also facing the same issue while running test case with only if statement. Even '/* istanbul ignore else */' statement doesn't ignore else case and results branch coverage less.

[basickarl]

The /* istanbul ignore else */ should only affect it's own level and not affect nested if else statements.

[elliot-nelson]

Definitely still a problem in [email protected] and [email protected]. In fact, else if statements cause endless problems for both ignore if and ignore else... example:

/* istanbul ignore if: this can cover branch 1 */
if (false) {
    console.log("branch 1");
} /* istanbul ignore if: does not work */ else /* istanbul ignore if: not here either */ if (false) {
    console.log("branch 2");
} else if (false) {
    console.log("branch 3");
}

As soon as you need to ignore an if entry in a series of else if chains, you can never ignore another one. Your only option is to use an ignore next -- but that actually ignores the entire rest of the chain, including branch 3.

(I realize this is slightly different than the ignore else case in the OP, but I think it's part of the larger problem with else-if chains.)

EDIT: I guess my example could be solved by putting /* istanbul ignore next*/ inside each if block, which isn't bad, whereas there is no easy solution for the else case. So maybe my complaint is lower priority.

[mgabeler-lee-6rs]

Still busted, still annoying as heck ...

I guess my example could be solved by putting /* istanbul ignore next*/ inside each if block, which isn't bad, whereas there is no easy solution for the else case.

This doesn't quite work -- I tried this too, and while it gets rid of part of the issue, it still results in the else if reporting that the else case was not taken

---

2 cents: #230 should never have been closed because the advice given doesn't actually work.

[fubar]

Using ignore next right before else seems to work fine.

} /* istanbul ignore next */ else if (foo) {

[getify]

@fubar that was already suggested up-thread 2 years ago.

[fubar]

@getify the only mention or suggestion of an "ignore next" (not "ignore else") before (not after) an else statement that I can see in this thread is in your comment (https://github.com/gotwarlost/istanbul/issues/781#issuecomment-572738364), in which you said it does not work, whereas it does work for me, so I figured someone might find it helpful.

I have not tested this in chained else statements; however, chaining elses is not exactly good practice, and I'd suggest improving the OP's code by getting rid of the unnecessary else statements and using guards instead:

if (location) {
  return ...;
}
if (timezone) {
  return ...;
}
/* istanbul ignore next */
throw new Error(...);

[getify]

@fubar Here's the comment from 2 years ago I'm referring to: https://github.com/gotwarlost/istanbul/issues/781#issuecomment-528292165

[fubar]

Like I said, the "ignore next" in that comment is after the else, not before. I'm not sure if istanbul interprets it any differently though. And when I put it behind the else, prettier reformats it to be before it, so there may be another reason for it that I'm not aware of.

[greim]

It was counter-intuitive, but I finally managed to isolate the last else by inserting /* istanbul ignore else */ before the second-to-last step in the chain.

if (val === 'up') {
  // ...
} else /* istanbul ignore else */ if (val === 'down') {
  // ...
} else {
  // this should never happen and should
  // be ignored from branch coverage
  assertNever(val, `${val} is invalid, should be up or down`);
}

I think the problem is thinking of if/else if/else as a list data structure, whereas Istanbul sees it as a nested data structure. Making braces explicit makes it clear:

// how istanbul sees the above code...
if (val === 'up') {
  // ...
} else {
  /* istanbul ignore else */
  if (val === 'down') {
    // ...
  } else {
    // this should never happen and should
    // be ignored from branch coverage
    assertNever(val, `${val} is invalid, should be up or down`);
  }
}

[fubar]

@greim make it a switch (or guards if you can make it a separate function). Excessive else statements and nested ifs are code smells. In fact, your second example would be reformatted by prettier into an else if.

switch (val) {
  case 'up':
    // ...
    break;
  case 'down':
    // ...
    break;
  /* istanbul ignore next */
  default:
    throw new Error(`Invalid value: ${val}`);
}

[greim]

@fubar - I didn't mean to suggest my second example is how code should be written, but rather to illustrate what Istanbul sees when it parses my first example, in order to understand its behavior here. Hope that clears it up.

[getify]

@greim this is basically what I suggested up-thread here.

[greim]

@getify You suggested putting the comment immediately above the first if. Instead, try putting it in the second-to-last position like this:

if (thing) {
  // included
} else /* istanbul ignore else */ if (thing) {
  // included
} else {
  // excluded
}

As far as I can tell, nobody in this thread has suggested this, except for OP, who was apparently complaining it doesn't work? Maybe something got fixed in the interim, but it works fine for me.