Skip to content

Avoid using hardcoded document paths which could cause conflicts when tests are running concurrently (e.g. on Travis). #250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 20, 2017
Merged

Avoid using hardcoded document paths which could cause conflicts when tests are running concurrently (e.g. on Travis). #250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e4bb6b8
Avoid using hardcoded document paths which could cause conflicts when…
Oct 20, 2017
6c789b1
Review feedback.
Oct 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 22 additions & 30 deletions packages/firestore/test/integration/api/database.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,19 @@ import * as firestore from 'firestore';
import { Deferred } from '../../../src/util/promise';
import { asyncIt } from '../../util/helpers';
import firebase from '../util/firebase_export';
import { apiDescribe, withTestCollection, withTestDb } from '../util/helpers';
import {
apiDescribe,
withTestCollection,
withTestDb,
withTestDoc
} from '../util/helpers';
import { Firestore } from '../../../src/api/database';

apiDescribe('Database', persistence => {
asyncIt('can set a document', () => {
return withTestDb(persistence, db => {
return db.doc('rooms/Eros').set({
desc: 'Stuff related to Eros project...',
return withTestDoc(persistence, docRef => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably not worth the effort, but it would be niceish if we picked either docRef or doc and not both :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. If we did that I'd say we should do it in a separate PR... but I don't think it's worth the effort. :-)

return docRef.set({
desc: 'Stuff related to Firestore project...',
owner: {
name: 'Jonny',
title: 'scallywag'
Expand All @@ -46,8 +51,7 @@ apiDescribe('Database', persistence => {
});

asyncIt('can delete a document', () => {
return withTestDb(persistence, db => {
const docRef = db.doc('rooms/Eros');
return withTestDoc(persistence, docRef => {
return docRef
.set({ foo: 'bar' })
.then(() => {
Expand All @@ -67,8 +71,7 @@ apiDescribe('Database', persistence => {
});

asyncIt('can update existing document', () => {
return withTestDb(persistence, db => {
const doc = db.doc('rooms/Eros');
return withTestDoc(persistence, doc => {
const initialData = {
desc: 'Description',
owner: { name: 'Jonny', email: '[email protected]' }
Expand All @@ -93,8 +96,7 @@ apiDescribe('Database', persistence => {
});

asyncIt('can merge data with an existing document using set', () => {
return withTestDb(persistence, db => {
const doc = db.doc('rooms/Eros');
return withTestDoc(persistence, doc => {
const initialData = {
desc: 'description',
'owner.data': { name: 'Jonny', email: '[email protected]' }
Expand All @@ -120,8 +122,7 @@ apiDescribe('Database', persistence => {
});

asyncIt('can merge server timestamps', () => {
return withTestDb(persistence, db => {
const doc = db.doc('rooms/Eros');
return withTestDoc(persistence, doc => {
const initialData = {
updated: false
};
Expand All @@ -141,8 +142,7 @@ apiDescribe('Database', persistence => {
});

asyncIt('can replace an array by merging using set', () => {
return withTestDb(persistence, db => {
const doc = db.doc('rooms/Eros');
return withTestDoc(persistence, doc => {
const initialData = {
untouched: true,
data: 'old',
Expand Down Expand Up @@ -172,8 +172,7 @@ apiDescribe('Database', persistence => {
});

asyncIt('cannot update nonexistent document', () => {
return withTestDb(persistence, db => {
const doc = db.collection('rooms').doc();
return withTestDoc(persistence, doc => {
return doc
.update({ owner: 'abc' })
.then(
Expand All @@ -192,8 +191,7 @@ apiDescribe('Database', persistence => {
});

asyncIt('can delete a field with an update', () => {
return withTestDb(persistence, db => {
const doc = db.doc('rooms/Eros');
return withTestDoc(persistence, doc => {
const initialData = {
desc: 'Description',
owner: { name: 'Jonny', email: '[email protected]' }
Expand All @@ -219,8 +217,7 @@ apiDescribe('Database', persistence => {
asyncIt('can update nested fields', () => {
const FieldPath = firebase.firestore.FieldPath;

return withTestDb(persistence, db => {
const doc = db.doc('rooms/Eros');
return withTestDoc(persistence, doc => {
const initialData = {
desc: 'Description',
owner: { name: 'Jonny' },
Expand Down Expand Up @@ -248,8 +245,7 @@ apiDescribe('Database', persistence => {
const invalidDocValues = [undefined, null, 0, 'foo', ['a'], new Date()];
for (const val of invalidDocValues) {
asyncIt('set/update should reject: ' + val, () => {
return withTestDb(persistence, db => {
const doc = db.collection('rooms').doc();
return withTestDoc(persistence, doc => {
// tslint:disable-next-line:no-any Intentionally passing bad types.
expect(() => doc.set(val as any)).to.throw();
// tslint:disable-next-line:no-any Intentionally passing bad types.
Expand Down Expand Up @@ -337,10 +333,9 @@ apiDescribe('Database', persistence => {
});

asyncIt('Local document events are fired with hasLocalChanges=true.', () => {
return withTestDb(persistence, db => {
return withTestDoc(persistence, docRef => {
let gotLocalDocEvent = false;
const remoteDocEventDeferred = new Deferred();
const docRef = db.collection('rooms').doc();
const unlisten = docRef.onSnapshot(
{ includeMetadataChanges: true },
doc => {
Expand All @@ -366,10 +361,9 @@ apiDescribe('Database', persistence => {
asyncIt(
'Metadata only changes are not fired when no options provided',
() => {
return withTestDb(persistence, db => {
return withTestDoc(persistence, docRef => {
const secondUpdateFound = new Deferred();
let count = 0;
const docRef = db.collection('rooms').doc();
const unlisten = docRef.onSnapshot(doc => {
if (doc) {
count++;
Expand Down Expand Up @@ -563,8 +557,7 @@ apiDescribe('Database', persistence => {
});

asyncIt('can queue writes while offline', () => {
return withTestDb(persistence, db => {
const docRef = db.collection('rooms').doc();
return withTestDoc(persistence, docRef => {
// TODO(mikelehen): Find better way to expose this to tests.
// tslint:disable-next-line:no-any enableNetwork isn't exposed via d.ts
const firestoreInternal = docRef.firestore.INTERNAL as any;
Expand All @@ -585,8 +578,7 @@ apiDescribe('Database', persistence => {
});

asyncIt('can get documents while offline', () => {
return withTestDb(persistence, db => {
const docRef = db.collection('rooms').doc();
return withTestDoc(persistence, docRef => {
// TODO(mikelehen): Find better way to expose this to tests.
// tslint:disable-next-line:no-any enableNetwork isn't exposed via d.ts
const firestoreInternal = docRef.firestore.INTERNAL as any;
Expand Down
57 changes: 18 additions & 39 deletions packages/firestore/test/integration/api/query.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,52 +106,31 @@ apiDescribe('Queries', persistence => {
});

asyncIt('can use unary filters', () => {
return withTestDbs(persistence, 2, ([writerDb, readerDb]) => {
return Promise.all([
writerDb
.collection('query_test')
.doc('a')
.set({ null: null, nan: NaN }),
writerDb
.collection('query_test')
.doc('b')
.set({ null: null, nan: 0 }),
writerDb
.collection('query_test')
.doc('c')
.set({ null: false, nan: NaN })
])
.then(() => {
return readerDb
.collection('query_test')
.where('null', '==', null)
.where('nan', '==', NaN)
.get();
})
const testDocs = {
a: { null: null, nan: NaN },
b: { null: null, nan: 0 },
c: { null: false, nan: NaN }
};
return withTestCollection(persistence, testDocs, coll => {
return coll
.where('null', '==', null)
.where('nan', '==', NaN)
.get()
.then(docs => {
expect(toDataArray(docs)).to.deep.equal([{ null: null, nan: NaN }]);
});
});
});

asyncIt('can filter on infinity', () => {
return withTestDbs(persistence, 2, ([writerDb, readerDb]) => {
return Promise.all([
writerDb
.collection('query_test')
.doc('a')
.set({ inf: Infinity }),
writerDb
.collection('query_test')
.doc('b')
.set({ inf: -Infinity })
])
.then(() => {
return readerDb
.collection('query_test')
.where('inf', '==', Infinity)
.get();
})
const testDocs = {
a: { inf: Infinity },
b: { inf: -Infinity }
};
return withTestCollection(persistence, testDocs, coll => {
return coll
.where('inf', '==', Infinity)
.get()
.then(docs => {
expect(toDataArray(docs)).to.deep.equal([{ inf: Infinity }]);
});
Expand Down
41 changes: 21 additions & 20 deletions packages/firestore/test/integration/api/smoke.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,8 @@ apiDescribe('Smoke Test', persistence => {
});

asyncIt('can read a written document with DocumentKey', () => {
return integrationHelpers.withTestDb(persistence, db => {
const ref1 = db.doc('rooms/eros/messages/2');
const ref2 = db.doc('users/patryk');
return integrationHelpers.withTestDoc(persistence, ref1 => {
const ref2 = ref1.firestore.collection('users').doc();
const data = { user: ref2, message: 'We are writing data' };
return ref2.set({ name: 'patryk' }).then(() => {
return ref1
Expand All @@ -79,8 +78,8 @@ apiDescribe('Smoke Test', persistence => {
persistence,
2,
([reader, writer]) => {
const readerRef = reader.doc('rooms/eros/messages/1');
const writerRef = writer.doc('rooms/eros/messages/1');
const readerRef = reader.collection('rooms/firestore/messages').doc();
const writerRef = writer.doc(readerRef.path);
const data = {
name: 'Patryk',
message: 'We are actually writing data!'
Expand All @@ -95,27 +94,29 @@ apiDescribe('Smoke Test', persistence => {
expect(docSnap.exists).to.equal(true);
expect(docSnap.data()).to.deep.equal(data);
})
.then(() => unlisten(), () => unlisten());
.then(() => unlisten());
});
}
);
});

asyncIt('will fire value events for empty collections', () => {
return integrationHelpers.withTestDb(persistence, db => {
const collection = db.collection('empty-collection');

const accum = new EventsAccumulator<firestore.QuerySnapshot>();
const unlisten = collection.onSnapshot(accum.storeEvent);
return accum
.awaitEvent()
.then(querySnap => {
expect(querySnap.empty).to.equal(true);
expect(querySnap.size).to.equal(0);
expect(querySnap.docs.length).to.equal(0);
})
.then(() => unlisten(), () => unlisten());
});
return integrationHelpers.withTestCollection(
persistence,
{},
collection => {
const accum = new EventsAccumulator<firestore.QuerySnapshot>();
const unlisten = collection.onSnapshot(accum.storeEvent);
return accum
.awaitEvent()
.then(querySnap => {
expect(querySnap.empty).to.equal(true);
expect(querySnap.size).to.equal(0);
expect(querySnap.docs.length).to.equal(0);
})
.then(() => unlisten());
}
);
});

asyncIt('can get collection query', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/test/integration/api/transactions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ apiDescribe('Database transactions', persistence => {

asyncIt('cannot have a get without mutations', () => {
return integrationHelpers.withTestDb(persistence, db => {
const docRef = db.doc('foo/bar');
const docRef = db.collection('foo').doc();
return (
docRef
.set({ foo: 'bar' })
Expand Down
8 changes: 3 additions & 5 deletions packages/firestore/test/integration/api/type.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const asyncIt = testHelpers.asyncIt;

apiDescribe('Firestore', persistence => {
function expectRoundtrip(db: firestore.Firestore, data: {}): Promise<void> {
const doc = db.doc('rooms/Eros');
const doc = db.collection('rooms').doc();
return doc
.set(data)
.then(() => doc.get())
Expand All @@ -47,8 +47,7 @@ apiDescribe('Firestore', persistence => {
});

asyncIt('can read and write geo point fields', () => {
return withTestDb(persistence, db => {
const doc = db.doc('rooms/Eros');
return withTestDoc(persistence, doc => {
return doc
.set({ geopoint: new firebase.firestore.GeoPoint(1.23, 4.56) })
.then(() => {
Expand All @@ -64,8 +63,7 @@ apiDescribe('Firestore', persistence => {
});

asyncIt('can read and write bytes fields', () => {
return withTestDb(persistence, db => {
const doc = db.doc('rooms/Eros');
return withTestDoc(persistence, doc => {
return doc
.set({
bytes: firebase.firestore.Blob.fromUint8Array(
Expand Down
4 changes: 2 additions & 2 deletions packages/firestore/test/integration/api/validation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,8 @@ apiDescribe('Validation:', persistence => {
validationIt(persistence, 'may contain indirectly nested arrays.', db => {
const data = { 'nested-array': [1, { foo: [2] }] };

const ref = db.doc('foo/bar');
const ref2 = db.doc('foo/quux');
const ref = db.collection('foo').doc();
const ref2 = db.collection('foo').doc();

ref
.set(data)
Expand Down