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.

Sign up for GitHub

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

Jump to bottom

Cleaning up after make coverage #15214

Closed
ssbrewster opened this issue Sep 6, 2017 · 4 comments
Closed

Cleaning up after make coverage #15214

ssbrewster opened this issue Sep 6, 2017 · 4 comments
Labels
build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory.

Comments

@ssbrewster
Copy link
Contributor

ssbrewster commented Sep 6, 2017 •
edited
Loading

make coverage has a number of side effects including downloading tools to generate the coverage and overwriting the lib/ directory; make coverage-clean allows the user to clean these up.

While working on #15190 I suggested passing an option to allow make coverage and make coverage-clean to be done in one run but leaving the coverage/ dir intact (and adding it to .gitignore. @TimothyGu agreed this could be good and welcomed a PR for it (and added the correct syntax for the command line option).

I think there are a couple of options for how this could be done (plus leave as is) that would be great to get feedback on. I think the options are:

  1. Pass CLEAN=true to make coverage that runs coverage-clean after coverage-test
  2. Run coverage-clean after coverage-test as default and allow the user to pass NOCLEAN=true or CLEAN=false if they do not want the side-effects of coverage removed.
  3. Leave as-is and require the user to run coverage-clean after coverage

Both options 1 and 2 would leave the coverage/ dir intact (so not rm -r that directory during coverage-clean as is done currently), and add that dir to .gitignore. I'm happy for the coverage/ dir to remain intact because I might want to keep referencing those files (as long as they are ignored by git) but do not want all the other side-effects showing up each time I run git status.

Any feedback is really appreciated.
@ssbrewster ssbrewster changed the title Cleaning up after make coverage Cleaning up after make coverage Sep 6, 2017
@ssbrewster ssbrewster changed the title Cleaning up after make coverage Cleaning up after make coverage Sep 6, 2017
@mscdex mscdex added tools Issues and PRs related to the tools directory. discuss Issues opened for discussions and feedbacks. build Issues and PRs related to build files or the CI. labels Sep 6, 2017
@apapirovski
Copy link
Member

Either 1 or 3 would be nice, I think. And I wholeheartedly agree re: keeping coverage dir intact and adding it to .gitignore. Not sure if there are any reasons not to.

@ssbrewster
Copy link
Contributor Author

Thanks @apapirovski. Yes I can understand the principle of coverage-clean cleaning up all coverage files but the coverage reports themselves are a useful reference.

@ssbrewster
Copy link
Contributor Author

@TimothyGu any thoughts about this?

@ssbrewster ssbrewster mentioned this issue Sep 19, 2017
Closed
1 task
ssbrewster added a commit to ssbrewster/node that referenced this issue Sep 19, 2017
@ssbrewster
test: allow setting clean as option for coverage
5636f0c 
Allow running make coverage with CLEAN=true so that the coverage reports
can be generated and cleaned up afterwards (right now this is a
workaround but with the drawback that this creates another make process

Adjust coverage-clean so that it only removes the coverage/ dir if
CLEAN=false otherwise leave it intact

Update .gitignore to ignore the coverage/ dir

Refs: https://github.com/nodejs/node/pull/15190/files#r136934721
Fixes: nodejs#15214
@Trott Trott removed the discuss Issues opened for discussions and feedbacks. label Mar 11, 2018
@apapirovski
Copy link
Member

This was addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@apapirovski @mscdex @Trott @ssbrewster