refactor(ChangeStream): use maybePromise for close, improve tests #2343
Conversation
emadum
changed the title
| @@ -2609,34 +2619,24 @@ describe('Change Streams', function() { | |||
| describe('tryNext', function() { | |||
There was a problem hiding this comment.
I oddly started getting an error in the tryNext tests:
MongoError: cannot open $changeStream for non-existent database: test
Possibly related to deleting my test_data and switching mongo versions, then running that describe block with .only.
I thought it would be more robust to use a specifically named database and drop it after each test, to ensure weird behavior doesn't carry over between tests.
To that purpose I created the withTempDb helper that wraps withClient to create and drop the specified database. This refactor made above error go away.
emadum
changed the title
| @@ -37,7 +38,11 @@ function triggerResumableError(changeStream, onCursorClosed) { | |||
| * @param {function} callback | |||
| */ | |||
| function waitForStarted(changeStream, callback) { | |||
| const timeout = setTimeout(() => { | |||
| throw new Error('Change stream never started'); | |||
| }, 2000); | |||
There was a problem hiding this comment.
This shouldn't do anything. I'm just concerned some tests are timing out after 60s because for some reason waitForStarted never triggers. If that's the case, this will at least fail a lot sooner, and more importantly we'll know for sure whether this is the root cause of test flakiness.
| // if the arity is 1 then this a callback for `more` | ||
| if (arguments.length === 1) { | ||
| if (err instanceof Error) { |
There was a problem hiding this comment.
in what case is an error returned here? When I look at where more is emitted in change_stream.js, the errors are always handled. This would seem to imply the getMore callback was called with err, err or something.
There was a problem hiding this comment.
Oops I think I should remove this now, I was just trying to track down an issue but it's no longer necessary.
Description
NODE-2598
What changed?
closeto usemaybePromisehelpermid-closetests by delaying the final writewaitForStartedhelper to tests where appropriate to reduce flakinessAre there any files to ignore?