Skip to content
This repository has been archived by the owner on Dec 8, 2024. It is now read-only.

Support ignore start/ignore end comments #413

Closed
phated opened this issue Jul 28, 2015 · 7 comments
Closed

Support ignore start/ignore end comments #413

phated opened this issue Jul 28, 2015 · 7 comments

Comments

@phated
Copy link

phated commented Jul 28, 2015

We've been having some discussion on babel to add auxiliary comments around the code it generates. See issue babel/babel#2055

This is useful for code coverage tools like those built into the lab testing library.

I know that istanbul supports ignore next, if, else and was wondering if it could support start/end?

Thanks!

@gotwarlost
Copy link
Owner

This is hard to do for the general case. Sick people can put start /end comments in a way that spans the AST in weird ways

@gotwarlost
Copy link
Owner

I meant "since" not "sick" - damn autocorrect

@sebmck
Copy link

sebmck commented Jul 29, 2015

@gotwarlost What do you mean? The comments don't need to be attached to the AST at all to work out what lines should be ignored, right?

@ariporad
Copy link

ariporad commented Nov 3, 2015

@gotwarlost: +1, this would make babel +Istanbul so much better.

Although, source maps might fix the babel-Istanbul combo entirely.

kpdecker added a commit to kpdecker/istanbul that referenced this issue Nov 13, 2015
Adds the ability to do `/* istanbul ignore start */` paired with `/* istanbul ignore end */` to ignore ranges of code rather than AST subtrees.

Functionally this allows for ignore flags at the same points within the tree and just provides syntax that doesn’t require annotating every AST node with the ignore flags.

When the flags partially cover a AST node, the AST is considered NOT skip, but any children that are fully covered will be skipped. Tests for both of these cases are included.

Fixes gotwarlost#413
@kpdecker
Copy link
Contributor

#483 provides a solution to this that accounts for crazy AST coverage and does not alter the existing ignore hint points.

@scottohara
Copy link

+1, this would make babel +Istanbul so much better.

Although, source maps might fix the babel-Istanbul combo entirely.

Just curious, can anyone confirm if this is actually the case?

Will source map support in istanbul mean that non-user, babel-generated helper code is ignored when reporting coverage? Meaning that we no longer need auxiliary comments at all?

Currently I use auxiliaryCommentBefore: 'istanbul ignore next' in my babel config; but since upgrading from babel 5 -> 6, this is now horribly broken (https://phabricator.babeljs.io/T2757).

As an aside, I'm keenly awaiting source map support (coming in 1.0.0, according to #212), so that I can run coverage reporting on the final output of my build process (the transpiled/minified/concatenated/bundled JS that is ultimately deployed), but have the reports show the original file names/line numbers.

I have three gulp tasks for testing:

Task When Invoked Preprocessing Coverage Reporting?
test:bdd During development Automatically, on file change (via gulp watch) babel No
test:src Before merging/pushing a feature branch or bug fix Manually babel Yes
test:build Before deployment CI (Travis) babel, uglify, concat Yes

The only reason I have both test:src and test:build is that the coverage reports for test:src are easier to read (separate coverage stats for each original file).

The reports for test:build show combined stats for the bundle (eg. app.js or bundle.js), making it harder to see exactly where coverage is missed.

I'm hoping that with source map support, I'll be able to drop test:src and just use test:build; while still retaining coverage reports that show the original file names/statistics.

If source map support also removes the need to use (the now broken) auxiliaryComment{Before,After}, then that will indeed be huge improvement.

@gotwarlost
Copy link
Owner

@scottohara - take a look at this stupid sample project that shows how to integrate babel and istanbul v1 for node.js

https://github.com/istanbuljs/sample-babel-node

Key ideas are:

kpdecker added a commit to kpdecker/istanbul that referenced this issue Nov 24, 2016
Adds the ability to do `/* istanbul ignore start */` paired with `/* istanbul ignore end */` to ignore ranges of code rather than AST subtrees.

Functionally this allows for ignore flags at the same points within the tree and just provides syntax that doesn’t require annotating every AST node with the ignore flags.

When the flags partially cover a AST node, the AST is considered NOT skip, but any children that are fully covered will be skipped. Tests for both of these cases are included.

Fixes gotwarlost#413
@phated phated closed this as completed Sep 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants