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

tests(crypto): Increase coverage and mark private methods #1950

Merged
merged 7 commits into from Jan 8, 2019

Conversation

Projects
None yet
4 participants
@JeremiGendron
Copy link
Contributor

JeremiGendron commented Jan 6, 2019

Proposed changes

  • document validate rules param correct types (Object|Function|String)
  • __validateWith<> methods are now private
  • tests for __validateWith<> changed to validate("...", <>)
  • increase coverage for errors() function
  • modify validate to check for "typeof rules === 'string'" (more below)

Refs: #1782 #1775

The validate function was checking for rules instanceof String, which never evaluates to true for a string literal. Changed to typeof rules === 'string'

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (improve a current implementation without adding a new feature or fixing a bug)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Build (changes that affect the build system)
  • Docs (documentation only changes)
  • Test (adding missing tests or fixing existing tests)
  • Other... Please describe:

Checklist

  • I have read the CONTRIBUTING documentation
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

JeremiGendron added some commits Jan 6, 2019

tests(crypto): Increase coverage and mark private methods
 - __validateWith<> methods are now private
 - tests for __validateWith<> changed to validate("...", <>)
 - increase coverage for errors() function
 - modify validate to check for "typeof rules === 'string'" (more below)

Refs: #1782 #1775
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 6, 2019

Codecov Report

Merging #1950 into develop will increase coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1950      +/-   ##
===========================================
+ Coverage    38.49%   38.62%   +0.12%     
===========================================
  Files          353      353              
  Lines         7747     7747              
  Branches      1141     1155      +14     
===========================================
+ Hits          2982     2992      +10     
+ Misses        4752     4741      -11     
- Partials        13       14       +1
Impacted Files Coverage Δ
packages/crypto/src/validation/validator.ts 100% <100%> (+35.71%) ⬆️
packages/core-logger-winston/src/formatter.ts 38.88% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0634da...34372b1. Read the comment docs.

@faustbrian faustbrian self-requested a review Jan 7, 2019

Show resolved Hide resolved packages/crypto/src/validation/validator.ts Outdated
Show resolved Hide resolved packages/crypto/src/validation/validator.ts Outdated
Show resolved Hide resolved packages/crypto/src/validation/validator.ts Outdated
Show resolved Hide resolved packages/crypto/src/validation/validator.ts Outdated
@JeremiGendron

This comment has been minimized.

Copy link
Contributor

JeremiGendron commented Jan 7, 2019

@faustbrian thank you

@faustbrian

This comment has been minimized.

Copy link
Collaborator

faustbrian commented Jan 7, 2019

The core-api changes should not be included in this PR.

@JeremiGendron

This comment has been minimized.

Copy link
Contributor

JeremiGendron commented Jan 7, 2019

@faustbrian not sure if git revert 0abbee01 was the right move

faustbrian added some commits Jan 7, 2019

@faustbrian faustbrian merged commit 69421d7 into ArkEcosystem:develop Jan 8, 2019

6 checks passed

ci/circleci: test-node10-0 Your tests passed on CircleCI!
Details
ci/circleci: test-node10-1 Your tests passed on CircleCI!
Details
ci/circleci: test-node10-2 Your tests passed on CircleCI!
Details
ci/circleci: test-node11-0 Your tests passed on CircleCI!
Details
ci/circleci: test-node11-1 Your tests passed on CircleCI!
Details
ci/circleci: test-node11-2 Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment