Skip to content

Commit

Permalink
✅ Fix flaky tests
Browse files Browse the repository at this point in the history
Fixes share/sharedb-mongo#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
  • Loading branch information
alecgibson committed Feb 15, 2023
1 parent b84290f commit 7681077
Showing 1 changed file with 14 additions and 4 deletions.
18 changes: 14 additions & 4 deletions test/client/query-subscribe.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 7681077

Please sign in to comment.