Skip to content

build,test: fail coverage target if tests fail - #25432

Closed
refack wants to merge 1 commit into
nodejs:masterfrom
refack:race-safe-coverage
Closed

build,test: fail coverage target if tests fail#25432
refack wants to merge 1 commit into
nodejs:masterfrom
refack:race-safe-coverage

Conversation

@refack

Copy link
Copy Markdown
Contributor

Part of fix for #24966
Make the coverage-test target fail if running the tests fail. This way we get early warning that something is borked and we don't publish spurious results.

/CC @nodejs/build-files @nodejs/benchmarking @nodejs/testing

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-botnodejs-github-bot added the build Issues and PRs related to build files or the CI. label Jan 10, 2019
@refackrefack added the coverage Issues and PRs related to native coverage support. label Jan 10, 2019

@mhdawsonmhdawson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM we can easily back out if this does not agree with the coverage CI.

@danbev

Copy link
Copy Markdown
Contributor

Landed in f216d5b.

@danbevdanbev closed this Jan 15, 2019
pullBot pushed a commit to shakir-abdo/node that referenced this pull request Jan 15, 2019
PR-URL: nodejs#25432
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
addaleax pushed a commit that referenced this pull request Jan 15, 2019
PR-URL: #25432
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@refack
refack deleted the race-safe-coverage branch January 15, 2019 21:02
@BridgeARBridgeAR mentioned this pull request Jan 16, 2019
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
PR-URL: nodejs#25432
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@BridgeAR

Copy link
Copy Markdown
Member

Due to the recent failures and the request not to add this to the v11.7.0 release I marked this as don't land.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.coverageIssues and PRs related to native coverage support.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@refack@nodejs-github-bot@danbev@BridgeAR@lpinca@mhdawson@targos