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

Test: Add Completion statement #87

Merged
merged 3 commits into from
Jan 3, 2019
Merged

Test: Add Completion statement #87

merged 3 commits into from
Jan 3, 2019

Conversation

Andrew-Lees11
Copy link
Contributor

Description

Added a completion statement to dropDatabaseIfExists so that if there isn't a database it will still call the next function and continue the tests.

Also changed the default database name in the tests to the suggested local database to help when testing locally.

Copy link
Contributor

@djones6 djones6 left a comment

Choose a reason for hiding this comment

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

This fix looks good, and has uncovered test failures which will need resolving separately. I'd vote for merging this as the tests should have been failing before.

@djones6
Copy link
Contributor

djones6 commented Dec 18, 2018

@Andrew-Lees11 after talking with @ianpartridge we'd like to disable the tests that are failing (the Kitura-NIO builds) and merge this PR. #91 needs to be investigated and the a PR raised to fix the tests (or to re-enable the tests once Kitura-NIO is fixed).

@djones6
Copy link
Contributor

djones6 commented Dec 18, 2018

@Andrew-Lees11 I may have fixed this in Kitura/Kitura-NIO#116 (which has now been tagged). I've restarted the failing CI.

Edit: we're past that failure, but have uncovered a new one. I've updated #91 accordingly.

@Andrew-Lees11 Andrew-Lees11 merged commit cc8ef60 into master Jan 3, 2019
@djones6 djones6 deleted the completionCall branch January 4, 2019 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants