cleanup

Author: dlarocque

.github/workflows/test-all.yml

@@ -14,7 +14,7 @@
 
 name: Test All Packages
 
-on: push
+on: pull_request
 
 env:
   # make chromedriver detect installed Chrome version and download the corresponding driver
@@ -92,8 +92,7 @@ jobs:
     - name: Run unit tests
       # Ignore auth and firestore since they're handled in their own separate jobs.
       run: |
-        # xvfb-run yarn lerna run --ignore '{firebase-messaging-integration-test,@firebase/auth*,@firebase/firestore*,firebase-firestore-integration-test}' test:ci
-        xvfb-run yarn lerna run --scope @firebase/data-connect test
+        xvfb-run yarn lerna run --ignore '{firebase-messaging-integration-test,@firebase/auth*,@firebase/firestore*,firebase-firestore-integration-test}' test:ci
         node scripts/print_test_logs.js
       env:
         FIREBASE_TOKEN: ${{ secrets.FIREBASE_CLI_TOKEN }}

scripts/emulator-testing/emulators/emulator.ts

@@ -56,9 +56,16 @@ export abstract class Emulator {
     const { name: tempDir } = tmp.dirSync({ unsafeCleanup: true });
     const filepath = path.resolve(tempDir, this.binaryName);
     return new Promise<void>((resolve, reject) => {
-      // We want access to `this.binaryPath` after the download is finished, but we can't since in
-      // `readChunk`, `this` is not inherited from the parent (since it's a named function expression).
-      // To work around this, we wrap the fetch in a promise, then once it's resolved we can access `this` in the callback arrow function.
+      /**
+       * Once the download is `done` in `readChunk`, we want to set `this.binaryPath` to the path of the 
+       * downloaded emulator. Unfortunately, we can't access `this` when inside `readChunk`'s scope, since
+       * it's a named function expression, and does not inherit the `this` object from it's parent.
+       * To work around this, we wrap the fetch in a promise, 
+       * then once it's resolved we can access `this` in a callback arrow function that *does* inherit
+       * `this` from the parent object, allowing us to set `this.binaryPath`.
+       * Note that we can't make readChunk an arrow function, since it needs to be named so that we can
+       * perform recursion to read the next chunk.
+       */
       const downloadPromise = new Promise<Uint8Array>(
         (downloadComplete, downloadFailed) => {
           fetch(this.binaryUrl)