fix: improve worker tests (#1757)

Author: robertsLando

src/lib/PingTimer.ts

@@ -8,9 +8,15 @@ export default class PingTimer {
 
 	private checkPing: () => void
 
-	private setTimeout = isBrowser ? setT : setTimeout
+	// dont directly assign globals to class props otherwise this throws in web workers: Uncaught TypeError: Illegal invocation
+	// See: https://stackoverflow.com/questions/9677985/uncaught-typeerror-illegal-invocation-in-chrome
+	private _setTimeout: typeof setT =
+		isBrowser && !isWebWorker
+			? setT
+			: (func, time) => setTimeout(func, time)
 
-	private clearTimeout = isBrowser ? clearT : clearTimeout
+	private _clearTimeout: typeof clearT =
+		isBrowser && !isWebWorker ? clearT : (timer) => clearTimeout(timer)
 
 	constructor(keepalive: number, checkPing: () => void) {
 		this.keepalive = keepalive * 1000
@@ -19,15 +25,15 @@ export default class PingTimer {
 	}
 
 	private setup() {
-		this.timer = this.setTimeout(() => {
+		this.timer = this._setTimeout(() => {
 			this.checkPing()
 			this.reschedule()
 		}, this.keepalive)
 	}
 
 	clear() {
 		if (this.timer) {
-			this.clearTimeout(this.timer)
+			this._clearTimeout(this.timer)
 			this.timer = null
 		}
 	}

test/browser/test.js

@@ -2,16 +2,14 @@ import { expect } from '@esm-bundle/chai';
 import mqtt from '../../'; // this will resolve to mqtt/dist/mqtt.esm.js
 
 // needed to test no-esm version /dist/mqtt.js
-/** @type { import('../../src').MqttClient }*/
+/** @type { import('../../src') }*/
 const mqtt2 = window.mqtt
 
 // get browser name
 const userAgent = navigator.userAgent.toLowerCase().replace(/ /g, '_').replace(/\//g, '_')
 
 let browser = 'unknown'
 
-console.log('userAgent:', userAgent)
-
 if (userAgent.includes('chrome')) {
 	browser = 'chrome'
 } else if (userAgent.includes('firefox')) {
@@ -21,40 +19,50 @@ if (userAgent.includes('chrome')) {
 }
 
 const browserTopic = `test/${browser}`
+console.log('User Agent:', userAgent)
+console.log('Browser:', browser)
 
-console.log('browser:', browser)
-
-function run(proto, port, cb) {
-
+function testProto(proto, port, cb = () => { }) {
 	const testTopic = `${browserTopic}/${proto}`
 
 	describe('MQTT.js browser test with ' + proto.toUpperCase(), () => {
 		after(() => {
-			if (cb) {
+			if (client) {
+				client.end(() => {
+					cb()
+					client = null;
+				});
+			} else {
 				cb()
 			}
 		})
 
-		const client = mqtt.connect(`${proto}://localhost:${port}`, {
-			// log: console.log.bind(console),
-		})
-		client.on('offline', () => {
-			console.log('client offline')
-		})
-		client.on('connect', () => {
-			console.log('client connect')
-		})
-		client.on('reconnect', () => {
-			console.log('client reconnect')
-		})
+		/** @type { import('../../src').MqttClient }*/
+		let client = null;
 
 		it('should connect-publish-subscribe', (done) => {
+			client = mqtt.connect(`${proto}://localhost:${port}`, {
+				// log: console.log.bind(console),
+				clientId: `testClient-${browser}-${proto}`,
+			})
+			client.on('offline', () => {
+				console.log('client offline')
+				done(new Error('client offline'))
+			})
+			client.on('connect', () => {
+				console.log('client connect')
+			})
+			client.on('reconnect', () => {
+				console.log('client reconnect')
+			})
+
 			const payload = 'Hello World!'
 			client.on('connect', () => {
 				client.on('message', (topic, msg) => {
 					expect(topic).to.equal(testTopic);
 					expect(msg.toString()).to.equal(payload);
 					client.end(() => {
+						client = null;
 						done();
 					});
 				});
@@ -76,30 +84,32 @@ function run(proto, port, cb) {
 	})
 }
 
-it('should work with non-ESM version', () => {
-	expect(mqtt2).to.exist
-	expect(mqtt2.connect).to.exist
-	expect(mqtt2.connect).to.be.a('function')
-})
+describe('MQTT.js browser tests', () => {
+	it('should work with ESM version', (done) => {
+		expect(mqtt2).to.exist
+		expect(mqtt2.connect).to.be.a('function')
+		expect(mqtt2.Client).to.be.a('function')
+		done()
+	})
 
+	it('should work in a Web Worker', (done) => {
+		const worker = new Worker('test/browser/worker.js')
+		worker.onmessage = (e) => {
+			if (e.data === 'worker ready') {
+				done()
+			} else {
+				done(Error(e.data))
+			}
+		}
 
-run('ws', window.wsPort, () => {
-	run('wss', window.wssPort, () => {
-		describe('MQTT.js browser test with web worker', () => {
-			it('should work with web worker', async () => {
-				const worker = new Worker('test/browser/worker.js')
-				const ready = new Promise((resolve, reject) => {
-					worker.onmessage = (e) => {
-						if (e.data === 'worker ready') {
-							resolve()
-						} else {
-							reject(e.data)
-						}
-					}
-				})
-				await ready
-			})
-		})
+		worker.onerror = (e) => {
+			done(Error(e.message))
+		}
+	})
+
+	testProto('ws', window.wsPort, () => {
+		testProto('wss', window.wssPort)
 	})
 })
 
+

test/browser/worker.js

@@ -1,8 +1,28 @@
-// test mqttjs in worker
-
 importScripts('/dist/mqtt.js');
 
 /** @type { import('../../src').MqttClient }*/
 const MQTT = mqtt;
 
-postMessage(typeof MQTT?.connect === 'function' ? 'worker ready' : 'worker error');
+const client = MQTT.connect(`ws://localhost:4000`, {
+    clientId: `testClient-worker_` + Math.random().toString(16).substr(2, 8),
+});
+
+client.on('offline', () => {
+    console.log('worker client offline');
+})
+
+client.on('reconnect', () => {
+    console.log('worker client reconnect');
+})
+
+client.on('error', (err) => {
+    console.log('worker client error', err);
+})
+
+client.on('connect', () => {
+    console.log('worker client connect');
+    client.end(() => {
+        console.log('worker client end');
+        postMessage('worker ready');
+    });
+})

web-test-runner.config.mjs

@@ -11,6 +11,8 @@ await start({
     wssPort,
     key: './test/certs/server-key.pem',
     cert: './test/certs/server-cert.pem',
+    verbose: true,
+    stats: false
 })
 
 console.log('Broker setup done')