Skip to content

Commit 74d72ce

Browse files
authored
Fix flaky test. (#1511)
The "can return previous value through consecutive updates" test was flaky because it did two consecutive server timestamp writes to the same field and expected 2 local events. But if you do both writes within 1ms then the snapshots will be indistinguishable (the estimated value for the server timestamps will be the same) and so only 1 event will be raised. I added an additional change to the second write to guarantee it triggers an event. While looking at the test, I also noticed it was slightly broken / incomplete. There was an incorrect line attempting to disable the network and the test was missing the remaining code to re-enable the network and verify the behavior. I fixed that as well.
1 parent d0f981e commit 74d72ce

File tree

1 file changed

+14
-3
lines changed

1 file changed

+14
-3
lines changed

packages/firestore/test/integration/api/server_timestamp.test.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,11 +245,12 @@ apiDescribe('Server Timestamps', persistence => {
245245
it('can return previous value through consecutive updates', () => {
246246
return withTestSetup(() => {
247247
return writeInitialData()
248-
.then(() => docRef.firestore.disableNetwork)
248+
.then(() => docRef.firestore.disableNetwork())
249249
.then(() => {
250250
// We set up two consecutive writes with server timestamps.
251251
docRef.update('a', FieldValue.serverTimestamp());
252-
docRef.update('a', FieldValue.serverTimestamp());
252+
// include b=1 to ensure there's a change resulting in a new snapshot.
253+
docRef.update('a', FieldValue.serverTimestamp(), 'b', 1);
253254
return accumulator.awaitLocalEvents(2);
254255
})
255256
.then(snapshots => {
@@ -260,14 +261,19 @@ apiDescribe('Server Timestamps', persistence => {
260261
expect(
261262
snapshots[1].get('a', { serverTimestamps: 'previous' })
262263
).to.equal(42);
264+
return docRef.firestore.enableNetwork();
265+
})
266+
.then(() => accumulator.awaitRemoteEvent())
267+
.then(remoteSnapshot => {
268+
expect(remoteSnapshot.get('a')).to.be.an.instanceof(Timestamp);
263269
});
264270
});
265271
});
266272

267273
it('uses previous value from local mutation', () => {
268274
return withTestSetup(() => {
269275
return writeInitialData()
270-
.then(() => docRef.firestore.disableNetwork)
276+
.then(() => docRef.firestore.disableNetwork())
271277
.then(() => {
272278
// We set up three consecutive writes.
273279
docRef.update('a', FieldValue.serverTimestamp());
@@ -284,6 +290,11 @@ apiDescribe('Server Timestamps', persistence => {
284290
expect(
285291
snapshots[2].get('a', { serverTimestamps: 'previous' })
286292
).to.equal(1337);
293+
return docRef.firestore.enableNetwork();
294+
})
295+
.then(() => accumulator.awaitRemoteEvent())
296+
.then(remoteSnapshot => {
297+
expect(remoteSnapshot.get('a')).to.be.an.instanceof(Timestamp);
287298
});
288299
});
289300
});

0 commit comments

Comments
 (0)