fix: worker being killed after being spawned and other worker bugs

packages/jest-worker/src/types.ts

@@ -190,12 +190,12 @@ export type WorkerOptions = {
    */
   on?: {
     [WorkerEvents.STATE_CHANGE]:
-      | onStateChangeHandler
-      | Array<onStateChangeHandler>;
+      | OnStateChangeHandler
+      | ReadonlyArray<OnStateChangeHandler>;
   };
 };
 
-export type onStateChangeHandler = (
+export type OnStateChangeHandler = (
   state: WorkerStates,
   oldState: WorkerStates,
 ) => void;

packages/jest-worker/src/workers/ChildProcessWorker.ts

@@ -117,6 +117,11 @@ export default class ChildProcessWorker
     const silent = this._options.silent ?? true;
 
     if (!silent) {
+      // NOTE: Detecting an out of memory crash is independent of idle memory usage monitoring. We want to
+      // monitor for a crash occurring so that it can be handled as required and so we can tell the difference
+      // between an OOM crash and another kind of crash. We need to do this because if a worker crashes due to
+      // an OOM event sometimes it isn't seen by the worker pool and it just sits there waiting for the worker
+      // to respond and it never will.
       console.warn('Unable to detect out of memory event if silent === false');
     }
 

packages/jest-worker/src/workers/WorkerAbstract.ts

@@ -17,7 +17,11 @@ export default abstract class WorkerAbstract
   extends EventEmitter
   implements Pick<WorkerInterface, 'waitForWorkerReady' | 'state'>
 {
-  private __state = WorkerStates.STARTING;
+  /**
+   * DO NOT WRITE TO THIS DIRECTLY.
+   * Use this.state getter/setters so events are emitted correctly.
+   */
+  #state = WorkerStates.STARTING;
 
   protected _fakeStream: PassThrough | null = null;
 
@@ -28,12 +32,12 @@ export default abstract class WorkerAbstract
   protected _resolveWorkerReady: (() => void) | undefined;
 
   public get state(): WorkerStates {
-    return this.__state;
+    return this.#state;
   }
   protected set state(value: WorkerStates) {
-    if (this.__state !== value) {
-      const oldState = this.__state;
-      this.__state = value;
+    if (this.#state !== value) {
+      const oldState = this.#state;
+      this.#state = value;
 
       this.emit(WorkerEvents.STATE_CHANGE, value, oldState);
     }
@@ -44,12 +48,14 @@ export default abstract class WorkerAbstract
 
     if (typeof options.on === 'object') {
       for (const [event, handlers] of Object.entries(options.on)) {
-        if (Array.isArray(handlers)) {
+        // Can't do Array.isArray on a ReadonlyArray<T>.
+        // https://github.com/microsoft/TypeScript/issues/17002
+        if (typeof handlers === 'function') {
+          super.on(event, handlers);
+        } else {
           for (const handler of handlers) {
             super.on(event, handler);
           }
-        } else {
-          super.on(event, handlers);
         }
       }
     }

packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.js

@@ -19,7 +19,6 @@ import {
 import ChildProcessWorker, {SIGKILL_DELAY} from '../ChildProcessWorker';
 import ThreadsWorker from '../NodeThreadsWorker';
 
-jest.retryTimes(0);
 jest.setTimeout(10000);
 
 const root = join('../../');
@@ -58,6 +57,15 @@ test.each(filesToBuild)('%s.js should exist', async file => {
   await expect(async () => await access(path)).not.toThrowError();
 });
 
+async function closeWorkerAfter(worker, testBody) {
+  try {
+    await testBody(worker);
+  } finally {
+    worker.forceExit();
+    await worker.waitForExit();
+  }
+}
+
 describe.each([
   {
     name: 'ProcessWorker',
@@ -101,62 +109,50 @@ describe.each([
   }
 
   test('should get memory usage', async () => {
-    let worker;
-
-    try {
-      worker = new workerClass({
+    await closeWorkerAfter(
+      new workerClass({
         childWorkerPath: workerPath,
         maxRetries: 0,
         workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'),
-      });
+      }),
+      async worker => {
+        const memoryUsagePromise = worker.getMemoryUsage();
+        expect(memoryUsagePromise).toBeInstanceOf(Promise);
 
-      const memoryUsagePromise = worker.getMemoryUsage();
-      expect(memoryUsagePromise).toBeInstanceOf(Promise);
-
-      expect(await memoryUsagePromise).toBeGreaterThan(0);
-    } finally {
-      if (worker) {
-        worker.forceExit();
-        await worker.waitForExit();
-      }
-    }
+        expect(await memoryUsagePromise).toBeGreaterThan(0);
+      },
+    );
   });
 
   test('should recycle on idle limit breach', async () => {
-    let worker;
-
-    try {
-      worker = new workerClass({
+    await closeWorkerAfter(
+      new workerClass({
         childWorkerPath: workerPath,
         // There is no way this is fitting into 1000 bytes, so it should restart
         // after requesting a memory usage update
         idleMemoryLimit: 1000,
         maxRetries: 0,
         workerPath: join(__dirname, '__fixtures__', 'EdgeCasesWorker'),
-      });
-
-      const startSystemId = worker.getWorkerSystemId();
-      expect(startSystemId).toBeGreaterThanOrEqual(0);
+      }),
+      async worker => {
+        const startSystemId = worker.getWorkerSystemId();
+        expect(startSystemId).toBeGreaterThanOrEqual(0);
 
-      worker.checkMemoryUsage();
+        worker.checkMemoryUsage();
 
-      await waitForChange(() => worker.getWorkerSystemId());
+        await waitForChange(() => worker.getWorkerSystemId());
 
-      const systemId = worker.getWorkerSystemId();
-      expect(systemId).toBeGreaterThanOrEqual(0);
-      expect(systemId).not.toEqual(startSystemId);
+        const systemId = worker.getWorkerSystemId();
+        expect(systemId).toBeGreaterThanOrEqual(0);
+        expect(systemId).not.toEqual(startSystemId);
 
-      await new Promise(resolve => {
-        setTimeout(resolve, SIGKILL_DELAY + 100);
-      });
+        await new Promise(resolve => {
+          setTimeout(resolve, SIGKILL_DELAY + 100);
+        });
 
-      expect(worker.isWorkerRunning()).toBeTruthy();
-    } finally {
-      if (worker) {
-        worker.forceExit();
-        await worker.waitForExit();
-      }
-    }
+        expect(worker.isWorkerRunning()).toBeTruthy();
+      },
+    );
   });
 
   describe('should automatically recycle on idle limit breach', () => {