test: improve shared helpers #2362
Conversation
reggi
left a comment
reggi
left a comment
There was a problem hiding this comment.
LGTM, Makes testing easier +++++
@reggi Made some more changes after you reviewed - re-requesting. Might be easiest to just check the new commits. Sorry about that! |
mbroadst
left a comment
mbroadst
left a comment
There was a problem hiding this comment.
one little nit you can take or leave, but otherwise LGTM! Great refactor on withClient here, I think it's really evolving into something we'd want to use everywhere
test/functional/index.test.js
Outdated
| const withClient = require('./shared').withClient; | ||
| const withMonitoredClient = require('./shared').withMonitoredClient; | ||
| const shared = require('./shared'); | ||
| const { withClient, withMonitoredClient } = shared; |
There was a problem hiding this comment.
if we're destructuring anyway it looks like just about all of these come from shared:
const {
withClient,
withMonitoredClient,
assert,
setupDatabase
} = require('./shared');
There was a problem hiding this comment.
@mbroadst I updated a few things since your review, can you take a look when you get a chance? Refactoring withMonitoredClient exposed an issue in withClient, where it was swallowing errors returned by the done(err) callback in tests. I've updated withClient to now throw those errors after cleanup.
Description
NODE-2605
What changed?
withTempDbclientargument ofwithClientoptionalmakeCleanupFnintowithClientconnectToDbhelperfilterForCommands/filterOutCommands/withMonitoredClientshared.jsAlso introduced some CI-related cleanup:
eslintversion before lintingnvmand use--no-progressoption to clean up log outputAre there any files to ignore?