Firestore: query.test.ts: use a different Firestore instance instead of a WriteBatch

[dconeybe]

Use a different Firestore instance instead of a WriteBatch to avoid affecting the local cache in query.test.ts.

I had originally thought that WriteBatch sidestepped the local cache, just like a transaction did; however, that turned out to be incorrect and using a WriteBatch nullified the test cases that were modified to use it. This is a follow-up to #7415.

This PR was ported to the Android SDK in firebase/firebase-android-sdk#5184 and to the iOS SDK in firebase/firebase-ios-sdk#11615.

[changeset-bot]

⚠️ No Changeset found

Latest commit: c59f042

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[google-oss-bot]

Size Report ¹

Affected Products

No changes between base commit (223ac18) and merge commit (daf3069).

Test Logs

• Base (223ac18): https://github.com/firebase/firebase-js-sdk/actions/runs/5616065435
• Merge (daf3069): https://github.com/firebase/firebase-js-sdk/actions/runs/5616959166

1. https://storage.googleapis.com/firebase-sdk-metric-reports/ZFdl0c5I2B.html

[google-oss-bot]

Size Analysis Report ¹

Affected Products

No changes between base commit (223ac18) and merge commit (daf3069).

Test Logs

• Base (223ac18): https://github.com/firebase/firebase-js-sdk/actions/runs/5616065435
• Merge (daf3069): https://github.com/firebase/firebase-js-sdk/actions/runs/5616959166

1. https://storage.googleapis.com/firebase-sdk-metric-reports/MEQzNXThWf.html

[DellaBitta]

Checking to see if this should be in the release notes.

Is this a change related to a user issue? Should users expect a behavior change?

[dconeybe]

Checking to see if this should be in the release notes.

Is this a change related to a user issue? Should users expect a behavior change?

No, this is just a fix to an integration test. No user behavior is affected.

[DellaBitta]

Ah, I see. My bad! Thanks.

[milaGGL]

wouldn't this be returning the same Firestore instance(db === db2)? How it would help us avoid affecting cache?

[dconeybe]

Good question (I had to look into this too). But the answer is no because withTestDb() creates and returns a newly-created, distinct Firestore instance. Therefore, db and db2 have distinct caches and do not share anything, and using db2 to delete/update documents does not affect the local cache of db.

Here is the definition of withTestDb():

firebase-js-sdk/packages/firestore/test/lite/helpers.ts

Lines 57 to 61 in 223ac18

	export function withTestDb(
	fn: (db: Firestore) => void | Promise<void>
	): Promise<void> {
	return withTestDbSettings(DEFAULT_PROJECT_ID, DEFAULT_SETTINGS, fn);
	}

which calls withTestDbSettings():

firebase-js-sdk/packages/firestore/test/lite/helpers.ts

Lines 43 to 55 in 223ac18

	export async function withTestDbSettings(
	projectId: string,
	settings: FirestoreSettings,
	fn: (db: Firestore) => void | Promise<void>
	): Promise<void> {
	const app = initializeApp(
	{ apiKey: 'fake-api-key', projectId },
	'test-app-' + appCount++
	);
	const firestore = initializeFirestore(app, settings);
	return fn(firestore);
	}

which creates a brand new FirestoreApp with a unique name (the appCount counter increments for each invocation) and then creates a brand new Firestore object from it.

[milaGGL]

Interesting. So they are instances to a same firestore, but from different apps.

    { apiKey: 'fake-api-key', projectId },
    'test-app-' + appCount++
  );``` 

[dconeybe]

Yep, exactly.

[milaGGL]

LGTM