loadBundleFromServer test: remove legacy Jest timers, increased coverage and improved assertions (facebook#55863)

Summary:
This PR removes the dependency on Jest legacy fake timers from the `loadBundleFromServer` test suite.

Together with facebook#55757, this should remove the last usages of `jest.useFakeTimers({legacyFakeTimers: true});` in the codebase.

In this PR in particular, instead of migrating to modern  timers, I refactored the `addListener` mock to use a Promise based approach. By leveraging microtask scheduling, the tests simulate asynchronous functions flushed after the execution of the `sendRequest` without relying on timers.

In addition, I also improved the tests in other areas:

* improved readability by extracting a constant for the request id, used in both `sendRequest` and `addListener` mocks
* improved errors assertions to explicitly verify the errors are thrown (before the refactoring these tests could still be green if no error was thrown, because the expectations were only in the catch clause that could be skipped)
* improved the tests that verify the successful bundle load:
  * spit long test in ad hoc isolated ones
  * added listeners registration and cleanup verification to improve the coverage
  * added more fine-grained check for the `sendRequest` parameters

I tried to separate all the changes above in specific commit so that the review can check the incremental improvements I applied with the refactoring steps.

## Changelog:

[GENERAL] [CHANGED] - loadBundleFromServer test: remove legacy Jest timers, increased coverage and improved assertions

Pull Request resolved: facebook#55863

Test Plan:
* Ran loadBundleFromServer-test and verified all tests cases passed
* Ran the React Native test suite to ensure all tests pass
* Verified test correctness by intentionally breaking production code after fixing the tests (eg. remove observer/change listener code)

Reviewed By: cortinico

Differential Revision: D95034650

Pulled By: vzaidman

fbshipit-source-id: f6ae7cbc95ccfe606c1054c9044bf733e41bc2f9

Author: chicio

packages/react-native/Libraries/Core/Devtools/__tests__/loadBundleFromServer-test.js

@@ -10,8 +10,11 @@
 
 'use strict';
 
-// TODO(legacy-fake-timers): Fix these tests to work with modern timers.
-jest.useFakeTimers({legacyFakeTimers: true});
+import {
+  LoadBundleFromServerError,
+  LoadBundleFromServerRequestError,
+} from '../loadBundleFromServer';
+import invariant from 'invariant';
 
 jest.mock('../../../Utilities/HMRClient');
 
@@ -36,6 +39,8 @@ jest.mock('../../../Utilities/DevLoadingView', () => ({
   default: loadingViewMock,
 }));
 
+const REQUEST_ID = 1;
+
 const sendRequest = jest.fn(
   (
     method,
@@ -49,32 +54,31 @@ const sendRequest = jest.fn(
     callback,
     withCredentials,
   ) => {
-    callback(1);
+    callback(REQUEST_ID);
   },
 );
 
+const removeListener = jest.fn();
+const addListener = jest.fn((name, fn) => {
+  if (name === 'didReceiveNetworkData') {
+    void Promise.resolve().then(() => fn([REQUEST_ID, mockDataResponse]));
+  } else if (name === 'didCompleteNetworkResponse') {
+    void Promise.resolve().then(() => fn([REQUEST_ID, mockRequestError]));
+  } else if (name === 'didReceiveNetworkResponse') {
+    void Promise.resolve().then(() => fn([REQUEST_ID, null, mockHeaders]));
+  }
+  return {remove: removeListener};
+});
+
 jest.mock('../../../Network/RCTNetworking', () => ({
   __esModule: true,
   default: {
     sendRequest,
-    addListener: jest.fn((name, fn) => {
-      if (name === 'didReceiveNetworkData') {
-        setImmediate(() => fn([1, mockDataResponse]));
-      } else if (name === 'didCompleteNetworkResponse') {
-        setImmediate(() => fn([1, mockRequestError]));
-      } else if (name === 'didReceiveNetworkResponse') {
-        setImmediate(() => fn([1, null, mockHeaders]));
-      }
-      return {remove: () => {}};
-    }),
+    addListener,
   },
 }));
 
 let loadBundleFromServer: (bundlePathAndQuery: string) => Promise<void>;
-const {
-  LoadBundleFromServerError,
-  LoadBundleFromServerRequestError,
-} = require('../loadBundleFromServer');
 
 let mockHeaders: {'Content-Type': string};
 let mockDataResponse;
@@ -91,30 +95,36 @@ test('loadBundleFromServer will throw for JSON responses', async () => {
   mockDataResponse = JSON.stringify({message: 'Error thrown from Metro'});
   mockRequestError = null;
 
-  try {
-    await loadBundleFromServer('/Fail.bundle?platform=ios');
-  } catch (e) {
-    expect(e).toBeInstanceOf(LoadBundleFromServerError);
-    expect(e.message).toBe('Could not load bundle');
-    expect(e.cause).toBe('Error thrown from Metro');
-  }
+  const error: mixed = await loadBundleFromServer(
+    '/Fail.bundle?platform=ios',
+  ).catch(e => e);
+
+  invariant(
+    error instanceof LoadBundleFromServerError,
+    'Expected a LoadBundleFromServerError',
+  );
+  expect(error.message).toBe('Could not load bundle');
+  expect(error.cause).toBe('Error thrown from Metro');
 });
 
 test('loadBundleFromServer will throw LoadBundleFromServerError for request errors', async () => {
   mockHeaders = {'Content-Type': 'application/json'};
   mockDataResponse = '';
   mockRequestError = 'Some error';
 
-  try {
-    await loadBundleFromServer('/Fail.bundle?platform=ios');
-  } catch (e) {
-    expect(e).toBeInstanceOf(LoadBundleFromServerRequestError);
-    expect(e.message).toBe('Could not load bundle');
-    expect(e.cause).toBe('Some error');
-  }
+  const error: mixed = await loadBundleFromServer(
+    '/Fail.bundle?platform=ios',
+  ).catch(e => e);
+
+  invariant(
+    error instanceof LoadBundleFromServerRequestError,
+    'Expected a LoadBundleFromServerRequestError',
+  );
+  expect(error.message).toBe('Could not load bundle');
+  expect(error.cause).toBe('Some error');
 });
 
-test('loadBundleFromServer will request a bundle if import bundles are available', async () => {
+test('loadBundleFromServer successfully requests a bundle (root bundle)', async () => {
   mockDataResponse = '"code";';
   mockHeaders = {'Content-Type': 'application/javascript'};
   mockRequestError = null;
@@ -126,37 +136,48 @@ test('loadBundleFromServer will request a bundle if import bundles are available
   expect(sendRequest.mock.calls).toEqual([
     [
       'GET',
-      expect.anything(),
+      'asyncRequest',
       'localhost:8042/Banana.bundle?platform=ios&dev=true&minify=false&unusedExtraParam=42&modulesOnly=true&runModule=false',
-      expect.anything(),
-      expect.anything(),
-      expect.anything(),
-      expect.anything(),
-      expect.anything(),
-      expect.anything(),
-      expect.anything(),
+      {},
+      '',
+      'text',
+      true,
+      0,
+      expect.any(Function),
+      true,
     ],
   ]);
 
-  sendRequest.mockClear();
+  expect(addListener).toHaveBeenCalledTimes(4);
+  expect(removeListener).toHaveBeenCalledTimes(addListener.mock.calls.length);
+});
+
+test('loadBundleFromServer successfully requests a bundle (subdir bundle)', async () => {
+  mockDataResponse = '"code";';
+  mockHeaders = {'Content-Type': 'application/javascript'};
+  mockRequestError = null;
+
   await loadBundleFromServer(
     '/Tiny/Apple.bundle?platform=ios&dev=true&minify=false&unusedExtraParam=42&modulesOnly=true&runModule=false',
   );
 
   expect(sendRequest.mock.calls).toEqual([
     [
       'GET',
-      expect.anything(),
+      'asyncRequest',
       'localhost:8042/Tiny/Apple.bundle?platform=ios&dev=true&minify=false&unusedExtraParam=42&modulesOnly=true&runModule=false',
-      expect.anything(),
-      expect.anything(),
-      expect.anything(),
-      expect.anything(),
-      expect.anything(),
-      expect.anything(),
-      expect.anything(),
+      {},
+      '',
+      'text',
+      true,
+      0,
+      expect.any(Function),
+      true,
     ],
   ]);
+
+  expect(addListener).toHaveBeenCalledTimes(4);
+  expect(removeListener).toHaveBeenCalledTimes(addListener.mock.calls.length);
 });
 
 test('shows and hides the loading view around a request', async () => {