Skip to content

Commit c3c3c0c

Browse files
author
Alec Gibson
committed
Improve test coverage for SnapshotRequest.onConnectionStateChanged
This change improves `SnapshotRequest` test coverage to 100%. Our current test for checking that a dropped connection works didn't actually fail properly when removing `this.sent = false` from `SnapshotRequest._onConnectionStateChanged`, so an improved test has been written that use middleware to wait until the request has definitely been sent, before dropping the connection and reconnecting. It also adds a previously uncovered test case for checking that the connection cannot send the same request multiple times, by using a similar setup to the rewritten dropped connection case. This change also adds some documentation recording why we might want to reset `this.sent = false`, because just looking at the code it's unclear why we would ever want to do that.
1 parent 0249c91 commit c3c3c0c

File tree

2 files changed

+32
-8
lines changed

2 files changed

+32
-8
lines changed

lib/client/snapshot-request.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,12 @@ SnapshotRequest.prototype.send = function () {
5656
};
5757

5858
SnapshotRequest.prototype._onConnectionStateChanged = function () {
59-
if (this.connection.canSend && !this.sent) {
60-
this.send();
61-
} else if (!this.connection.canSend) {
59+
if (this.connection.canSend) {
60+
if (!this.sent) this.send();
61+
} else {
62+
// If the connection can't send, then we've had a disconnection, and even if we've already sent
63+
// the request previously, we need to re-send it over this reconnected client, so reset the
64+
// sent flag to false.
6265
this.sent = false;
6366
}
6467
};

test/client/snapshot-request.js

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -214,13 +214,34 @@ describe('SnapshotRequest', function () {
214214
it('can drop its connection and reconnect, and the callback is just called once', function (done) {
215215
var connection = backend.connect();
216216

217-
connection.fetchSnapshot('books', 'don-quixote', function (error) {
218-
if (error) return done(error);
219-
done();
217+
var connectionInterrupted = false;
218+
backend.use(backend.MIDDLEWARE_ACTIONS.readSnapshots, function (request, callback) {
219+
if (!connectionInterrupted) {
220+
connection.close();
221+
backend.connect(connection);
222+
connectionInterrupted = true;
223+
}
224+
225+
callback();
226+
});
227+
228+
connection.fetchSnapshot('books', 'don-quixote', done);
229+
});
230+
231+
it('cannot send the same request twice over a connection', function (done) {
232+
var connection = backend.connect();
233+
234+
var hasResent = false;
235+
backend.use(backend.MIDDLEWARE_ACTIONS.readSnapshots, function (request, callback) {
236+
if (!hasResent) {
237+
connection._snapshotRequests[1]._onConnectionStateChanged();
238+
hasResent = true;
239+
}
240+
241+
callback();
220242
});
221243

222-
connection.close();
223-
backend.connect(connection);
244+
connection.fetchSnapshot('books', 'don-quixote', done);
224245
});
225246

226247
describe('readSnapshots middleware', function () {

0 commit comments

Comments
 (0)