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

Enable JS branch coverage #2545

Merged
merged 8 commits into from Jan 3, 2018

Conversation

Projects
None yet
3 participants
@brianhelba
Copy link
Member

commented Dec 21, 2017

This makes two significant changes:

  • Javascript branch coverage is enabled, showing lines that may only be partially covered.
    • This causes the overall coverage percentage to decrease, but it is a (hopefully) more accurate reflection of which Javascript branch conditions are actually getting executed during test.
  • Codecov will now report separate metrics for server, python-client, and web-client coverage.
    • These are fundamentally separate components, so conflating them under one coverage metric is misleading.

The full list of commits are:

  • Use nyc instead of istanbul to report coverage
  • Report Javascript branch coverage
  • Capture Javascript HTML coverage report as a CircleCI artifact
  • Disable Codecov patch status
  • Split Codecov reports into separate values for different components
  • Don't cause test failures when coverage is too low
    • Coverage being too low should not trigger test-time failures, as these cause CircleCIs workflow steps to fail and subsequent operations to not run. Coverage threshold failures will be reported to the PR via Codecov, which will still block merging.
  • Add PR #2543 to the changelog
  • Move a Python coverage setting to config file

Fixes #2539

@brianhelba brianhelba force-pushed the js-branch-coverage branch from 2e98ca9 to e9b92f5 Jan 3, 2018

@brianhelba brianhelba force-pushed the js-branch-coverage branch from e9b92f5 to 465fe66 Jan 3, 2018

brianhelba added some commits Jan 3, 2018

Don't cause test failures when coverage is too low
Coverage being too low should not trigger test-time failures, as these
cause CircleCIs workflow steps to fail and subsequent operations to not
run. Coverage threshold failures will be reported to the PR via Codecov,
which will still block merging.

@brianhelba brianhelba force-pushed the js-branch-coverage branch from 465fe66 to 90a5de6 Jan 3, 2018

@brianhelba brianhelba changed the title WIP: Enable JS branch coverage Enable JS branch coverage Jan 3, 2018

@brianhelba brianhelba added domain: test and removed status: WIP labels Jan 3, 2018

@brianhelba brianhelba requested review from manthey and jbeezley Jan 3, 2018

@brianhelba

This comment has been minimized.

Copy link
Member Author

commented Jan 3, 2018

project:
default:
threshold: 1

This comment has been minimized.

Copy link
@jbeezley

jbeezley Jan 3, 2018

Member

You might keep the threshold option. Our javascript coverage in particular tends to vary from run to run due to async timing.

This comment has been minimized.

Copy link
@brianhelba

brianhelba Jan 3, 2018

Author Member

This is obsoleted by the target option. The check now passes if the coverage is above the threshold (which means we can insist on it always passing).

@brianhelba brianhelba merged commit 39875ef into master Jan 3, 2018

11 checks passed

ci/circleci: py2_coverage Your tests passed on CircleCI!
Details
ci/circleci: py2_integrationTests Your tests passed on CircleCI!
Details
ci/circleci: py2_serverInstall_serverTest Your tests passed on CircleCI!
Details
ci/circleci: py3_coverage Your tests passed on CircleCI!
Details
ci/circleci: py3_integrationTests Your tests passed on CircleCI!
Details
ci/circleci: py3_serverInstall Your tests passed on CircleCI!
Details
ci/circleci: py3_serverTest Your tests passed on CircleCI!
Details
ci/circleci: py3_webBuild_webTest Your tests passed on CircleCI!
Details
codecov/project/python_client 85.1% (target 80%)
Details
codecov/project/server 88.35% (target 85%)
Details
codecov/project/web_client 82.68% (target 80%)
Details

@brianhelba brianhelba deleted the js-branch-coverage branch Jan 3, 2018

@jeffbaumes jeffbaumes restored the js-branch-coverage branch Jan 5, 2018

@jeffbaumes jeffbaumes deleted the js-branch-coverage branch Jan 5, 2018

@danlamanna

This comment has been minimized.

Copy link
Member

commented Jan 10, 2018

Introducing nyc as a dependency has caused sporadic issues with the base vagrant up command for Girder. There appear to be some issues with how it does file locking during build, see Hashnode/mern-cli#16 (comment).

@brianhelba

This comment has been minimized.

Copy link
Member Author

commented Jan 10, 2018

@danlamanna Can you create an issue for this? It sounds like an npm issue with installing packages under Vagrant that happens to affect NYC more often than others?

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