From 57f0003b963da57ee27a6f50397272095487334a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=8E?= Date: Thu, 26 Oct 2023 22:17:09 +0800 Subject: [PATCH 1/4] feat(runner): minimize path pollution --- src/client/runner.ts | 23 +++++++++++++---------- src/server/BrowserLogger.ts | 12 ++++++++---- src/server/Runner.ts | 11 +++++------ 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/client/runner.ts b/src/client/runner.ts index aedcd3e..aa22439 100644 --- a/src/client/runner.ts +++ b/src/client/runner.ts @@ -21,35 +21,38 @@ export const inject = (uuid: string) => ( new Promise((resolve) => { const script = document.createElement('script'); + // Avoid test interfering + // eslint-disable-next-line no-console + const consoleError = console.error; + // Detect inject error script.addEventListener('error', () => { - // eslint-disable-next-line no-console - console.error('Failed to inject test script'); + consoleError('Failed to inject test script'); resolve(1); }, { once: true }); // Detect init error - const onUncaughtError = () => { - // eslint-disable-next-line no-console - console.error('Uncaught error detected while initializing the tests'); + const initErrorListenerAbortController = new AbortController(); + window.addEventListener('error', () => { + consoleError('Uncaught error detected while initializing the tests'); resolve(1); - }; - window.addEventListener('error', onUncaughtError, { once: true }); + }, { once: true, signal: initErrorListenerAbortController.signal }); // Detect init end window.addEventListener(`__wrightplay_${uuid}_init__`, () => { - window.removeEventListener('error', onUncaughtError); + initErrorListenerAbortController.abort(); }, { once: true }); // Detect test done window.addEventListener(`__wrightplay_${uuid}_done__`, ({ exitCode }) => { - window.removeEventListener('error', onUncaughtError); + initErrorListenerAbortController.abort(); resolve(exitCode); }, { once: true }); // Inject - script.src = '/stdin.js'; + script.src = '/__wrightplay__/stdin.js'; script.type = 'module'; document.head.append(script); + document.head.removeChild(script); }) ); diff --git a/src/server/BrowserLogger.ts b/src/server/BrowserLogger.ts index aea40c7..d7aefb5 100644 --- a/src/server/BrowserLogger.ts +++ b/src/server/BrowserLogger.ts @@ -174,10 +174,14 @@ export default class BrowserLogger { originalColumn, } = sourceMap.findEntry(+line - 1, +column - 1); - // The type of `findEntry` is loose. It may return {}. - return originalSource !== undefined - ? `${pathToFileURL(path.resolve(cwd, originalSource)).href}:${originalLine + 1}:${originalColumn + 1}` - : original; + // The return type of `findEntry` is inaccurate. It may return {}. + if (originalSource === undefined) { + return original; + } + + const baseDir = path.join(cwd, path.dirname(pathname)); + const originalSourcePath = path.resolve(baseDir, originalSource); + return `${pathToFileURL(originalSourcePath).href}:${originalLine + 1}:${originalColumn + 1}`; }); } diff --git a/src/server/Runner.ts b/src/server/Runner.ts index f262e89..3877d94 100644 --- a/src/server/Runner.ts +++ b/src/server/Runner.ts @@ -195,7 +195,7 @@ export default class Runner implements Disposable { private updateBuiltFiles(files: esbuild.OutputFile[]) { const { cwd, fileContents, sourceMapPayloads } = this; return files.reduce((changed, { path: absPath, hash, text }) => { - const pathname = `/${path.relative(cwd, absPath)}`; + const pathname = `/${path.relative(cwd, absPath).replace(/\\/g, '/')}`; // Skip unchanged files. const same = fileContents.get(pathname)?.hash === hash; @@ -241,7 +241,7 @@ export default class Runner implements Disposable { // The stdin API doesn't support onLoad callbacks, // so we use the entry point workaround. // https://github.com/evanw/esbuild/issues/720 - stdin: '', + '__wrightplay__/stdin': '', }, metafile: watch, bundle: true, @@ -254,8 +254,8 @@ export default class Runner implements Disposable { { name: 'import files loader', setup: (pluginBuild) => { - pluginBuild.onResolve({ filter: /^$/ }, () => ({ path: 'stdin', namespace: 'stdin' })); - pluginBuild.onLoad({ filter: /^/, namespace: 'stdin' }, async () => { + pluginBuild.onResolve({ filter: /^$/ }, () => ({ path: 'stdin', namespace: 'wrightplay' })); + pluginBuild.onLoad({ filter: /^/, namespace: 'wrightplay' }, async () => { const importFiles = await testFinder.getFiles(); if (setupFile) importFiles.unshift(setupFile.replace(/\\/g, '\\\\')); if (importFiles.length === 0) { @@ -349,8 +349,7 @@ export default class Runner implements Disposable { } const esbuildListener: http.RequestListener = (request, response) => { - const { url } = request as typeof request & { url: string }; - const pathname = url.split(/[?#]/, 1)[0]; + const { pathname } = new URL(request.url!, `http://${request.headers.host}`); const handleRequest = () => { const builtContent = fileContents.get(pathname); From 81a007d629068ecf2da5982c0585cfce64f0f54d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=8E?= Date: Fri, 27 Oct 2023 19:19:17 +0800 Subject: [PATCH 2/4] docs: suggest full trace for mocha --- README.md | 5 +++-- test/default.setup.ts | 5 +++-- test/third-party/mocha/setup.ts | 5 +++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index fcb899e..9b4920f 100644 --- a/README.md +++ b/README.md @@ -415,9 +415,10 @@ import 'mocha'; import { onInit, done } from 'wrightplay'; mocha.setup({ - ui: 'bdd', - reporter: 'spec', color: true, + fullTrace: true, + reporter: 'spec', + ui: 'bdd', }); onInit(() => { diff --git a/test/default.setup.ts b/test/default.setup.ts index b734385..678d227 100644 --- a/test/default.setup.ts +++ b/test/default.setup.ts @@ -14,9 +14,10 @@ const { const { onInit, done } = await import('../src/client/api.js'); mocha.setup({ - ui: 'bdd', - reporter: 'spec', color: true, + fullTrace: true, + reporter: 'spec', + ui: 'bdd', }); onInit(() => { diff --git a/test/third-party/mocha/setup.ts b/test/third-party/mocha/setup.ts index 56bb1a8..7e8bc2a 100644 --- a/test/third-party/mocha/setup.ts +++ b/test/third-party/mocha/setup.ts @@ -2,9 +2,10 @@ import 'mocha'; import { onInit, done } from 'wrightplay'; mocha.setup({ - ui: 'bdd', - reporter: 'spec', color: true, + fullTrace: true, + reporter: 'spec', + ui: 'bdd', }); onInit(() => { From 47ea7108053814da28aa354665039f679fc3d8f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=8E?= Date: Fri, 27 Oct 2023 20:23:50 +0800 Subject: [PATCH 3/4] fix(runner): discard log errors by auto rerun calls --- src/server/BrowserLogger.ts | 7 +++++++ src/server/Runner.ts | 41 +++++++++++++++++++++++++++++-------- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/server/BrowserLogger.ts b/src/server/BrowserLogger.ts index d7aefb5..5065a73 100644 --- a/src/server/BrowserLogger.ts +++ b/src/server/BrowserLogger.ts @@ -187,6 +187,13 @@ export default class BrowserLogger { lastPrint: Promise = Promise.resolve(); + /** + * Discard the last print error. + */ + discardLastPrintError() { + this.lastPrint = this.lastPrint.catch(() => {}); + } + /** * Print messages to console with specified log level and color. */ diff --git a/src/server/Runner.ts b/src/server/Runner.ts index 3877d94..47dc6f3 100644 --- a/src/server/Runner.ts +++ b/src/server/Runner.ts @@ -459,20 +459,43 @@ export default class Runner implements Disposable { const wsServer = new WSServer(this.uuid, fileServer, page); const run = async () => { - await wsServer.reset(); - return page.evaluate(clientRunner.inject, this.uuid) - .catch((error) => { - // eslint-disable-next-line no-console - console.error(error); - return 1; - }); + // Listen to the file change event during the test run to + // ignore the evaluate error caused by automatic test reruns. + let fileChanged = false; + const fileChangeListener = () => { fileChanged = true; }; + fileServer.once('wrightplay:changed', fileChangeListener); + + try { + await wsServer.reset(); + return await page.evaluate(clientRunner.inject, this.uuid); + } catch (error) { + // Skip the error print if the file has changed. + // eslint-disable-next-line no-console + if (!fileChanged) console.error(error); + return 1; + } finally { + // Remove the listener to avoid potential memory leak. + fileServer.off('wrightplay:changed', fileChangeListener); + } }; - await page.goto('/'); + await page.goto('/', { waitUntil: 'domcontentloaded' }); // Rerun the tests on file changes. fileServer.on('wrightplay:changed', () => { - page.reload().catch(() => { + (async () => { + // Discard the print error on navigation. + page.off('console', bLog.forwardConsole); + page.off('pageerror', bLog.forwardError); + bLog.discardLastPrintError(); + + // Reload the page to rerun the tests. + await page.reload({ waitUntil: 'commit' }); + + // Restore the print forwarding. + page.on('console', bLog.forwardConsole); + page.on('pageerror', bLog.forwardError); + })().catch(() => { // eslint-disable-next-line no-console console.error('Failed to rerun the tests after file changes'); }); From 86ed5b50cb4573a1af0dde4333299cbf01c2674d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=8E?= Date: Fri, 27 Oct 2023 20:25:04 +0800 Subject: [PATCH 4/4] fix(runner): sort test imports to make the output stable --- src/server/Runner.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/server/Runner.ts b/src/server/Runner.ts index 47dc6f3..d107dac 100644 --- a/src/server/Runner.ts +++ b/src/server/Runner.ts @@ -256,8 +256,13 @@ export default class Runner implements Disposable { setup: (pluginBuild) => { pluginBuild.onResolve({ filter: /^$/ }, () => ({ path: 'stdin', namespace: 'wrightplay' })); pluginBuild.onLoad({ filter: /^/, namespace: 'wrightplay' }, async () => { + // Sort to make the output stable const importFiles = await testFinder.getFiles(); + importFiles.sort(); + + // Prepend the setup file if any if (setupFile) importFiles.unshift(setupFile.replace(/\\/g, '\\\\')); + if (importFiles.length === 0) { if (watch) { // eslint-disable-next-line no-console @@ -266,6 +271,7 @@ export default class Runner implements Disposable { throw new Error('No test file found'); } } + const importStatements = importFiles.map((file) => `import '${file}'`).join('\n'); return { contents: `${importStatements}\n(${clientRunner.init.toString()})('${this.uuid}')`,