node-cover icon indicating copy to clipboard operation
node-cover copied to clipboard

Allow ignoring lines

Open phihag opened this issue 12 years ago • 7 comments

Sometimes, I'm aware that a certain line is not going to be covered (say, because it's related to another platform. For example, when I run my test under node.js, browser-specific branches are not going to be touched).

There should be an option to ignore those lines (so that I can still automatically require 100% coverage). It could look like this:

if (!require) {
    XMLSerializer = window.XMLSerializer; // pragma: ignore
}

In the HTML output, the ignored lines could be marked with a light yellow hint.

phihag avatar Jul 19 '12 19:07 phihag

That'd be great. One issue is that we're dependent on Esprima to do the parsing, but if I can somehow get the comment attached that parse node, I can probably do it. There have been some new things with Esprima recently.

itay avatar Jul 19 '12 19:07 itay

I'm working on that. I've some questions if you're still interested in that feature:

  • syntax of the comment ( // cover:false ?)
  • should it only ignore one line or the whole sub-tree?

Example: var a = function(){ // cover:false //... };

I think it should ignore the whole function, not just the variable assignment.

It's a small change (~ 15 LOC in instrument.js) + an esprima.js update.

floriancargoet avatar Dec 04 '12 15:12 floriancargoet

Excellent.

I don't really care about the syntax. pragma:ignore is apparently used by some exotic C# validators, and Python's nosetest uses pragma: no cover. cover: false is excellent and allows for extensions such as cover: block-false.

Ignoring the subtree would be nice. However, that'd make code like

{foo;}; bar = function() { // cover: false
  baz;
}.blorg();
if (x) { // cover:false
  blarg;
};
blurg;

ambiguous, wouldn't it? At least I wouldn't be sure whether that included foo, the assignment to bar, baz, or blorg. And what exactly does this block-bases solution apply to, any block with braces or just new scopes (i.e. functions)?

So, personally, I'd be happy with the line-based solution. However, I imagine a lot of other users would love to ignore whole blocks, so why not implement both?

phihag avatar Dec 04 '12 16:12 phihag

  • Being on an ignored line, foo would be ignored.
  • Being on an ignored line, the assignment to bar would be ignored.
  • Being inside a syntax tree started on an ignored line, baz would be ignored.
  • .blorg() belongs to a statement started on an ignored line, it would be ignored.

The "block-based solution" is in fact a node-based solution. And by node, I mean a node from the syntax tree generated by esprima.

When the code encounters a "cover:false", I can say:

  • ignore this node
  • ignore this node and its descendants

@itay what do you think?

P.S.: code's coming, I'm preparing a pull request.

floriancargoet avatar Dec 04 '12 16:12 floriancargoet

@floriancargoet Great, that works. Thank you very much for implementing it!

phihag avatar Dec 04 '12 16:12 phihag

@floriancargoet this is great, I would be happy to merge it in. I'd also be happy to give you commit access if there are other improvements you'd like to do. I would love to have another contributor to help get things going :)

itay avatar Dec 04 '12 17:12 itay

Here's the pull request. Hope you like it. About the commit access, let's first see if I come up with other fixes/features ideas.

floriancargoet avatar Dec 04 '12 17:12 floriancargoet