From 9be7894676f17d3145012834364b190c3b3780c9 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 5 Jul 2023 13:04:14 -0400 Subject: [PATCH 1/6] query.test.ts: Use a WriteBatch to delete documents rather than a transaction, for simplicity. --- .../test/integration/api/query.test.ts | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 90d146344ab..8e3150e62df 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -2096,16 +2096,16 @@ apiDescribe('Queries', persistence => { expect(snapshot1.size, 'snapshot1.size').to.equal(100); const createdDocuments = snapshot1.docs.map(snapshot => snapshot.ref); - // Delete 50 of the 100 documents. Do this in a transaction, rather than + // Delete 50 of the 100 documents. Use a WriteBatch, rather than // deleteDoc(), to avoid affecting the local cache. const deletedDocumentIds = new Set(); - await runTransaction(db, async txn => { - for (let i = 0; i < createdDocuments.length; i += 2) { - const documentToDelete = createdDocuments[i]; - txn.delete(documentToDelete); - deletedDocumentIds.add(documentToDelete.id); - } - }); + const writeBatchForDocumentDeletes = writeBatch(db); + for (let i = 0; i < createdDocuments.length; i += 2) { + const documentToDelete = createdDocuments[i]; + writeBatchForDocumentDeletes.delete(documentToDelete); + deletedDocumentIds.add(documentToDelete.id); + } + await writeBatchForDocumentDeletes.commit(); // Wait for 10 seconds, during which Watch will stop tracking the query // and will send an existence filter rather than "delete" events when @@ -2264,20 +2264,12 @@ apiDescribe('Queries', persistence => { 'snapshot1DocumentIds' ).to.have.members(testDocIds); - // Delete one of the documents so that the next call to getDocs() will - // experience an existence filter mismatch. Do this deletion in a - // transaction, rather than using deleteDoc(), to avoid affecting the - // local cache. - await runTransaction(db, async txn => { - const snapshotOfDocumentToDelete = await txn.get( - doc(coll, 'DocumentToDelete') - ); - expect( - snapshotOfDocumentToDelete.exists(), - 'snapshotOfDocumentToDelete.exists()' - ).to.be.true; - txn.delete(snapshotOfDocumentToDelete.ref); - }); + // Delete one of the documents so that the next call to getDocs() + // will experience an existence filter mismatch. Use a WriteBatch, + // rather than deleteDoc(), to avoid affecting the local cache. + const writeBatchForDocumentDeletes = writeBatch(db); + writeBatchForDocumentDeletes.delete(doc(coll, 'DocumentToDelete')); + await writeBatchForDocumentDeletes.commit(); // Wait for 10 seconds, during which Watch will stop tracking the // query and will send an existence filter rather than "delete" events From 32f3f148658e8bad477bd1a9a14e61104894ac71 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 5 Jul 2023 14:09:24 -0400 Subject: [PATCH 2/6] query.test.ts: remove unused import of `runTransaction` --- packages/firestore/test/integration/api/query.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 8e3150e62df..a2a0acb5520 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -48,7 +48,6 @@ import { Query, query, QuerySnapshot, - runTransaction, setDoc, startAfter, startAt, From 70858b1c5207bad619cbcbed3b523c11dbb6879c Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Fri, 7 Jul 2023 01:10:56 +0000 Subject: [PATCH 3/6] REVERT ME: ../../.github/workflows/test-all.yml isolate the test that's failing --- .github/workflows/test-all.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-all.yml b/.github/workflows/test-all.yml index c937d2921e4..a622070da02 100644 --- a/.github/workflows/test-all.yml +++ b/.github/workflows/test-all.yml @@ -172,7 +172,7 @@ jobs: run: echo "FIREBASE_CI_TEST_START_TIME=$(date +%s)" >> $GITHUB_ENV - name: Run unit tests run: | - xvfb-run yarn lerna run --concurrency 4 test:ci --scope '{@firebase/firestore*,firebase-firestore-integration-test}' + xvfb-run yarn lerna run --concurrency 1 test:ci --scope '{@firebase/firestore-compat}' node scripts/print_test_logs.js env: FIREBASE_TOKEN: ${{ secrets.FIREBASE_CLI_TOKEN }} From 793100e6be32f82c5a14ebaf5a990ea3e6e6fcce Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Fri, 7 Jul 2023 03:25:22 +0000 Subject: [PATCH 4/6] REVERT ME: fix typo: {@firebase/firestore-compat} -> @firebase/firestore-compat --- .github/workflows/test-all.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-all.yml b/.github/workflows/test-all.yml index a622070da02..84882ce0f7b 100644 --- a/.github/workflows/test-all.yml +++ b/.github/workflows/test-all.yml @@ -172,7 +172,7 @@ jobs: run: echo "FIREBASE_CI_TEST_START_TIME=$(date +%s)" >> $GITHUB_ENV - name: Run unit tests run: | - xvfb-run yarn lerna run --concurrency 1 test:ci --scope '{@firebase/firestore-compat}' + xvfb-run yarn lerna run --concurrency 1 test:ci --scope '@firebase/firestore-compat' node scripts/print_test_logs.js env: FIREBASE_TOKEN: ${{ secrets.FIREBASE_CLI_TOKEN }} From a63fe417395f4d568fec4091e8a80cb596b9da46 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Fri, 7 Jul 2023 01:56:59 -0400 Subject: [PATCH 5/6] REVERT ME: Add back the other packages, since it passed with @firebase/firestore-compat, but leave concurrency at 1. --- .github/workflows/test-all.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-all.yml b/.github/workflows/test-all.yml index 84882ce0f7b..aa9fc79db4b 100644 --- a/.github/workflows/test-all.yml +++ b/.github/workflows/test-all.yml @@ -172,7 +172,7 @@ jobs: run: echo "FIREBASE_CI_TEST_START_TIME=$(date +%s)" >> $GITHUB_ENV - name: Run unit tests run: | - xvfb-run yarn lerna run --concurrency 1 test:ci --scope '@firebase/firestore-compat' + xvfb-run yarn lerna run --concurrency 1 test:ci --scope '{@firebase/firestore*,firebase-firestore-integration-test}' node scripts/print_test_logs.js env: FIREBASE_TOKEN: ${{ secrets.FIREBASE_CLI_TOKEN }} From 64b9f09c2fad77a904a4e7f5a5ef6c206be33c97 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Fri, 7 Jul 2023 09:21:53 -0400 Subject: [PATCH 6/6] .github/workflows/test-all.yml: revert all changes; the test passed with concurrency=1 so fingers are crossed that it will pass again with concurrency=4 --- .github/workflows/test-all.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-all.yml b/.github/workflows/test-all.yml index aa9fc79db4b..c937d2921e4 100644 --- a/.github/workflows/test-all.yml +++ b/.github/workflows/test-all.yml @@ -172,7 +172,7 @@ jobs: run: echo "FIREBASE_CI_TEST_START_TIME=$(date +%s)" >> $GITHUB_ENV - name: Run unit tests run: | - xvfb-run yarn lerna run --concurrency 1 test:ci --scope '{@firebase/firestore*,firebase-firestore-integration-test}' + xvfb-run yarn lerna run --concurrency 4 test:ci --scope '{@firebase/firestore*,firebase-firestore-integration-test}' node scripts/print_test_logs.js env: FIREBASE_TOKEN: ${{ secrets.FIREBASE_CLI_TOKEN }}