From 76810776e7d165de0129973e5a52b0665a792cc8 Mon Sep 17 00:00:00 2001 From: Alec Gibson <12036746+alecgibson@users.noreply.github.com> Date: Wed, 15 Feb 2023 11:12:21 +0000 Subject: [PATCH] =?UTF-8?q?=E2=9C=85=20Fix=20flaky=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/share/sharedb-mongo/issues/131 This change attempts to fix some tests that are flaky in `sharedb-mongo`. The flakiness can be reproduced locally by wrapping the `Agent._querySubscribe()` [call to `_fetchBulkOps()`][1] in a long `setTimeout()`: ```js if (options.fetchOps) { wait++; setTimeout(function() { console.log('fetch bulk ops'); agent._fetchBulkOps(collection, options.fetchOps, finish); }, 1000); } ``` This forces us into an edge case where the subscribe query triggers and returns the diff from a [`queryPoll()`][2], which triggers the tests' `'insert'` handlers, finishes the test, and closes the backend, all before the `_fetchBulkOps()` call is issued (which subsequently fails because the database has been closed). Handling this query subscribe actually triggers a variety of responses to be sent to the client at different times: 1. The actual `_querySubscribe()` [callback][3] (which ultimately triggers [`agent._reply()`][4] in response to the original request) 2. Ops sent [independently][5] by [`_fetchBulkOps()`][6] 3. The diff resulting from [`queryPoll()`][2] In order to reduce flakiness, this change adds checks that the query's [`'ready'` event][7] has been called, which will happen once the resubscribe request has been replied to by the `Agent`. This ensures we've waited for events 1 & 3 of the above list, although we notably aren't waiting for event 2 (which is where the error is actually coming from). Since no ops will actually be sent to the client, I'm not sure how best to wait for this. Hopefully waiting for the subscribe ack should be sufficient. [1]: https://github.com/share/sharedb/blob/b84290f8a66ee42b3d2c75d6de780e125b99c514/lib/agent.js#L521-L524 [2]: https://github.com/share/sharedb/blob/b84290f8a66ee42b3d2c75d6de780e125b99c514/lib/agent.js#L534 [3]: https://github.com/share/sharedb/blob/b84290f8a66ee42b3d2c75d6de780e125b99c514/lib/agent.js#L511 [4]: https://github.com/share/sharedb/blob/b84290f8a66ee42b3d2c75d6de780e125b99c514/lib/agent.js#L344 [5]: https://github.com/share/sharedb/blob/b84290f8a66ee42b3d2c75d6de780e125b99c514/lib/agent.js#L265 [6]: https://github.com/share/sharedb/blob/b84290f8a66ee42b3d2c75d6de780e125b99c514/lib/agent.js#L523 [7]: https://github.com/share/sharedb/blob/b84290f8a66ee42b3d2c75d6de780e125b99c514/lib/client/query.js#L148 --- test/client/query-subscribe.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/test/client/query-subscribe.js b/test/client/query-subscribe.js index 5a331465..eef9685f 100644 --- a/test/client/query-subscribe.js +++ b/test/client/query-subscribe.js @@ -187,18 +187,25 @@ module.exports = function(options) { } ], function(err) { if (err) return done(err); + + var wait = 2; + function finish() { + if (--wait) return; + expect(util.pluck(query.results, 'id')).to.eql(['fido', 'spot', 'taco']); + done(); + } + var query = connection.createSubscribeQuery('dogs', matchAllDbQuery, null, function(err) { if (err) return done(err); connection.close(); connection2.get('dogs', 'taco').on('error', done).create({age: 2}); process.nextTick(function() { backend.connect(connection); + query.on('ready', finish); }); }); query.on('error', done); - query.on('insert', function() { - done(); - }); + query.on('insert', finish); }); }); @@ -234,11 +241,14 @@ module.exports = function(options) { ], function(error) { if (error) return done(error); backend.connect(connection); + query.once('ready', function() { + finish(); + }); }); }); }); - var wait = 2; + var wait = 3; function finish() { if (--wait) return; var results = util.sortById(query.results);