-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add conditional ignore support via //#JSCOVERAGE_IF #537
Conversation
-1 from me, jscoverage is just an implementation detail right now, I dont really want anything jscoverage-specific in other people's source |
Makes sense. Would you be open to my adding a new instrumentation-agnostic syntax, recognized directly in json-cov? My own personal preference would be to have a comment along the lines of "//#SUPPRESS_BLOCK_COVERAGE" that does exactly what it claims -- excludes from coverage statistics whatever block of code the comment is in, with no need for an endif. On Aug 10, 2012, at 11:52 AM, TJ Holowaychuk <[email protected]mailto:[email protected]> wrote: -1 from me, jscoverage is just an implementation detail right now, I dont really want anything jscoverage-specific in other people's source — |
I would rather just ignore those blocks in the reports, 100% coverage is a fallacy anyway |
I agree wholeheartedly that 100% coverage is a fallacy. To me, that's exactly why something like this is a huge help. I can skim through my reports and know that a red line means "cover this, or formally declare that it's not worth covering." This way, red lines are always "Stuff I Should Think About." |
yeah, hmmm there's gotta be a nicer way to do it, not a fan of the inline comment thing |
I'm not wild about it, but I must admit that I'm stumped for alternatives. It seems like we start with two possible solutions:
#2 sounds bad. I'd hate to have to maintain that exclusion file, and I'd hate to step into someone's project with no familiarity with these tools and try to divine that there's a magic file somewhere that dictates certain blocks are ignored. That means we have metadata to encode into the file somehow. I see 3 broad categories of places we can do that:
Let's just go ahead and ignore #3. I don't think anyone wants to write a preprocessor just for this, nor would it solve any problems. I mention it purely for completeness. So that leaves comments, or magic code. With magic code, the idea would seem be that there would be some property of deliberately untested code that json-cov could recognize. It seems highly unlikely that we can find a way to automatically infer which code is deliberately untested, so it would have to be some explicit alteration made to the structure of the code. I guess there's a few ways you could do that, but the whole premise sounds pretty bad to me. I shouldn't have to restructure code if I want to decide to exclude it. (edit: I was thinking about it and there are non-structural ways to do magic code; e.g. we could do something similar to "use strict", but I can't think of an advantage in that versus comments.) I guess we could do magic code that's not in-line, but then essentially we have a magic block of code somewhere with exclusion data -- with the same maintenance problems that we had with an external problem. So that leaves the non-executable components of the source: comments and whitespace. I don't think we can do whitespace-based exclusion. I mean, we CAN, but... ick. So it seems to me that for those who wish to exclude code, comments are the only practical option. |
@jonasacres - Is there a solution to this available somewhere? |
I've been using the patch I submitted in this pull request and that's added support back in. I haven't tried merging it into a newer version of mocha, so if this isn't working so well now, please let me know. |
@jonasacres - Do you think you can publish to npm? |
pretty obscure, closing because I think almost no one on the planet uses this feature of jscoverage |
If you use component this is actually a really useful feature for running front-end tests on your built JS file. Really anybody who wants to run tests on a fully built JS file and doesn't want to collect test coverage for dependencies would need this |
Can we brainstorm a bit about how we can round out mocha's code coverage tools to do suppression of blocks that do not require testing in a way that fits with your vision for the project? Because honestly, I'm building large-scale backends with a team, and I'm finding code coverage tests to be a great way to ensure consistent quality. Like you, I think 100% coverage is a fallacy and do not wish to waste anyone's time testing cases that just don't need test coverage. But as it stands, without code suppression, our coverage reports would have decreasing value as the size of the source tree increases. |
JSCoverage has support for doing conditional measurement of lines by adding //#JSCOVERAGE_IF condition to the start of a line. If condition evaluates to false, it is expected that the lines enclosed by //#JSCOVERAGE_IF and //#JSCOVERAGE_ENDIF will not be counted towards SLOC, hits or misses, regardless of whether the enclosed lines are executed or not.
Although node-jscoverage generates instrumentation for this, json-cov does not factor this into its analysis, nor does html-cov have the capability to use those statistics in rendering the HTML file.
This pull request causes json-cov to track conditionally ignored code. Objects representing an ignored line have "ignore":true; other lines omit the ignore property, to avoid adding unwanted verbosity. Code which is ignored does not count towards SLOC, hits or misses; code which is conditionally enabled is counted towards statistics as normal. A "skips" tally is added to overall statistics.
html-cov's templates are also updated to highlight conditionally ignored code in yellow. The templates do not make use of the skipped line statistics.