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

Fix tests (#18) and mocking NCBI API #21

Merged
merged 8 commits into from Apr 9, 2017

Conversation

2 participants
@bmpvieira
Member

bmpvieira commented Apr 6, 2017

Tests should no longer break often due to NCBI changes and we can once in a while update the mock data automatically to make sure our code is still relevant.

bmpvieira added some commits Apr 6, 2017

Add tape-nock module to tests
This allows to record http request response into fixtures files
and later use those to mock the external API (NCBI) so that tests
don't break all the time due to NCBI side changes rather than ours

@bmpvieira bmpvieira requested a review from thejmazz Apr 6, 2017

@bmpvieira bmpvieira self-assigned this Apr 6, 2017

@bmpvieira bmpvieira added this to In Progress in Bionode Project Board Apr 6, 2017

@bmpvieira bmpvieira added the bug label Apr 7, 2017

bmpvieira added some commits Apr 7, 2017

@bmpvieira bmpvieira requested review from IsmailM and removed request for thejmazz Apr 7, 2017

cli.js Outdated
@@ -46,8 +46,8 @@ if (command === 'link') {
var arg2 = argv._[2]
var arg3 = argv._[3]
} else {
var arg2 = argv._.slice(2).join(' ')
var arg3 = null
arg2 = argv._.slice(2).join(' ')

This comment has been minimized.

@IsmailM

IsmailM Apr 9, 2017

Member

These variables aren't declared if command !== 'link'

This comment has been minimized.

@bmpvieira

bmpvieira Apr 9, 2017

Member

They are declared because JavaScript doesn't have block scope, only function scope, so they get hoisted. However, this might be confusing so I'll fix it. Thanks for noticing it!

This comment has been minimized.

@IsmailM
@IsmailM

IsmailM approved these changes Apr 9, 2017

@bmpvieira bmpvieira merged commit 096fec8 into master Apr 9, 2017

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+64.2%) to 81.791%
Details
security/snyk No new vulnerabilities
Details

@bmpvieira bmpvieira deleted the development branch Apr 9, 2017

@bmpvieira bmpvieira moved this from In Progress to Done in Bionode Project Board Apr 9, 2017

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