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

Can exclude files from coverage #811

Merged
merged 1 commit into from Feb 13, 2018

Conversation

@geek
Copy link
Member

geek commented Feb 12, 2018

No description provided.

@geek geek added the feature label Feb 12, 2018
@geek geek self-assigned this Feb 12, 2018
@geek geek requested a review from cjihrig Feb 12, 2018
isFile = pathStat.isFile();
}
// ok to swallow the error as these can be sub folders, or not even exist (which is ok)
catch (ex) {}

This comment has been minimized.

Copy link
@cjihrig

cjihrig Feb 13, 2018

Contributor

Maybe check if ex is an expected/acceptable error before ignore it?

isFile,
escaped: internals.escape(path)
};
}) : '';

This comment has been minimized.

Copy link
@cjihrig

cjihrig Feb 13, 2018

Contributor

It might just be me, but the large map() function combined with the ternary makes this a bit harder to read.

escaped: internals.escape(path)
};
}) : '';
excludes = excludes ? excludes.map((exclude) => {

This comment has been minimized.

Copy link
@cjihrig

cjihrig Feb 13, 2018

Contributor

It looks like excludes will either be an empty string or array at this point. If so, couldn't you move this logic up into the previous map()?

@geek geek force-pushed the geek:cover_exclude branch from 7e17231 to 1dec7a1 Feb 13, 2018
@geek

This comment has been minimized.

Copy link
Member Author

geek commented Feb 13, 2018

@cjihrig fixed up

@geek geek added this to the 15.3.0 milestone Feb 13, 2018
@geek geek merged commit a4764bc into hapijs:master Feb 13, 2018
2 checks passed
2 checks passed
Node Security No known vulnerabilities found
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@geek geek deleted the geek:cover_exclude branch Feb 13, 2018
@danielo515

This comment has been minimized.

Copy link

danielo515 commented Aug 27, 2018

How is this supposed to work ? Does it work with globs ?

@geek

This comment has been minimized.

Copy link
Member Author

geek commented Aug 27, 2018

@danielo515 you are able to exclude specific files or you can exclude an entire directory. The feature adds a --coverage-exclude CLI argument that you can provide multiple times for excluding many files/directories:

lab --coverage-exclude ignore.js --coverage-exclude something/else.js --coverage-exclude excludedir
@danielo515

This comment has been minimized.

Copy link

danielo515 commented Aug 30, 2018

Thabks for the clarification @geek.
Have you considered adding support for wildcard or at least regular expressions?

@geek

This comment has been minimized.

Copy link
Member Author

geek commented Aug 30, 2018

@danielo515 At the moment I've been fine with simply listing the folders/files to exclude directly. Is there a case when this won't work?

@danielo515

This comment has been minimized.

Copy link

danielo515 commented Aug 31, 2018

Thanks for keeping the conversation open @geek ,

As I explained on my issue #853 I have my test files along with the files being tested. This is quite convenient for visibility and easy of file requiring. The problem is that test files themseves are also being checked for coverage.
This should not be a problem normally because test are all executed, but in the case I exposed for some weird reason the coverage is not properly detected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.