Skip to content
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

fix: ignore lines have to be dropped from coverage and not to be included as covered #240

Closed
wants to merge 2 commits into from

Conversation

anthony-redFox
Copy link

Html coverage report base on statementMap and s then if line doesn't exists will be marked 'neutral' by default.

It will be repeat behavior "istanbul ignore"

@anthony-redFox
Copy link
Author

Also add new commit which will be ignore empty lines and JavaScript comments

Copy link
Contributor

@AriPerkkio AriPerkkio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excluding the lines based on whitespace can be error prone. How does it report following case?

1 | function unCoveredFunction() {
2 |     const something = 1;
3 |
4 |     return something;
5 | }

Is line 3 marked as uncovered or excluded completely?

Maybe better approach would be to exclude all lines that are not present in source maps.

@anthony-redFox
Copy link
Author

It will be excluded completely, so that mean you have 4 lines of code in report. (HTML report will marks as gray).

image

If you add more empty lines or comments then it is not effect on coverage report %

Copy link
Contributor

@AriPerkkio AriPerkkio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detecting lines for exclusion with regular expression will be really difficult. There are so many ways to write Javascript. For example patterns below are not detected by the current implementation:

/* Line not excluded */ console.log()

/** Line not excluded

 * Line not excluded

 */ // Line not excluded

/* Line not excluded

Line not excluded

*/

console.log(`
// This line is excluded ${console.log("even though there's code running here!")}
`)

I still think it would be better to exclude lines based on whether they are present in source maps or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants