Skip to content
This repository has been archived by the owner on Jan 11, 2022. It is now read-only.

Increase test coverage #22

Merged
merged 4 commits into from
Feb 15, 2018
Merged

Increase test coverage #22

merged 4 commits into from
Feb 15, 2018

Conversation

holgerd77
Copy link
Member

Fixes #21 (partly) and #20

Copy link
Member

@axic axic left a comment

Choose a reason for hiding this comment

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

How about merging this as is and creating a new PR for other tests?

@axic
Copy link
Member

axic commented Feb 15, 2018

const getTests = exports.getTests = (testType, onFile, fileFilter = /.json$/, skipFn = () => {

It doesn't work on 4, I'm fine skipping that.

@coveralls
Copy link

coveralls commented Feb 15, 2018

Coverage Status

Coverage increased (+8.06%) to 90.323% when pulling d99f8ac on increase-test-coverage into aba5c37 on master.

@holgerd77
Copy link
Member Author

That's ok, will push asap.

@holgerd77 holgerd77 force-pushed the increase-test-coverage branch from 14754ff to 80516c4 Compare February 15, 2018 13:56
@holgerd77
Copy link
Member Author

@axic Ok, done.

Any insight on the getLength method? I know understand what it does, I have no imagination what it is used for though. Is this useful and does it make sense to include this in the API later on or should this be removed? I think it should be at least renamed to give more semantics on what it does.

So does it make sense to add tests for this respectively add the checks to existing tests?

@holgerd77 holgerd77 force-pushed the increase-test-coverage branch from 80516c4 to df43db0 Compare February 15, 2018 14:04
@holgerd77
Copy link
Member Author

@axic Had a linting error, pushed again.

@axic
Copy link
Member

axic commented Feb 15, 2018

I think it tries to extract the size needed to encode the value? Not sure.

@holgerd77 holgerd77 changed the title [WIP] Increase test coverage Increase test coverage Feb 15, 2018
@holgerd77 holgerd77 merged commit 90661f1 into master Feb 15, 2018
@holgerd77 holgerd77 deleted the increase-test-coverage branch February 15, 2018 21:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants