Repository: avajs/ava
Pull request #3151 (2022-12-19 22:45:55)

De-register all shared workers
This fixes the bug introduced in https://github.com/avajs/ava/pull/3149 that I briefly mentioned.

Since we were assigning the launched shared worker to a single variable per test worker, we inadvertently assumed that a test worker would launch a maximum of 1 shared worker.

I tested this patch with two of our internal codebases that heavily use shared workers and everything's working as expected now.

Sorry for the previous buggy PR!

novemberborn: Thanks for the follow-up @codetheweb! I haven't touched a keyboard much these past few weeks but hopefully I can find some time soon. Of course it's back to work so busy busy again 😄 
novemberborn: @codetheweb thoughts on the commit I pushed up?

Thanks for adding all these test cases, it's pretty hard to _think_ about the expected behavior.
codetheweb: Looks perfect to me! Much clearer on what's going on.

Patch:
diff --git a/lib/plugin-support/shared-workers.js b/lib/plugin-support/shared-workers.js
index 1a23fa23..2d65454a 100644
--- a/lib/plugin-support/shared-workers.js
+++ b/lib/plugin-support/shared-workers.js
@@ -49,40 +49,54 @@ function launchWorker(filename, initialData) {
 }
 
 export async function observeWorkerProcess(fork, runStatus) {
-	let registrationCount = 0;
-	let signalDeregistered;
-	let launched;
-	const deregistered = new Promise(resolve => {
-		signalDeregistered = () => {
-			// Only unref the worker once all test workers have been deregistered, otherwise the worker may exit before test workers are deregistered
-			launched?.worker.unref();
+	let signalDone;
+
+	const done = new Promise(resolve => {
+		signalDone = () => {
 			resolve();
 		};
 	});
 
-	fork.promise.finally(() => {
-		if (registrationCount === 0) {
-			signalDeregistered();
+	const activeInstances = new Set();
+
+	const removeInstance = instance => {
+		instance.worker.unref();
+		activeInstances.delete(instance);
+
+		if (activeInstances.size === 0) {
+			signalDone();
+		}
+	};
+
+	const removeAllInstances = () => {
+		if (activeInstances.size === 0) {
+			signalDone();
+			return;
 		}
+
+		for (const instance of activeInstances) {
+			removeInstance(instance);
+		}
+	};
+
+	fork.promise.finally(() => {
+		removeAllInstances();
 	});
 
 	fork.onConnectSharedWorker(async ({filename, initialData, port, signalError}) => {
-		launched = launchWorker(filename, initialData);
+		const launched = launchWorker(filename, initialData);
+		activeInstances.add(launched);
 
 		const handleWorkerMessage = async message => {
 			if (message.type === 'deregistered-test-worker' && message.id === fork.threadId) {
 				launched.worker.off('message', handleWorkerMessage);
-
-				registrationCount--;
-				if (registrationCount === 0) {
-					signalDeregistered();
-				}
+				removeInstance(launched);
 			}
 		};
 
 		launched.statePromises.error.then(error => {
-			signalDeregistered();
 			launched.worker.off('message', handleWorkerMessage);
+			removeAllInstances();
 			runStatus.emitStateChange({type: 'shared-worker-error', err: serializeError('Shared worker error', true, error)});
 			signalError();
 		});
@@ -90,8 +104,6 @@ export async function observeWorkerProcess(fork, runStatus) {
 		try {
 			await launched.statePromises.available;
 
-			registrationCount++;
-
 			port.postMessage({type: 'ready'});
 
 			launched.worker.postMessage({
@@ -112,5 +124,5 @@ export async function observeWorkerProcess(fork, runStatus) {
 		} catch {}
 	});
 
-	return deregistered;
+	return done;
 }


Test patch:
diff --git a/test/shared-workers/multiple-workers-are-loaded/fixtures/package.json b/test/shared-workers/multiple-workers-are-loaded/fixtures/package.json
new file mode 100644
index 00000000..54f67245
--- /dev/null
+++ b/test/shared-workers/multiple-workers-are-loaded/fixtures/package.json
@@ -0,0 +1,8 @@
+{
+	"type": "module",
+	"ava": {
+		"files": [
+			"*.js"
+		]
+	}
+}
diff --git a/test/shared-workers/multiple-workers-are-loaded/fixtures/test.js b/test/shared-workers/multiple-workers-are-loaded/fixtures/test.js
new file mode 100644
index 00000000..6cf895d7
--- /dev/null
+++ b/test/shared-workers/multiple-workers-are-loaded/fixtures/test.js
@@ -0,0 +1,30 @@
+import test from 'ava';
+import {registerSharedWorker} from 'ava/plugin';
+
+const worker1 = registerSharedWorker({
+	filename: new URL('worker.mjs#1', import.meta.url),
+	supportedProtocols: ['ava-4'],
+	initialData: {
+		id: '1',
+	},
+});
+
+const worker2 = registerSharedWorker({
+	filename: new URL('worker.mjs#2', import.meta.url),
+	supportedProtocols: ['ava-4'],
+	initialData: {
+		id: '2',
+	},
+});
+
+const messageFromWorker1 = worker1.subscribe().next();
+const messageFromWorker2 = worker2.subscribe().next();
+
+test('can load multiple workers', async t => {
+	const {value: {data: dataFromWorker1}} = await messageFromWorker1;
+	const {value: {data: dataFromWorker2}} = await messageFromWorker2;
+
+	t.deepEqual(dataFromWorker1, {id: '1'});
+	t.deepEqual(dataFromWorker2, {id: '2'});
+	t.pass();
+});
diff --git a/test/shared-workers/multiple-workers-are-loaded/fixtures/worker.mjs b/test/shared-workers/multiple-workers-are-loaded/fixtures/worker.mjs
new file mode 100644
index 00000000..1943f14c
--- /dev/null
+++ b/test/shared-workers/multiple-workers-are-loaded/fixtures/worker.mjs
@@ -0,0 +1,9 @@
+export default async ({negotiateProtocol}) => {
+	const protocol = negotiateProtocol(['ava-4']);
+
+	await protocol.ready();
+
+	for await (const testWorker of protocol.testWorkers()) {
+		testWorker.publish(protocol.initialData);
+	}
+};
diff --git a/test/shared-workers/multiple-workers-are-loaded/test.js b/test/shared-workers/multiple-workers-are-loaded/test.js
new file mode 100644
index 00000000..a15ff6a2
--- /dev/null
+++ b/test/shared-workers/multiple-workers-are-loaded/test.js
@@ -0,0 +1,9 @@
+import test from '@ava/test';
+
+import {fixture} from '../../helpers/exec.js';
+
+test('can load multiple workers', async t => {
+	await fixture();
+
+	t.pass();
+});