test: refactor coverage logic by bcoe · Pull Request #35767 · nodejs/node

bcoe · 2020-10-23T03:09:41Z

Cleanup logic in Makefile for coverage. Update BUILDING.md accordingly.

The coverage logic in the Makefile had become a bit crufty:

it had logic specific to nyc, the coverage tool we were using prior to c8.
the instructions for collecting coverage reports locally were more complex than necessary (given improvements we've made).

CC: @nodejs/testing

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Cleanup logic in Makefile for coverage. Update BUILDING.md accordingly.

bcoe · 2020-10-23T03:10:40Z

BUILDING.md

+
+```text
+$ make coverage-clean
+$ NODE_V8_COVERAGE=coverage/tmp python tools/test.py test/parallel/test-stream2-transform.js


You can just set NODE_V8_COVERAGE, and then run tests any which way.

bcoe · 2020-10-23T03:11:35Z

Makefile

-	$(RM) out/$(BUILDTYPE)/obj.target/embedtest/src/*.gcno
-	$(RM) out/$(BUILDTYPE)/obj.target/embedtest/test/embedding/*.gcno
+	$(RM) -r coverage/tmp
+	$(FIND) out/$(BUILDTYPE)/obj.target -name "*.gcda" -type f -delete


I was having trouble on OSX with left over gcno and gcda files, I believe it's safe to simply remove all files generated by gcov.

bcoe · 2020-10-23T03:11:57Z

Makefile

 		| sed 's/<[^>]*>//g'| sed 's/ //g'

-COV_REPORT_OPTIONS = --reporter=html \
-	--temp-directory=out/$(BUILDTYPE)/.coverage --omit-relative=false \


these options are encapsulated in the .nycrc file.

codecov-io · 2020-10-23T04:20:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.91%. Comparing base (f44029e) to head (cd901bb).

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #35767    +/-   ##
========================================
  Coverage   87.90%   87.91%            
========================================
  Files         477      477            
  Lines      113172   113090    -82     
  Branches    25428    24628   -800     
========================================
- Hits        99480    99419    -61     
+ Misses       7991     7956    -35     
- Partials     5701     5715    +14

see 89 files with indirect coverage changes

BUILDING.md

Makefile

Trott

LGTM with or without my suggestions/comments addressed

Co-authored-by: Rich Trott <[email protected]>

Trott · 2020-10-23T16:12:38Z

@nodejs/build-files

Makefile

Co-authored-by: Rich Trott <[email protected]>

Trott · 2020-10-25T00:27:23Z

It sure seems as if the Raspberry Pi build failures are related to the Makefile changes here, but I'm not sure how?

nodejs-github-bot · 2020-10-25T00:29:17Z

CI: https://ci.nodejs.org/job/node-test-pull-request/33848/

Trott · 2020-10-25T00:29:59Z

In the same vein as "quit and restart the program to see if the problem goes away", I've kicked off a CI with Rebuild rather than Resume Build to see if that fixes it....

bcoe · 2020-10-25T02:38:23Z

It sure seems as if the Raspberry Pi build failures are related to the Makefile changes here, but I'm not sure how?

@Trott odd, I don't think any of the coverage rules should be being executed right?

nodejs-github-bot · 2020-10-25T02:39:04Z

CI: https://ci.nodejs.org/job/node-test-pull-request/33849/ ✅

bcoe · 2020-10-25T02:51:04Z

@Trott restarting worked, but I'm slightly concerned it took so many restarts?

Trott · 2020-10-25T13:19:09Z

@Trott restarting worked, but I'm slightly concerned it took so many restarts?

Perhaps the other times were all Resume Builds and not Rebuilds and that is why the results were the same?

Cleanup logic in Makefile for coverage. Update BUILDING.md accordingly. PR-URL: #35767 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

bcoe · 2020-10-25T14:24:15Z

Landed in ba907ff

Cleanup logic in Makefile for coverage. Update BUILDING.md accordingly. PR-URL: #35767 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

test: refactor coverage logic

5877a15

Cleanup logic in Makefile for coverage. Update BUILDING.md accordingly.

bcoe requested review from Trott, addaleax and mhdawson October 23, 2020 03:09

nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. labels Oct 23, 2020

bcoe commented Oct 23, 2020

View reviewed changes

Trott reviewed Oct 23, 2020

View reviewed changes

BUILDING.md Outdated Show resolved Hide resolved

Trott reviewed Oct 23, 2020

View reviewed changes

Makefile Outdated Show resolved Hide resolved

Trott approved these changes Oct 23, 2020

View reviewed changes

nodejs deleted a comment Oct 23, 2020

Benjamin E. Coe and others added 2 commits October 23, 2020 09:59

Apply suggestions from code review

31359a5

Co-authored-by: Rich Trott <[email protected]>

chore: address code review

025a363

This comment has been minimized.

Sign in to view

bcoe added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 23, 2020

Trott reviewed Oct 23, 2020

View reviewed changes

Makefile Outdated Show resolved Hide resolved

Update Makefile

cd901bb

Co-authored-by: Rich Trott <[email protected]>

Trott approved these changes Oct 23, 2020

View reviewed changes

This comment has been minimized.

Sign in to view

joyeecheung approved these changes Oct 25, 2020

View reviewed changes

bcoe pushed a commit that referenced this pull request Oct 25, 2020

test: refactor coverage logic

ba907ff

Cleanup logic in Makefile for coverage. Update BUILDING.md accordingly. PR-URL: #35767 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

bcoe closed this Oct 25, 2020

bcoe deleted the coverage-refactor branch October 25, 2020 14:24

targos pushed a commit that referenced this pull request Nov 3, 2020

test: refactor coverage logic

110ef8a

Cleanup logic in Makefile for coverage. Update BUILDING.md accordingly. PR-URL: #35767 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

targos mentioned this pull request Nov 3, 2020

v15.1.0 release proposal #35948

Merged

richardlau mentioned this pull request Nov 20, 2020

403 Forbidden for the most recent coverage reports #36040

Closed

BethGriggs pushed a commit that referenced this pull request Dec 8, 2020

test: refactor coverage logic

92da13f

Cleanup logic in Makefile for coverage. Update BUILDING.md accordingly. PR-URL: #35767 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

BethGriggs pushed a commit that referenced this pull request Dec 10, 2020

test: refactor coverage logic

22e8076

Cleanup logic in Makefile for coverage. Update BUILDING.md accordingly. PR-URL: #35767 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

BethGriggs mentioned this pull request Dec 10, 2020

v14.15.2 proposal #36476

Merged

targos pushed a commit that referenced this pull request Mar 3, 2021

test: refactor coverage logic

c5a61d3

Cleanup logic in Makefile for coverage. Update BUILDING.md accordingly. PR-URL: #35767 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

MylesBorins pushed a commit that referenced this pull request Apr 6, 2021

test: refactor coverage logic

48e3f59

Cleanup logic in Makefile for coverage. Update BUILDING.md accordingly. PR-URL: #35767 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>

danielleadams mentioned this pull request May 3, 2021

v14.17.0 proposal #38507

Merged

Uh oh!

Conversation

bcoe commented Oct 23, 2020

Checklist

Uh oh!

bcoe Oct 23, 2020

Choose a reason for hiding this comment

Uh oh!

bcoe Oct 23, 2020

Choose a reason for hiding this comment

Uh oh!

bcoe Oct 23, 2020

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Oct 23, 2020 • edited by codecov bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Trott left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Trott commented Oct 23, 2020

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Trott commented Oct 25, 2020

Uh oh!

nodejs-github-bot commented Oct 25, 2020

Uh oh!

Trott commented Oct 25, 2020

Uh oh!

bcoe commented Oct 25, 2020

Uh oh!

nodejs-github-bot commented Oct 25, 2020 • edited by bcoe Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bcoe commented Oct 25, 2020

Uh oh!

Trott commented Oct 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bcoe commented Oct 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-io commented Oct 23, 2020 •

edited by codecov bot

Loading

nodejs-github-bot commented Oct 25, 2020 •

edited by bcoe

Loading

Trott commented Oct 25, 2020 •

edited

Loading