Cypress: One watcher per test, wait for network idleness

packages/cypress/src/index.ts

@@ -7,6 +7,7 @@
   ResourceArchive,
   Viewport,
 } from '@chromatic-com/shared-e2e';
+import { NetworkIdleWatcher } from './network-idle-watcher';
 
 interface CypressSnapshot {
   // the name of the snapshot (optionally provided for manual snapshots, never provided for automatic snapshots)
@@ -58,50 +59,131 @@
   );
 };
 
-// using a single ResourceArchiver instance across all tests (for the test run)
-// each time a test completes, we'll save to disk whatever archives are there at that point.
-// This should be safe since the same resource from the same URL should be the same during the entire test run.
-// Cypress doesn't give us a way to share variables between the "before test" and "after test" lifecycle events on the server.
-let resourceArchiver: ResourceArchiver = null;
+// Cypress doesn't have a way (on the server) of scoping things per-test.
+// Thus we'll make a lookup table of ResourceArchivers (one per test, with testId as the key)
+// So we can still have test-specific archiving configuration (like which domains to archive)
+const resourceArchivers: Record<string, ResourceArchiver> = {};
+// same for network idle watchers
+const networkIdleWatchers: Record<string, NetworkIdleWatcher> = {};
+// Each test's (archivable) network requests. Includes requests that have a cached response.
+const testSpecificArchiveUrls: Record<string, string[]> = {};
+// The test-run-wide archive, includes all of the resources in all of the tests
+let mainArchive: ResourceArchive = {};
 
 let host = '';
 let port = 0;
+let debuggerUrl = '';
 
 const setupNetworkListener = async ({
   allowedDomains,
+  testId,
 }: {
   allowedDomains?: string[];
+  testId: string;
 }): Promise<null> => {
   try {
-    const { webSocketDebuggerUrl } = await Version({
-      host,
-      port,
-    });
+    if (!debuggerUrl) {
+      const { webSocketDebuggerUrl } = await Version({
+        host,
+        port,
+      });
+      debuggerUrl = webSocketDebuggerUrl;
+    }
 
     const cdp = await CDP({
-      target: webSocketDebuggerUrl,
+      target: debuggerUrl,
     });
 
-    if (!resourceArchiver) {
-      resourceArchiver = new ResourceArchiver(cdp, allowedDomains);
-      await resourceArchiver.watch();
-    }
+    const networkIdleWatcher = new NetworkIdleWatcher();
+    networkIdleWatchers[testId] = networkIdleWatcher;
+    testSpecificArchiveUrls[testId] = [];
+    resourceArchivers[testId] = new ResourceArchiver({
+      cdpClient: cdp,
+      allowedDomains,
+      // important that we don't directly pass networkIdleWatcher.onRequest here,
+      // as that'd bind `this` in that method to the ResourceArchiver
+      onRequest: (url) => {
+        networkIdleWatcher.onRequest(url);
+        // gather all the requests that went out, so we can archive even the cached resources
+        // it's possible requests are sent out after we're done waiting for the test (and we delete `testSpecificArchiveUrls`
+        // when that happens), so ensure it still exists first.
+        if (testSpecificArchiveUrls[testId]) {
+          testSpecificArchiveUrls[testId].push(url);
+        }
+      },
+      // important that we don't directly pass networkIdleWatcher.onResponse here,
+      // as that'd bind `this` in that method to the ResourceArchiver
+      onResponse: (url) => {
+        networkIdleWatcher.onResponse(url);
+      },
+    });
+    await resourceArchivers[testId].watch();
   } catch (err) {
     console.log('err', err);
   }
 
   return null;
 };
 
-const saveArchives = (archiveInfo: WriteParams) => {
+const saveArchives = (archiveInfo: WriteParams & { testId: string }) => {
   return new Promise((resolve) => {
-    // the resourceArchiver's archives come from the server, everything else (DOM snapshots, test info, etc) comes from the browser
-    // notice we're not calling + awaiting resourceArchiver.idle() here...
-    // that's because in Cypress, cy.visit() waits until all resources have loaded before finishing
-    // so at this point (after the test) we're confident that the resources are all there already without having to wait more
-    return writeArchives({ ...archiveInfo, resourceArchive: resourceArchiver.archive }).then(() => {
+    const { testId, ...rest } = archiveInfo;
+    const resourceArchiver = resourceArchivers[testId];
+    if (!resourceArchiver || !testSpecificArchiveUrls[testId]) {
+      console.error('Unable to archive results for test');
       resolve(null);
-    });
+    }
+
+    const networkIdleWatcher = networkIdleWatchers[testId];
+    if (!networkIdleWatcher) {
+      console.error('No idle watcher found for test');
+      resolve(null);
+    }
+
+    // `finally` instead of `then` because we need to know idleness however it happened
+    return (
+      networkIdleWatcher
+        .idle()
+        // errors that happened when detecting network idleness should be logged,
+        // but shouldn't error out the entire Cypress test run
+        .catch((err: Error) => {
+          console.error(`Error when archiving resources for test "${testId}": ${err.message}`);
+        })
+        .finally(() => {
+          // the archives come from the server, everything else (DOM snapshots, test info, etc) comes from the browser
+          const { archive } = resourceArchiver;
+
+          // include any new resources from this archive
+          mainArchive = {
+            ...mainArchive,
+            ...archive,
+          };
+
+          const finalArchive: ResourceArchive = {};
+
+          // make a subset of this archive that you will actually save
+          testSpecificArchiveUrls[testId].forEach((url) => {
+            if (mainArchive[url]) {
+              // since the test's resources may have been cached,
+              // pull the resource off of the main (test-run-wide) archive,
+              // to get the actual resource
+              finalArchive[url] = mainArchive[url];
+            }
+          });
+
+          // clean up the CDP instance
+          return resourceArchivers[testId].close().then(() => {
+            // remove archives off of object after write them
+            delete resourceArchivers[testId];
+            // clean up now-unneeded objects
+            delete testSpecificArchiveUrls[testId];
+            delete networkIdleWatchers[testId];
+            return writeArchives({ ...rest, resourceArchive: finalArchive }).then(() => {
+              resolve(null);
+            });
+          });
+        })
+    );
   });
 };
 

packages/cypress/src/network-idle-watcher.test.ts

@@ -1,6 +1,17 @@
-import { NetworkIdleWatcher } from './network-idle-watcher';
-
-jest.useFakeTimers();
+import {
+  NetworkIdleWatcher,
+  TOTAL_TIMEOUT_DURATION,
+  WATERFALL_BETWEEN_STEPS_DURATION,
+} from './network-idle-watcher';
+
+const A_PAGE_URL = 'https://some-url.com';
+const A_RESOURCE_URL = 'https://some-url.com/images/cool.jpg';
+const ANOTHER_RESOURCE_URL = 'https://some-url.com/images/nice.jpg';
+const YET_ANOTHER_RESOURCE_URL = 'https://some-url.com/images/awesome.jpg';
+
+beforeEach(() => {
+  jest.useFakeTimers();
+});
 
 it('Resolves when there is no network activity', async () => {
   const watcher = new NetworkIdleWatcher();
@@ -10,34 +21,34 @@ it('Resolves when there is no network activity', async () => {
 it('Resolves when there is a single request and response', async () => {
   const watcher = new NetworkIdleWatcher();
 
-  watcher.onRequest();
-  watcher.onResponse();
+  watcher.onRequest(A_PAGE_URL);
+  watcher.onResponse(A_PAGE_URL);
 
   await expect(watcher.idle()).resolves.toBeDefined();
 });
 
 it('Resolves when there are an equal amount of requests and responses', async () => {
   const watcher = new NetworkIdleWatcher();
   // in total 4 requests, and 4 responses
-  watcher.onRequest();
-  watcher.onResponse();
+  watcher.onRequest(A_PAGE_URL);
+  watcher.onResponse(A_PAGE_URL);
 
-  watcher.onRequest();
-  watcher.onResponse();
+  watcher.onRequest(A_RESOURCE_URL);
+  watcher.onResponse(A_RESOURCE_URL);
 
-  watcher.onRequest();
-  watcher.onRequest();
+  watcher.onRequest(ANOTHER_RESOURCE_URL);
+  watcher.onRequest(YET_ANOTHER_RESOURCE_URL);
 
-  watcher.onResponse();
-  watcher.onResponse();
+  watcher.onResponse(ANOTHER_RESOURCE_URL);
+  watcher.onResponse(YET_ANOTHER_RESOURCE_URL);
 
   await expect(watcher.idle()).resolves.toBeDefined();
 });
 
 it('Rejects if response never sent for request', async () => {
   const watcher = new NetworkIdleWatcher();
   // fire off request
-  watcher.onRequest();
+  watcher.onRequest(A_RESOURCE_URL);
   const promise = watcher.idle();
   jest.runAllTimers();
   // no response fired off
@@ -47,30 +58,150 @@ it('Rejects if response never sent for request', async () => {
 it("Resolves if response hasn't happened at time of idle(), but comes back before timeout", async () => {
   const watcher = new NetworkIdleWatcher();
   // fire off request
-  watcher.onRequest();
+  watcher.onRequest(A_RESOURCE_URL);
 
   const promise = watcher.idle();
 
-  watcher.onResponse();
-  // makes sure we finish the idle watcher as soon as the reponse comes back, and not waiting the full timeout duration
-  jest.advanceTimersByTime(1);
+  watcher.onResponse(A_RESOURCE_URL);
+  // makes sure we finish the idle watcher as soon as we can, and not waiting the full timeout duration
+  jest.advanceTimersByTime(WATERFALL_BETWEEN_STEPS_DURATION);
 
   await expect(promise).resolves.toBeDefined();
 });
 
 it("Rejects if response hasn't happened at time of idle(), and doesn't come back before timeout", async () => {
   const watcher = new NetworkIdleWatcher();
   // fire off request
-  watcher.onRequest();
+  watcher.onRequest(A_RESOURCE_URL);
 
   const promise = watcher.idle();
 
   // response returns after idle() has been called, but will take too long
   setTimeout(() => {
-    watcher.onResponse();
-  }, 10000);
+    watcher.onResponse(A_RESOURCE_URL);
+  }, TOTAL_TIMEOUT_DURATION * 2);
 
   jest.runAllTimers();
 
   await expect(promise).rejects.toBeDefined();
 });
+
+/*
+  Waits until the next event loop, so that promise resolutions happen before code that comes after it
+  This is needed because promises otherwise resolve after their assertions, like so:
+    promise.then(() => {
+      callback();
+    });
+
+    // some code that should cause `promise` to resolve
+
+    // this will fail because the promise resolution happens on the next event loop
+    expect(callback).toHaveBeenCalled();
+
+  `flushPromises` ensures that we wait for the (next event loop) promise resolutions before asserting.
+*/
+const flushPromises = () => {
+  // props to https://github.com/jestjs/jest/issues/2157#issuecomment-897935688
+  return new Promise((resolve) => {
+    jest.requireActual('timers').setImmediate(() => resolve(null));
+  });
+};
+
+it("Does not prematurely resolve if there's a small gap between one response ending and another request beginning (typical network waterfall)", async () => {
+  const callback = jest.fn();
+  // Simulate a typical page network waterfall, like this:
+  // --------HTML Document--------
+  //                                -------Resource HTML Document requests-------
+  const watcher = new NetworkIdleWatcher();
+  // fire off an initial request
+  watcher.onRequest(A_PAGE_URL);
+  const promise = watcher.idle();
+
+  // For some reason, trying to assert on promise.resolves didn't work for the first assertion --
+  // The eventual resolution of the promise (later on) would retroactively fail the first assertion.
+  // Hence we're using a callback instead.
+  promise.then(() => {
+    callback();
+  });
+
+  watcher.onResponse(A_PAGE_URL);
+  await flushPromises();
+  // verify idle() hasn't been resolved yet
+  expect(callback).not.toHaveBeenCalled();
+
+  // send off another request and response
+  watcher.onRequest(A_RESOURCE_URL);
+  watcher.onResponse(A_RESOURCE_URL);
+
+  jest.runAllTimers();
+  await flushPromises();
+  // verify that idle() has now been resolved
+  expect(callback).toHaveBeenCalled();
+});
+
+const waitForResponse = (durationInMs: number) => {
+  return new Promise((resolve) => {
+    setTimeout(() => {
+      resolve(null);
+    }, durationInMs);
+  });
+};
+
+it('Rejects if initial response comes back in time, but subsequent response does not', async () => {
+  const watcher = new NetworkIdleWatcher();
+  watcher.onRequest(A_PAGE_URL);
+  const promise = watcher.idle();
+
+  watcher.onResponse(A_PAGE_URL);
+  watcher.onRequest(A_RESOURCE_URL);
+  // simulate the response taking way too long to return
+  waitForResponse(TOTAL_TIMEOUT_DURATION * 2);
+  jest.advanceTimersByTime(TOTAL_TIMEOUT_DURATION * 2);
+
+  await expect(promise).rejects.toBeDefined();
+});
+
+it('Does not prematurely resolve if subsequent waterfall step requests take some time (but stay within the timeout time)', async () => {
+  const callback = jest.fn();
+  // Simulate a network waterfall with multiple steps
+  // ---HTML Document---
+  //                     ---Resource HTML Document requests---
+  //                                                           ---Resource Resource requests---
+  const watcher = new NetworkIdleWatcher();
+  // fire off an initial request
+  watcher.onRequest(A_PAGE_URL);
+  const promise = watcher.idle();
+
+  // For some reason, trying to assert on promise.resolves didn't work for the first assertion --
+  // The eventual resolution of the promise (later on) would retroactively fail the first assertion.
+  // Hence we're using a callback instead.
+  promise.then(() => {
+    callback();
+  });
+
+  watcher.onResponse(A_PAGE_URL);
+
+  await flushPromises();
+  // verify idle() hasn't been resolved yet
+  expect(callback).not.toHaveBeenCalled();
+
+  // send off another request and response
+  watcher.onRequest(A_RESOURCE_URL);
+
+  waitForResponse(WATERFALL_BETWEEN_STEPS_DURATION * 2);
+  jest.advanceTimersByTime(WATERFALL_BETWEEN_STEPS_DURATION * 2);
+  watcher.onResponse(A_RESOURCE_URL);
+  await flushPromises();
+  // we still shouldn't call the network idle yet
+  expect(callback).not.toHaveBeenCalled();
+
+  watcher.onRequest(ANOTHER_RESOURCE_URL);
+  waitForResponse(WATERFALL_BETWEEN_STEPS_DURATION * 2);
+  jest.advanceTimersByTime(WATERFALL_BETWEEN_STEPS_DURATION * 2);
+  watcher.onResponse(ANOTHER_RESOURCE_URL);
+
+  jest.runAllTimers();
+  await flushPromises();
+  // verify that idle() has now been resolved
+  expect(callback).toHaveBeenCalled();
+});