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

lib: remove an unnecessary coverage check #12023

Closed
wants to merge 1 commit into from
Closed

lib: remove an unnecessary coverage check #12023

wants to merge 1 commit into from

Conversation

Fishrock123
Copy link
Contributor

@Fishrock123 Fishrock123 commented Mar 24, 2017 •
edited
Loading

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
Affected core subsystem(s)

coverage

Fixes: #11445
Refs: f65a48f

Based on @addaleax's comment here.

Before (coverage.nodejs.org, master)

screen shot 2017-03-24 at 12 15 20 pm

After

screen shot 2017-03-24 at 12 15 27 pm

And the specific process._exiting lines that f65a48f was supposed to hit are indeed hit with this change:

screen shot 2017-03-24 at 12 25 52 pm

cc @addaleax, @mhdawson @CurryKitten

@Fishrock123 Fishrock123 force-pushed the coverage-remove-unnecessary-check branch from 7a1d341 to 4b7132c Compare March 24, 2017 16:30
@Fishrock123 Fishrock123 added the lib / src Issues and PRs related to general changes in the lib or src directory. label Mar 24, 2017
@fhinkel
Copy link
Member

fhinkel commented Mar 26, 2017

Landed in 59d2561

@fhinkel fhinkel closed this Mar 26, 2017
fhinkel pushed a commit that referenced this pull request Mar 26, 2017
@Fishrock123 @fhinkel
lib: remove an unnecessary coverage check
59d2561 
PR-URL: #12023
Fixes: #11445
Refs: f65a48f
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 28, 2017
@Fishrock123 @MylesBorins
lib: remove an unnecessary coverage check
837ff4b 
PR-URL: #12023
Fixes: #11445
Refs: f65a48f
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 28, 2017
Merged
@italoacasas italoacasas mentioned this pull request Apr 10, 2017
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

@fhinkel fhinkel fhinkel approved these changes

Assignees
No one assigned
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Coverage file writing hook is called too early during 'exit'.
6 participants
@Fishrock123 @fhinkel @jasnell @addaleax @cjihrig @MylesBorins