Skip to content

Commit

Permalink
test(firestore, e2e): de-flake more firestore tests
Browse files Browse the repository at this point in the history
one in particular was enabled and failing CI, the rest should work
now that I correctly handle callback delay and local firestore persistence
by using random collections
  • Loading branch information
mikehardy committed Dec 27, 2020
1 parent 96f17a7 commit 8916e19
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 35 deletions.
6 changes: 3 additions & 3 deletions packages/firestore/e2e/DocumentSnapshot/data.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ describe('firestore().doc() -> snapshot.data()', function () {
await ref.delete();
});

xit('handles SnapshotOptions', function () {
// TODO
});
// it('handles SnapshotOptions', function () {
// // TODO
// });

it('handles all data types', async function () {
const types = {
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/e2e/Query/endBefore.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('firestore().collection().endBefore()', function () {
qs.docs[0].id.should.eql('doc3');
});

xit('ends before snapshot field values', async function () {
it('ends before snapshot field values', async function () {
const colRef = firebase.firestore().collection(`${COLLECTION}/endBefore/snapshotFields`);
const doc1 = colRef.doc('doc1');
const doc2 = colRef.doc('doc2');
Expand Down
19 changes: 12 additions & 7 deletions packages/firestore/e2e/Query/limitToLast.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,21 @@ describe('firestore().collection().limitToLast()', function () {
should(colRef._modifiers.options.limit).equal(undefined);
});

// FIXME flaky on local tests
xit('removes limitToLast query if limit is set afterwards', function () {
const colRef = firebase.firestore().collection(COLLECTION).limitToLast(123).limit(2);
it('removes limitToLast query if limit is set afterwards', function () {
const colRef = firebase
.firestore()
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
.collection(`${COLLECTION}/${Utils.randString(12, '#aA')}/limitToLast-limit-after`);
const colRef2 = colRef.limitToLast(123).limit(2);

should(colRef._modifiers.options.limitToLast).equal(undefined);
should(colRef2._modifiers.options.limitToLast).equal(undefined);
});

// FIXME flaky on local tests
xit('limitToLast the number of documents', async function () {
const subCol = `${COLLECTION}/limitToLast/count`;
it('limitToLast the number of documents', async function () {
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
const subCol = `${COLLECTION}/${Utils.randString(12, '#aA')}/limitToLast-count`;
const colRef = firebase.firestore().collection(subCol);

// Add 3
Expand Down
20 changes: 15 additions & 5 deletions packages/firestore/e2e/Query/onSnapshot.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,12 @@ describe('firestore().collection().onSnapshot()', function () {
it('calls next with snapshot when successful', async function () {
const onNext = sinon.spy();
const onError = sinon.spy();
const unsub = firebase.firestore().collection(`${COLLECTION}/foo/bar6`).onSnapshot(
const colRef = firebase
.firestore()
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
.collection(`${COLLECTION}/${Utils.randString(12, '#aA')}/next-with-snapshot`);
const unsub = colRef.onSnapshot(
{
includeMetadataChanges: false,
},
Expand Down Expand Up @@ -297,18 +302,23 @@ describe('firestore().collection().onSnapshot()', function () {

// FIXME test disabled due to flakiness in CI E2E tests.
// Registered 4 of 3 expected calls once (!?), 3 of 2 expected calls once.
xit('unsubscribes from further updates', async function () {
it('unsubscribes from further updates', async function () {
const callback = sinon.spy();

const collection = firebase.firestore().collection(`${COLLECTION}/foo/bar7`);
const collection = firebase
.firestore()
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
.collection(`${COLLECTION}/${Utils.randString(12, '#aA')}/unsubscribe-updates`);

const unsub = collection.onSnapshot(callback);
await Utils.sleep(800);
await Utils.sleep(2000);
await collection.add({});
await collection.add({});
unsub();
await Utils.sleep(800);
await Utils.sleep(2000);
await collection.add({});
await Utils.sleep(2000);
callback.should.be.callCount(3);
});
});
18 changes: 12 additions & 6 deletions packages/firestore/e2e/Query/orderBy.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,12 @@ describe('firestore().collection().orderBy()', function () {
}
});

// FIXME flaky in local tests
xit('orders by a value ASC', async function () {
const colRef = firebase.firestore().collection(`${COLLECTION}/order/asc`);
it('orders by a value ASC', async function () {
const colRef = firebase
.firestore()
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
.collection(`${COLLECTION}/${Utils.randString(12, '#aA')}/order-asc`);

await colRef.add({ value: 1 });
await colRef.add({ value: 3 });
Expand All @@ -104,9 +107,12 @@ describe('firestore().collection().orderBy()', function () {
});
});

// FIXME flaky in local tests
xit('orders by a value DESC', async function () {
const colRef = firebase.firestore().collection(`${COLLECTION}/order/desc`);
it('orders by a value DESC', async function () {
const colRef = firebase
.firestore()
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
.collection(`${COLLECTION}/${Utils.randString(12, '#aA')}/order-desc`);

await colRef.add({ value: 1 });
await colRef.add({ value: 3 });
Expand Down
34 changes: 21 additions & 13 deletions packages/firestore/e2e/QuerySnapshot.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,21 @@ describe('firestore.QuerySnapshot', function () {

it('returns an array of DocumentChange instances', async function () {
const colRef = firebase.firestore().collection(COLLECTION);
colRef.add({});
await colRef.add({});
const snapshot = await colRef.limit(1).get();
const changes = snapshot.docChanges();
changes.should.be.Array();
changes.length.should.be.eql(1);
changes[0].constructor.name.should.eql('FirestoreDocumentChange');
});

// TODO: fixme @ehesp - flaky test: `AssertionError: expected 3 to equal 1` on line 155
xit('returns the correct number of document changes if listening to metadata changes', async function () {
it('returns the correct number of document changes if listening to metadata changes', async function () {
const callback = sinon.spy();
const colRef = firebase.firestore().collection('v6/metadatachanges/true-true');
const colRef = firebase
.firestore()
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
.collection(`${COLLECTION}/${Utils.randString(12, '#aA')}/metadatachanges-true-true`);
const unsub = colRef.onSnapshot({ includeMetadataChanges: true }, callback);
await colRef.add({ foo: 'bar' });
await Utils.spyToBeCalledTimesAsync(callback, 3);
Expand All @@ -151,10 +154,13 @@ describe('firestore.QuerySnapshot', function () {
snap3.docChanges({ includeMetadataChanges: true }).length.should.be.eql(1);
});

// TODO: fixme @ehesp - flaky test: `AssertionError: expected 5 to equal 1`
xit('returns the correct number of document changes if listening to metadata changes, but not including them in docChanges', async function () {
it('returns the correct number of document changes if listening to metadata changes, but not including them in docChanges', async function () {
const callback = sinon.spy();
const colRef = firebase.firestore().collection('v6/metadatachanges/true-false');
const colRef = firebase
.firestore()
// Firestore caches aggressively, even if you wipe the emulator, local documents are cached
// between runs, so use random collections to make sure `tests:*:test-reuse` works while iterating
.collection(`${COLLECTION}/${Utils.randString(12, '#aA')}/metadatachanges-true-false`);
const unsub = colRef.onSnapshot({ includeMetadataChanges: true }, callback);
await colRef.add({ foo: 'bar' });
await Utils.spyToBeCalledTimesAsync(callback, 3);
Expand All @@ -173,7 +179,8 @@ describe('firestore.QuerySnapshot', function () {
describe('forEach()', function () {
it('throws if callback is not a function', async function () {
try {
const colRef = firebase.firestore().collection(COLLECTION);
const colRef = firebase.firestore().collection(`${COLLECTION}/callbacks/nonfunction`);
await colRef.add({});
const snapshot = await colRef.limit(1).get();
snapshot.forEach(123);
return Promise.reject(new Error('Did not throw an Error.'));
Expand All @@ -184,13 +191,14 @@ describe('firestore.QuerySnapshot', function () {
});

it('calls back a function', async function () {
const colRef = firebase.firestore().collection(COLLECTION);
colRef.add({});
colRef.add({});
const colRef = firebase.firestore().collection(`${COLLECTION}/callbacks/function`);
await colRef.add({});
await colRef.add({});
const snapshot = await colRef.limit(2).get();
const callback = sinon.spy();
snapshot.forEach.should.be.Function();
snapshot.forEach(callback);
await Utils.spyToBeCalledTimesAsync(callback, 2, 20000);
callback.should.be.calledTwice();
callback.args[0][0].constructor.name.should.eql('FirestoreDocumentSnapshot');
callback.args[0][1].should.be.Number();
Expand All @@ -199,8 +207,8 @@ describe('firestore.QuerySnapshot', function () {
});

it('provides context to the callback', async function () {
const colRef = firebase.firestore().collection(COLLECTION);
colRef.add({});
const colRef = firebase.firestore().collection(`${COLLECTION}/callbacks/function-context`);
await colRef.add({});
const snapshot = await colRef.limit(1).get();
const callback = sinon.spy();
snapshot.forEach.should.be.Function();
Expand Down

1 comment on commit 8916e19

@vercel
Copy link

@vercel vercel bot commented on 8916e19 Dec 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.