You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
/* c8 ignore next */ does not ignore classes or functions #544
I ran into this recently. c8's ignore next doesn't work like Istanbul's. As stated in the readme, it only ignores the next line. Sadly, c8 doesn't seem to support ignoring the next whole block or specified blocks like ignore next branch.
@guilhermehn actually, this re-opens the question: why would we want to ignore the fact that a part of the code is not covered? This question already makes sense for lines, but it is even more critical for classes: why would we want to ignore a class from the coverage? What is the benefit of this? Why write a class if it is never tested anywhere?
@ericmorand I agree, most of the time you're doing it wrong when a piece of code is ignored and not tested. I'm not a fan of ignoring code coverage, but there are legitimate use-cases. The two I've seen most often is environment related, like using different values depending on the current environment the application is running (homolog/production or even browser/server), and analytics related. There are easy ways to circumvent the former (without duplicating tests), but the latter can be more of a hassle, specially when dealing with third-party APIs. In the company I work for there is a layer of abstraction for the Google Analytics API, which is accessible by a global variable in the window (like window.myCompanyAnalytics). The login page can have multiple branching paths depending on the user and it's condition, and we have to send multiple events detailing which step the user is going through and what actions did they took, so there is a lot of analytics code in mangled with the login front-end logic code, and there are certain events that would only be sent when an specific condition is met, e.g. if (userAccount.hasSpecificRestriction) window.myCompanyAnalytics(eventData).
One could argue that I could create tests for those cases, mocking the global API, but remember that there are a lot of other instances of this, so I would have twenty-some tests just to cover some lines that are not really testing much (it's just testing the mock I've created), or that there are better ways to do these things, like creating an abstraction of this analytics-related code. But, coming back to your question, I return another question: why should I create tests that are only there for coverage? You could say that it tests that the analytics API is actually called given the necessary conditions, but how valuable is that test in reality, since the mock was created by me (aka not real code) and the if statement is guaranteed by the runtime to function properly? Should I refactor lots of lines just so that the required coverage percentage is met?
I should be clear here: I don't like ignoring code coverage, 99.99% of the time I'm against it and I always aim for 100% coverage. But there are real cases where the cost of covering a single if statement is way higher than the value it yields. When the options available are:
create a test to check that a mock is called;
refactor a lot of code to abstract some if;
ignore said if block;
Ignoring it could be a valid answer depending on how much it would cost for the project/company.
One is with classes and inheritance. Setting aside any/all "inheritance is bad" arguments, JS doesn't have a good way to express methods of a class that are required to be implemented by subclasses, so I tend to do things like this:
class Foo {
/* c8 ignore next */
subclassShouldDefine() {
throw new Error(`Class ${this.constructor.name} does not implement subclassShouldDefine`);
}
}
I don't expect this error to ever be thrown, but it's helpful to detect such errors are runtime with a meaningful message. Maybe it's reasonable to say that this behavior should actually be tested, but I'm not sure I really care. It's a mild improvement over the default "no such method" error (or whatever JS spits out), and I don't think I really care if it doesn't do what it's supposed to.
The second use case complex to describe, but involves convoluted error cases in async browser operations. It's notoriously difficult to mock the behavior of native browser objects (e.g. IndexedDB), whose APIs are predominantly callback based. Absent a detailed explanation of this (which I could do, but would take a lot of words), suffice it to say that sometimes the effort needed to orchestrate a mock of the conditions required to exercise the code under test is so large, that the risk of having untested code that might be very, very unlikely to fire is preferrable to spending the time to create a valid test. In these cases, I might just ignore a block and get on with life.
Temporary ignores is another case. I find I'm constantly chasing coverage (I don't subscribe to the TDD philosophy, at all), and I will sometimes have classes or methods that are not fully baked, and not yet complete. I tend to want to just ignore them until I get around to implementing them completely.
There are a few other examples that I could cite, all of which are (frankly) debatable in their validity, but the main point (and the point of this issue), is that istanbul has/had this facility. Maybe it's valid to say, "it shouldn't exist" (I don't think it is), but even if we take that viewpoint, it's really just a compatibility issue. if c8 wants to be a drop-in replacement for istanbul (which it can be, and should be IMO); then it should support the same syntax and semantics, independent of any philosophical stance on what's "right" or "wrong" about how to correctly unit test.
Im migrating the npm cli to use a newer version of tap which uses c8 instead of istanbul and a lack of a comment in c8 that ignores a block and parallels /* istanbul ignore next */ makes the conversion much more difficult, can't just search and replace. Is there any workarounds?
This isn't really only about classes or functions, but about any constructs that span multiple lines, e.g.:
/* c8 ignore next */if(false){console.log('this should be ignored');}console.log('covered');
This would ignore the whole if block in Istanbul, but with c8, lines 3 and 4 are reported as uncovered. c8 requires either manually counting the lines to ignore:
/* c8 ignore next 3 */if(false){console.log('this should be ignored');}console.log('covered');
or using a start/stop pair:
/* c8 ignore start */if(false){console.log('this should be ignored');}/* c8 ignore stop */console.log('covered');
both of which are quite annoying/noisy to use in comparison with Istanbul's approach.
Activity
guilhermehn commentedon Oct 17, 2024
I ran into this recently. c8's
ignore nextdoesn't work like Istanbul's. As stated in the readme, it only ignores the next line. Sadly, c8 doesn't seem to support ignoring the next whole block or specified blocks likeignore next branch.ericmorand commentedon Oct 26, 2024
@guilhermehn actually, this re-opens the question: why would we want to ignore the fact that a part of the code is not covered? This question already makes sense for lines, but it is even more critical for classes: why would we want to ignore a class from the coverage? What is the benefit of this? Why write a class if it is never tested anywhere?
guilhermehn commentedon Oct 27, 2024
@ericmorand I agree, most of the time you're doing it wrong when a piece of code is ignored and not tested. I'm not a fan of ignoring code coverage, but there are legitimate use-cases. The two I've seen most often is environment related, like using different values depending on the current environment the application is running (homolog/production or even browser/server), and analytics related. There are easy ways to circumvent the former (without duplicating tests), but the latter can be more of a hassle, specially when dealing with third-party APIs. In the company I work for there is a layer of abstraction for the Google Analytics API, which is accessible by a global variable in the
window(likewindow.myCompanyAnalytics). The login page can have multiple branching paths depending on the user and it's condition, and we have to send multiple events detailing which step the user is going through and what actions did they took, so there is a lot of analytics code in mangled with the login front-end logic code, and there are certain events that would only be sent when an specific condition is met, e.g.if (userAccount.hasSpecificRestriction) window.myCompanyAnalytics(eventData).One could argue that I could create tests for those cases, mocking the global API, but remember that there are a lot of other instances of this, so I would have twenty-some tests just to cover some lines that are not really testing much (it's just testing the mock I've created), or that there are better ways to do these things, like creating an abstraction of this analytics-related code. But, coming back to your question, I return another question: why should I create tests that are only there for coverage? You could say that it tests that the analytics API is actually called given the necessary conditions, but how valuable is that test in reality, since the mock was created by me (aka not real code) and the if statement is guaranteed by the runtime to function properly? Should I refactor lots of lines just so that the required coverage percentage is met?
I should be clear here: I don't like ignoring code coverage, 99.99% of the time I'm against it and I always aim for 100% coverage. But there are real cases where the cost of covering a single if statement is way higher than the value it yields. When the options available are:
Ignoring it could be a valid answer depending on how much it would cost for the project/company.
jasonpolites commentedon Oct 28, 2024
There are a couple of use cases I run into.
One is with classes and inheritance. Setting aside any/all "inheritance is bad" arguments, JS doesn't have a good way to express methods of a class that are required to be implemented by subclasses, so I tend to do things like this:
I don't expect this error to ever be thrown, but it's helpful to detect such errors are runtime with a meaningful message. Maybe it's reasonable to say that this behavior should actually be tested, but I'm not sure I really care. It's a mild improvement over the default "no such method" error (or whatever JS spits out), and I don't think I really care if it doesn't do what it's supposed to.
The second use case complex to describe, but involves convoluted error cases in async browser operations. It's notoriously difficult to mock the behavior of native browser objects (e.g. IndexedDB), whose APIs are predominantly callback based. Absent a detailed explanation of this (which I could do, but would take a lot of words), suffice it to say that sometimes the effort needed to orchestrate a mock of the conditions required to exercise the code under test is so large, that the risk of having untested code that might be very, very unlikely to fire is preferrable to spending the time to create a valid test. In these cases, I might just ignore a block and get on with life.
Temporary ignores is another case. I find I'm constantly chasing coverage (I don't subscribe to the TDD philosophy, at all), and I will sometimes have classes or methods that are not fully baked, and not yet complete. I tend to want to just ignore them until I get around to implementing them completely.
There are a few other examples that I could cite, all of which are (frankly) debatable in their validity, but the main point (and the point of this issue), is that istanbul has/had this facility. Maybe it's valid to say, "it shouldn't exist" (I don't think it is), but even if we take that viewpoint, it's really just a compatibility issue. if c8 wants to be a drop-in replacement for istanbul (which it can be, and should be IMO); then it should support the same syntax and semantics, independent of any philosophical stance on what's "right" or "wrong" about how to correctly unit test.
reggi commentedon Jul 1, 2025
Im migrating the
npm clito use a newer version oftapwhich usesc8instead ofistanbuland a lack of a comment in c8 that ignores a block and parallels/* istanbul ignore next */makes the conversion much more difficult, can't just search and replace. Is there any workarounds?MartinKolarik commentedon Jun 22, 2026
This isn't really only about classes or functions, but about any constructs that span multiple lines, e.g.:
This would ignore the whole
ifblock in Istanbul, but withc8,lines 3 and 4 are reported as uncovered.c8requires either manually counting the lines to ignore:or using a start/stop pair:
both of which are quite annoying/noisy to use in comparison with Istanbul's approach.