test(cursor): ensure that we properly kill cursors #1610
Conversation
daprahamian
force-pushed
the
NODE-1214
branch
from
a000b16 to
15f7fa6
Compare
| const cursor = collection.find({}); | ||
|
|
||
| // Iterate cursor past first element | ||
| return cursor |
There was a problem hiding this comment.
I get keeping the promise chain one level deeper so you can maintain access to the client variable, but it looks like right about here we can keep the chain just at the level of the collection.insert(docs) above - starting to look a little callback hellish :)
There was a problem hiding this comment.
oh I take it back, you're using the cursor as well, disregard
| let cleanup = () => {}; | ||
| let caughtError = undefined; | ||
|
|
||
| return ( |
There was a problem hiding this comment.
what's the intent in wrapping all of it in parentheses?
There was a problem hiding this comment.
prettier did that automatically :(
test/functional/cursor_tests.js
Outdated
|
|
||
| // Kill cursor | ||
| return new Promise((resolve, reject) => { | ||
| cursor.kill((err, response) => { |
There was a problem hiding this comment.
nitpick, but can't we make this very short? cursor.kill((err, r) => err ? reject(err) : resolve(r))
test/functional/cursor_tests.js
Outdated
| .then(() => { | ||
| // Confirm that cursorId is non-zero | ||
| let id = cursor.cursorState.cursorId; | ||
| if (typeof id !== 'number') { |
There was a problem hiding this comment.
did you find a case where the id is not a Long? Seems like maybe we can cut this extra code?
Added a test to make sure we properly kill cursors Fixes NODE-1214
daprahamian
force-pushed
the
NODE-1214
branch
from
15f7fa6 to
46b749d
Compare
Added a test to make sure we properly kill cursors
Fixes NODE-1214