Skip to content
This repository has been archived by the owner on Dec 30, 2019. It is now read-only.

Upgrade to test on node 10 #114

Merged
merged 3 commits into from
Jan 2, 2019
Merged

Upgrade to test on node 10 #114

merged 3 commits into from
Jan 2, 2019

Conversation

brianc
Copy link
Owner

@brianc brianc commented Dec 14, 2018

  • Upgrade travis to test with node 10 instead of node 9.
  • Upgrade some dev dependencies which npm identified had security vulnerabilities. Not a huge deal since they're dev deps, but it's good hygiene.
  • check in package-lock.json generated by newer version of npm.
  • consume from socket in timeout tests or the server will never close them, and the tests will hang.
  • fix a test which was using both done and returning a promise.
  • remove --no-exit from mocha, which ironically I originally added many moons ago but has now become the default behavior.

@brianc
Copy link
Owner Author

brianc commented Dec 14, 2018

apparently error.errno and error.code are platform specific for socket errors? The tests pass for me on os x but fail in travis on ubuntu? 🤷‍♂️

@brianc brianc requested a review from charmander December 14, 2018 21:11
@@ -154,11 +154,15 @@ describe('pool error handling', function () {
const pool = new Pool({ max: 1, port: closeServer.address().port })
pool.connect((err) => {
expect(err).to.be.an(Error)
expect(err.message).to.be('Connection terminated unexpectedly')
if (err.errno) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is allowing any error that doesn’t have an errno to pass the test okay?

@brianc
Copy link
Owner Author

brianc commented Dec 15, 2018 via email

@charmander
Copy link
Collaborator

It’s probably fine to leave until the problematic version of Node goes EOL. :)

@brianc brianc merged commit 4d2ad36 into master Jan 2, 2019
@brianc brianc deleted the bmc/upgrade-for-node-10 branch January 2, 2019 18:11
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.

2 participants