From 18ed8c57d67305173e4018f51050482a86082c42 Mon Sep 17 00:00:00 2001 From: Andrew Herron Date: Mon, 27 Jan 2025 11:14:27 +1000 Subject: [PATCH] TINY-11177: Vastly improve remote testing (#145) Co-authored-by: Seb --- CHANGELOG.md | 16 + Jenkinsfile | 2 +- lerna.json | 2 +- modules/client/package.json | 4 +- modules/client/src/main/ts/api/UnitTest.ts | 22 +- modules/client/tsconfig.json | 6 +- modules/common/package.json | 2 +- modules/common/tsconfig.json | 7 +- modules/runner/package.json | 9 +- .../{rollup.config.js => rollup.config.mjs} | 0 modules/runner/src/main/ts/api/Main.ts | 15 +- modules/runner/src/main/ts/core/Globals.ts | 5 +- modules/runner/src/main/ts/core/TestLoader.ts | 3 +- modules/runner/src/main/ts/core/Utils.ts | 7 +- .../runner/src/main/ts/reporter/Callbacks.ts | 50 +- .../runner/src/main/ts/reporter/Reporter.ts | 153 +- modules/runner/src/main/ts/runner/Hooks.ts | 3 +- modules/runner/src/main/ts/runner/Run.ts | 3 +- modules/runner/src/main/ts/runner/Runner.ts | 57 +- modules/runner/src/main/ts/runner/TestRun.ts | 79 +- modules/runner/src/main/ts/runner/Utils.ts | 3 +- modules/runner/src/main/ts/ui/Ui.ts | 11 +- modules/runner/src/test/ts/TestUtils.ts | 4 +- modules/runner/src/test/ts/core/UtilsTest.ts | 4 +- .../src/test/ts/reporter/ReporterTest.ts | 112 +- .../src/test/ts/runner/RunnerTestUtils.ts | 9 +- .../runner/src/test/ts/runner/TestRunTest.ts | 3 +- .../runner/src/test/ts/runner/UtilsTest.ts | 3 +- modules/runner/tsconfig.json | 6 +- modules/sample/package.json | 17 +- .../src/test/ts/client/fail/AsyncFailTest.ts | 1 - .../src/test/ts/client/pass/AsyncPassTest.ts | 1 - modules/sample/src/test/ts/utils/Utils.ts | 3 +- modules/sample/tsconfig.json | 9 +- modules/server/package.json | 14 +- modules/server/src/main/ts/BedrockAuto.ts | 13 +- modules/server/src/main/ts/BedrockCli.ts | 4 + .../server/src/main/ts/bedrock/auto/Driver.ts | 23 +- .../src/main/ts/bedrock/auto/RemoteDriver.ts | 5 +- .../src/main/ts/bedrock/cli/ClOptions.ts | 2 +- modules/server/src/main/ts/bedrock/cli/Hud.ts | 11 +- .../src/main/ts/bedrock/compiler/Webpack.ts | 1 - .../src/main/ts/bedrock/core/Lifecycle.ts | 17 +- .../server/src/main/ts/bedrock/server/Apis.ts | 40 +- .../src/main/ts/bedrock/server/Controller.ts | 104 +- .../main/ts/bedrock/server/CustomRoutes.ts | 18 +- .../src/main/ts/bedrock/server/EffectUtils.ts | 8 +- .../src/main/ts/bedrock/server/Routes.ts | 3 +- .../src/main/ts/bedrock/server/Serve.ts | 6 +- .../server/src/resources/html/bedrock.html | 1 + .../src/test/resources/tsconfig.sample.json | 4 +- modules/server/src/test/ts/ClisTest.ts | 16 +- modules/server/tasks/bedrock.js | 4 +- modules/server/tsconfig.json | 4 +- yarn.lock | 3036 ++++++++++------- 55 files changed, 2401 insertions(+), 1564 deletions(-) rename modules/runner/{rollup.config.js => rollup.config.mjs} (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95758e75..cec1ee4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,22 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +## Added +- New server-side APIs to accept a batch of results instead of a single result #TINY-11177 + +## Changed +- Reverted TINY-10708 which was a server-side fix +- Client no longer waits for log requests to complete between tests, which should speed up remote testing #TINY-11177 +- Console HUD no longer updates for individual tests #TINY-11177 +- Client now posts test status only in batches every 30 seconds, this is the only time the console HUD will update #TINY-11177 + +## Removed +- Single result server-side API #TINY-11177 +- Server-side monitoring of single test timeouts. This is still monitored client side. #TINY-11177 +- The Promise polyfill is no longer allowed on modern NodeJS frameworks so it has been removed. #TINY-11177 + ## 14.1.5 - 2024-10-18 ### Added diff --git a/Jenkinsfile b/Jenkinsfile index 927f9cd1..e2f89efd 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -40,7 +40,7 @@ timestamps { sh 'yarn build' }, testDirs: [ 'modules/sample/src/test/ts/**/pass' ], - custom: '--config modules/sample/tsconfig.json --customRoutes modules/sample/routes.json --polyfills Promise Symbol' + custom: '--config modules/sample/tsconfig.json --customRoutes modules/sample/routes.json' ) } diff --git a/lerna.json b/lerna.json index 8225ab5f..3a3a0b54 100644 --- a/lerna.json +++ b/lerna.json @@ -1,7 +1,7 @@ { "npmClient": "yarn", "useWorkspaces": true, - "version": "14.1.5", + "version": "15.0.0-alpha.10", "publish": { "push": false } diff --git a/modules/client/package.json b/modules/client/package.json index 0abc5443..17d948e5 100644 --- a/modules/client/package.json +++ b/modules/client/package.json @@ -1,6 +1,6 @@ { "name": "@ephox/bedrock-client", - "version": "14.1.1", + "version": "15.0.0-alpha.10", "author": "Tiny Technologies Inc", "license": "Apache-2.0", "scripts": { @@ -10,7 +10,7 @@ "buildAndTest": "yarn prepublishOnly && yarn test" }, "dependencies": { - "@ephox/bedrock-common": "^14.1.1", + "@ephox/bedrock-common": "^15.0.0-alpha.7", "@ephox/dispute": "^1.0.3" }, "main": "./lib/main/ts/api/Main.js", diff --git a/modules/client/src/main/ts/api/UnitTest.ts b/modules/client/src/main/ts/api/UnitTest.ts index dbff7b23..e57dc776 100644 --- a/modules/client/src/main/ts/api/UnitTest.ts +++ b/modules/client/src/main/ts/api/UnitTest.ts @@ -1,5 +1,5 @@ import { Context, Failure, TestError, TestLabel, TestLogs } from '@ephox/bedrock-common'; -import { it } from './Bdd'; +import { describe, it } from './Bdd'; type TestLogs = TestLogs.TestLogs; type TestError = TestError.TestError; @@ -10,11 +10,13 @@ export type FailureCallback = (error: TestThrowable, logs?: TestLogs) => void; /** An asynchronous test with callbacks. */ export const asyncTest = (name: string, test: (this: Context, success: SuccessCallback, failure: FailureCallback) => void): void => { - it(name, function (done) { - test.call(this, () => done(), ((err, logs) => { - const r = Failure.prepFailure(err, logs); - done(r); - })); + describe('old-style test', () => { + it(name, function (done) { + test.call(this, () => done(), ((err, logs) => { + const r = Failure.prepFailure(err, logs); + done(r); + })); + }); }); }; @@ -23,10 +25,14 @@ export const asynctest = asyncTest; /** A synchronous test that fails if an exception is thrown */ export const test = (name: string, test: (this: Context) => void): void => { - it(name, test); + describe('old-style test', () => { + it(name, test); + }); }; /** Tests an async function (function that returns a Promise). */ export const promiseTest = (name: string, test: (this: Context) => Promise): void => { - it(name, test); + describe('old-style test', () => { + it(name, test); + }); }; diff --git a/modules/client/tsconfig.json b/modules/client/tsconfig.json index 39271dc8..fe987120 100644 --- a/modules/client/tsconfig.json +++ b/modules/client/tsconfig.json @@ -4,9 +4,9 @@ "strict": true, "noUnusedLocals": true, "useUnknownInCatchVariables": false, - "target": "es5", - "module": "es2015", - "lib": ["es2015", "dom"], + "target": "es2019", + "module": "es2020", + "lib": ["es2022", "dom"], "declaration": true, "sourceMap": true, "outDir": "lib", diff --git a/modules/common/package.json b/modules/common/package.json index 227f1bf2..60e504f9 100644 --- a/modules/common/package.json +++ b/modules/common/package.json @@ -1,6 +1,6 @@ { "name": "@ephox/bedrock-common", - "version": "14.1.1", + "version": "15.0.0-alpha.7", "author": "Tiny Technologies Inc", "license": "Apache-2.0", "scripts": { diff --git a/modules/common/tsconfig.json b/modules/common/tsconfig.json index 39271dc8..48d032e4 100644 --- a/modules/common/tsconfig.json +++ b/modules/common/tsconfig.json @@ -4,12 +4,13 @@ "strict": true, "noUnusedLocals": true, "useUnknownInCatchVariables": false, - "target": "es5", - "module": "es2015", - "lib": ["es2015", "dom"], + "target": "es2019", + "module": "es2020", + "lib": ["es2022", "dom"], "declaration": true, "sourceMap": true, "outDir": "lib", + "rootDir": "src", "types": [] }, diff --git a/modules/runner/package.json b/modules/runner/package.json index 2f0a708b..4cd1b7d3 100644 --- a/modules/runner/package.json +++ b/modules/runner/package.json @@ -1,6 +1,6 @@ { "name": "@ephox/bedrock-runner", - "version": "14.1.1", + "version": "15.0.0-alpha.10", "author": "Tiny Technologies Inc", "license": "Apache-2.0", "scripts": { @@ -10,8 +10,7 @@ "buildAndTest": "yarn prepublishOnly && yarn test" }, "dependencies": { - "@ephox/bedrock-common": "^14.1.1", - "@ephox/wrap-promise-polyfill": "^2.2.0", + "@ephox/bedrock-common": "^15.0.0-alpha.7", "jquery": "^3.4.1", "querystringify": "^2.1.1" }, @@ -19,10 +18,10 @@ "@types/diff": "^5.0.0", "@types/jquery": "^3.5.3", "@types/querystringify": "^2.0.0", - "rollup": "^1.20.2", + "rollup": "^4.30.1", "rollup-plugin-commonjs": "^10.1.0", "rollup-plugin-node-resolve": "^5.2.0", - "rollup-plugin-sourcemaps": "^0.4.2", + "rollup-plugin-sourcemaps": "^0.6.3", "sourcemapped-stacktrace": "^1.1.11" }, "main": "./lib/main/ts/api/Main.js", diff --git a/modules/runner/rollup.config.js b/modules/runner/rollup.config.mjs similarity index 100% rename from modules/runner/rollup.config.js rename to modules/runner/rollup.config.mjs diff --git a/modules/runner/src/main/ts/api/Main.ts b/modules/runner/src/main/ts/api/Main.ts index 667f8296..0d47a570 100644 --- a/modules/runner/src/main/ts/api/Main.ts +++ b/modules/runner/src/main/ts/api/Main.ts @@ -1,5 +1,4 @@ import { Failure, Global } from '@ephox/bedrock-common'; -import Promise from '@ephox/wrap-promise-polyfill'; import * as Globals from '../core/Globals'; import * as TestLoader from '../core/TestLoader'; import { UrlParams } from '../core/UrlParams'; @@ -23,16 +22,16 @@ const setupAndRun = (loadError?: Error) => { const runner = Runner(Globals.rootSuite(), params, callbacks, reporter, ui); runner.init().then((data) => { - if (data.mode === 'auto') { - // Try to ensure the page has focus - window.focus(); - } - // Run the tests if an error didn't occur during loading if (loadError !== undefined) { return Promise.reject(loadError); } else { - return runner.run(data.chunk, data.retries, data.timeout, data.stopOnFailure); + const autoMode = data.mode === 'auto'; + if (autoMode) { + // Try to ensure the page has focus + window.focus(); + } + return runner.run(data.chunk, data.retries, data.timeout, data.stopOnFailure, autoMode); } }).catch((e: Error) => { console.error('Unexpected error occurred', e); @@ -43,7 +42,7 @@ const setupAndRun = (loadError?: Error) => { }; const run = () => setupAndRun(); -const runError = (e: Error) => setupAndRun(e); +const runError = (e: Error) => setupAndRun(Failure.prepFailure(e)); const loadAndRun = (scripts: string[]) => { // Load the scripts and then run diff --git a/modules/runner/src/main/ts/core/Globals.ts b/modules/runner/src/main/ts/core/Globals.ts index 6c32d083..26655cae 100644 --- a/modules/runner/src/main/ts/core/Globals.ts +++ b/modules/runner/src/main/ts/core/Globals.ts @@ -79,6 +79,9 @@ export const afterEach = (title: TitleOrExecuteFn, fn?: ExecuteFn): void => { export const it = (title: string, fn: ExecuteFn): Test => { const suite = getCurrentSuiteOrDie(); + if (suite === root) { + throw new Error('Tests must be in a `describe` block'); + } const test = createTest(title, fn, suite); suite.tests.push(test); @@ -123,4 +126,4 @@ export const setup = (global: any = Global): void => { }); }; -export const rootSuite = (): Suite => root; \ No newline at end of file +export const rootSuite = (): Suite => root; diff --git a/modules/runner/src/main/ts/core/TestLoader.ts b/modules/runner/src/main/ts/core/TestLoader.ts index fcab9f50..c1cddec1 100644 --- a/modules/runner/src/main/ts/core/TestLoader.ts +++ b/modules/runner/src/main/ts/core/TestLoader.ts @@ -1,4 +1,3 @@ -import Promise from '@ephox/wrap-promise-polyfill'; import { ErrorCatcher } from '../errors/ErrorCatcher'; export const load = (scriptUrl: string): Promise => @@ -38,4 +37,4 @@ export const load = (scriptUrl: string): Promise => // Add the script to the dom to load it document.body.appendChild(script); - }); \ No newline at end of file + }); diff --git a/modules/runner/src/main/ts/core/Utils.ts b/modules/runner/src/main/ts/core/Utils.ts index 7225207c..2a42aa28 100644 --- a/modules/runner/src/main/ts/core/Utils.ts +++ b/modules/runner/src/main/ts/core/Utils.ts @@ -1,5 +1,4 @@ import { Suite, Test } from '@ephox/bedrock-common'; -import Promise from '@ephox/wrap-promise-polyfill'; import sourceMappedStackTrace from 'sourcemapped-stacktrace'; // eslint-disable-next-line @typescript-eslint/no-empty-function @@ -20,8 +19,8 @@ export const makeUrl = (session: string, offset: number, failed: number, skipped return baseUrl + makeQueryParams(session, offset, failed, skipped, retry); }; -export const formatElapsedTime = (start: Date, end: Date): string => { - const millis = end.getTime() - start.getTime(); +export const formatElapsedTime = (start: number, end: number): string => { + const millis = end - start; const seconds = Math.floor(millis / 1000); const point = Math.floor(millis - (seconds * 1000) / 100); const printable = @@ -62,4 +61,4 @@ export const setStack = (error: Error, stack: string | undefined): void => { } catch (err) { // Do nothing } -}; \ No newline at end of file +}; diff --git a/modules/runner/src/main/ts/reporter/Callbacks.ts b/modules/runner/src/main/ts/reporter/Callbacks.ts index ce10424e..78bd4c4c 100644 --- a/modules/runner/src/main/ts/reporter/Callbacks.ts +++ b/modules/runner/src/main/ts/reporter/Callbacks.ts @@ -1,7 +1,16 @@ import { ErrorData, Global } from '@ephox/bedrock-common'; -import Promise from '@ephox/wrap-promise-polyfill'; + import { HarnessResponse } from '../core/ServerTypes'; +export interface TestReport { + readonly file: string; + readonly name: string; + readonly passed: boolean; + readonly time: string; + readonly skipped: string | null; + readonly error: TestErrorData | null +} + export interface TestErrorData { readonly data: ErrorData; readonly text: string; @@ -11,15 +20,24 @@ export interface Callbacks { readonly loadHarness: () => Promise readonly sendKeepAlive: (session: string) => Promise; readonly sendInit: (session: string) => Promise; - readonly sendTestStart: (session: string, totalTests: number, file: string, name: string) => Promise; - readonly sendTestResult: (session: string, file: string, name: string, passed: boolean, time: string, error: TestErrorData | null, skipped: string | null) => Promise; + readonly sendTestStart: (session: string, number: number, totalTests: number, file: string, name: string) => Promise; + readonly sendTestResults: (session: string, results: TestReport[]) => Promise; readonly sendDone: (session: string, error?: string) => Promise; } declare const $: JQueryStatic; -const sendJson = (url: string, data: any): Promise => { +function generateErrorMessage(xhr: JQuery.jqXHR, onError: (reason?: any) => void, url: string, requestDetails: string, statusText: 'timeout' | 'error' | 'abort' | 'parsererror', e: string) { + if (xhr.readyState === 0) { + onError(`Unable to open connection to ${url}, ${requestDetails}. Status text "${statusText}", error thrown "${e}"`); + } else { + onError(`Response status ${xhr.status} connecting to ${url}, ${requestDetails}. Status text "${statusText}", error thrown "${e}"`); + } +} + +const sendJson = (url: string, jsonData: any): Promise => { return new Promise((onSuccess, onError) => { + const data = JSON.stringify(jsonData); $.ajax({ method: 'post', url, @@ -27,9 +45,9 @@ const sendJson = (url: string, data: any): Promise => { dataType: 'json', success: onSuccess, error: (xhr, statusText, e) => { - onError(e); + generateErrorMessage(xhr, onError, url, `sending ${data}`, statusText, e); }, - data: JSON.stringify(data), + data, }); }); }; @@ -41,7 +59,7 @@ const getJson = (url: string): Promise => { dataType: 'json', success: onSuccess, error: (xhr, statusText, e) => { - onError(e); + generateErrorMessage(xhr, onError, url, 'as a get request', statusText, e); } }); })); @@ -64,8 +82,9 @@ export const Callbacks = (): Callbacks => { }); }; - const sendTestStart = (session: string, totalTests: number, file: string, name: string): Promise => { + const sendTestStart = (session: string, number: number, totalTests: number, file: string, name: string): Promise => { return sendJson('/tests/start', { + number, totalTests, session, file, @@ -73,15 +92,10 @@ export const Callbacks = (): Callbacks => { }); }; - const sendTestResult = (session: string, file: string, name: string, passed: boolean, time: string, error: TestErrorData | null, skipped: string | null): Promise => { - return sendJson('/tests/result', { + const sendTestResults = (session: string, results: TestReport[]): Promise => { + return sendJson('/tests/results', { session, - file, - name, - passed, - skipped, - time, - error, + results, }); }; @@ -101,7 +115,7 @@ export const Callbacks = (): Callbacks => { sendInit, sendKeepAlive, sendTestStart, - sendTestResult, + sendTestResults, sendDone }; -}; \ No newline at end of file +}; diff --git a/modules/runner/src/main/ts/reporter/Reporter.ts b/modules/runner/src/main/ts/reporter/Reporter.ts index 75aafd69..c3499852 100644 --- a/modules/runner/src/main/ts/reporter/Reporter.ts +++ b/modules/runner/src/main/ts/reporter/Reporter.ts @@ -1,22 +1,23 @@ import { LoggedError, Reporter as ErrorReporter } from '@ephox/bedrock-common'; -import Promise from '@ephox/wrap-promise-polyfill'; -import { Callbacks } from './Callbacks'; +import { Callbacks, TestReport } from './Callbacks'; import { UrlParams } from '../core/UrlParams'; import { formatElapsedTime, mapStackTrace, setStack } from '../core/Utils'; type LoggedError = LoggedError.LoggedError; export interface TestReporter { - readonly start: () => Promise; - readonly retry: () => Promise; - readonly pass: () => Promise; - readonly skip: (reason: string) => Promise; - readonly fail: (e: LoggedError) => Promise; + readonly start: () => void; + readonly retry: () => void; + readonly pass: () => void; + readonly skip: (reason: string) => void; + readonly fail: (e: LoggedError) => void; } export interface Reporter { readonly summary: () => { offset: number; passed: number; failed: number; skipped: number }; readonly test: (file: string, name: string, totalNumTests: number) => TestReporter; + readonly waitForResults: () => Promise; + readonly retry: () => Promise; readonly done: (error?: LoggedError) => void; } @@ -30,7 +31,7 @@ export interface ReporterUi { readonly done: (totalTime: string) => void; } -const elapsed = (since: Date): string => formatElapsedTime(since, new Date()); +const elapsed = (since: number): string => formatElapsedTime(since, Date.now()); const mapError = (e: LoggedError) => mapStackTrace(e.stack).then((mappedStack) => { const originalStack = e.stack; @@ -42,16 +43,41 @@ const mapError = (e: LoggedError) => mapStackTrace(e.stack).then((mappedStack) = e.logs = logs.replace(originalStack, mappedStack).split('\n'); } - return Promise.resolve(e); + return e; }); export const Reporter = (params: UrlParams, callbacks: Callbacks, ui: ReporterUi): Reporter => { - const initial = new Date(); + const initial = Date.now(); + let timeOfLastReport = initial; let currentCount = params.offset || 0; let passCount = 0; let skipCount = 0; let failCount = 0; + // A list of test results we are going to send as a batch to the server + const testResults: TestReport[] = []; + + // A global list of requests that were sent to the server, we must wait for these before sending `/done` or it may confuse the HUD + const requestsInFlight: Promise[] = []; + + const sendCurrentResults = (): boolean => { + if (testResults.length > 0) { + requestsInFlight.push(callbacks.sendTestResults(params.session, testResults)); + testResults.length = 0; + return true; + } + return false; + }; + + const reportResult = (result: TestReport): void => { + testResults.push(result); + if (Date.now() - timeOfLastReport > 30 * 1000) { + // ping the server with results every 30 seconds or so, as a form of keep-alive + sendCurrentResults(); + timeOfLastReport = Date.now(); + } + }; + const summary = () => ({ offset: Math.max(0, currentCount - 1), passed: passCount + (params.offset - params.failed - params.skipped), @@ -60,73 +86,94 @@ export const Reporter = (params: UrlParams, callbacks: Callbacks, ui: ReporterUi }); const test = (file: string, name: string, totalNumTests: number) => { - let starttime: Date; + let starttime = Date.now(); let reported = false; let started = false; const testUi = ui.test(); - const start = (): Promise => { - if (started) { - return Promise.resolve(); - } else { + const start = (): void => { + if (!started) { started = true; - starttime = new Date(); + starttime = Date.now(); currentCount++; testUi.start(file, name); - return callbacks.sendTestStart(params.session, totalNumTests, file, name); + + if (currentCount === 1) { + // we need to send test start once to establish the session + requestsInFlight.push(callbacks.sendTestStart(params.session, currentCount, totalNumTests, file, name)); + } } }; - const retry = (): Promise => { - starttime = new Date(); - return Promise.resolve(); + const retry = (): void => { + // a test has used `this.retries()` and wants to retry without reloading the page + starttime = Date.now(); }; - const pass = (): Promise => { - if (reported) { - return Promise.resolve(); - } else { + const pass = (): void => { + if (!reported) { reported = true; passCount++; const testTime = elapsed(starttime); testUi.pass(testTime, currentCount); - return callbacks.sendTestResult(params.session, file, name, true, testTime, null, null); + reportResult({ + file, + name, + passed: true, + time: testTime, + error: null, + skipped: null, + }); } }; - const skip = (reason: string): Promise => { - if (reported) { - return Promise.resolve(); - } else { + const skip = (reason: string): void => { + if (!reported) { reported = true; skipCount++; const testTime = elapsed(starttime); testUi.skip(testTime, currentCount); - return callbacks.sendTestResult(params.session, file, name, false, testTime, null, reason); + + reportResult({ + file, + name, + passed: false, + time: testTime, + error: null, + skipped: reason, + }); } }; - const fail = (e: LoggedError): Promise => { - if (reported) { - return Promise.resolve(); - } else { + const fail = (e: LoggedError): void => { + if (!reported) { reported = true; failCount++; const testTime = elapsed(starttime); - return mapError(e).then((err) => { + + // `sourcemapped-stacktrace` is async, so we need to wait for it + requestsInFlight.push(mapError(e).then((err) => { const errorData = ErrorReporter.data(err); const error = { data: errorData, text: ErrorReporter.dataText(errorData) }; + reportResult({ + file, + name, + passed: false, + time: testTime, + error, + skipped: null, + }); + testUi.fail(err, testTime, currentCount); - return callbacks.sendTestResult(params.session, file, name, false, testTime, error, null); - }); + })); } }; @@ -139,6 +186,28 @@ export const Reporter = (params: UrlParams, callbacks: Callbacks, ui: ReporterUi }; }; + const waitForResults = async (): Promise => { + sendCurrentResults(); + if (requestsInFlight.length > 0) { + const currentRequests = requestsInFlight.slice(0); + requestsInFlight.length = 0; + await Promise.all(currentRequests); + // if more things have been queued, such as a failing test stack trace, wait for those as well + await waitForResults(); + } + }; + + // the page is about to reload to retry a test + const retry = (): Promise => { + // remove the last test failure from the stack so we don't confuse the server + const last = testResults.pop(); + if (last && last.error === null) { + // something isn't right, the last test didn't fail, put it back + testResults.push(last); + } + return waitForResults(); + }; + const done = (error?: LoggedError): void => { const setAsDone = (): void => { const totalTime = elapsed(initial); @@ -146,12 +215,18 @@ export const Reporter = (params: UrlParams, callbacks: Callbacks, ui: ReporterUi }; const textError = error !== undefined ? ErrorReporter.text(error) : undefined; - callbacks.sendDone(params.session, textError).then(setAsDone, setAsDone); + + // make sure any in progress updates are sent before we clean up + waitForResults().then(() => + callbacks.sendDone(params.session, textError).then(setAsDone, setAsDone) + ); }; return { summary, test, + retry, + waitForResults, done }; -}; \ No newline at end of file +}; diff --git a/modules/runner/src/main/ts/runner/Hooks.ts b/modules/runner/src/main/ts/runner/Hooks.ts index 05e8f7e5..9606c14c 100644 --- a/modules/runner/src/main/ts/runner/Hooks.ts +++ b/modules/runner/src/main/ts/runner/Hooks.ts @@ -1,5 +1,4 @@ import { Hook, HookType, RunnableState, Suite, Test } from '@ephox/bedrock-common'; -import Promise from '@ephox/wrap-promise-polyfill'; import * as Context from '../core/Context'; import { SkipError } from '../errors/Errors'; import { runWithErrorCatcher, runWithTimeout } from './Run'; @@ -75,4 +74,4 @@ export const runAfterEach = (currentTest: Test): Promise => { } else { return Promise.resolve(); } -}; \ No newline at end of file +}; diff --git a/modules/runner/src/main/ts/runner/Run.ts b/modules/runner/src/main/ts/runner/Run.ts index 48bb4109..b273314c 100644 --- a/modules/runner/src/main/ts/runner/Run.ts +++ b/modules/runner/src/main/ts/runner/Run.ts @@ -1,5 +1,4 @@ import { Context, ExecuteFn, Failure, Runnable, RunnableState, TestThrowable } from '@ephox/bedrock-common'; -import Promise from '@ephox/wrap-promise-polyfill'; import { isInternalError, MultipleDone, SkipError } from '../errors/Errors'; import { ErrorCatcher } from '../errors/ErrorCatcher'; import { Timer } from './Timer'; @@ -128,4 +127,4 @@ export const runWithTimeout = (runnable: Runnable, context: Context, defaultTime }).then(resolveIfNotTimedOut, reject); }); } -}; \ No newline at end of file +}; diff --git a/modules/runner/src/main/ts/runner/Runner.ts b/modules/runner/src/main/ts/runner/Runner.ts index e732935e..2bcd6a2c 100644 --- a/modules/runner/src/main/ts/runner/Runner.ts +++ b/modules/runner/src/main/ts/runner/Runner.ts @@ -1,5 +1,4 @@ import { Suite } from '@ephox/bedrock-common'; -import Promise from '@ephox/wrap-promise-polyfill'; import { HarnessResponse } from '../core/ServerTypes'; import { UrlParams } from '../core/UrlParams'; import { noop } from '../core/Utils'; @@ -12,7 +11,7 @@ import { countTests, filterOnly } from './Utils'; export interface Runner { readonly init: () => Promise; - readonly run: (chunk: number, retries: number, timeout: number, stopOnFailure: boolean) => Promise; + readonly run: (chunk: number, retries: number, timeout: number, stopOnFailure: boolean, autoMode: boolean) => Promise; } // 5sec interval for the server to know the client hasn't disconnected @@ -30,8 +29,10 @@ export const Runner = (rootSuite: Suite, params: UrlParams, callbacks: Callbacks }; const runNextChunk = (offset: number) => { - const sum = reporter.summary(); - actions.reloadPage(offset, sum.failed, sum.skipped); + reporter.waitForResults().then(() => { + const sum = reporter.summary(); + actions.reloadPage(offset, sum.failed, sum.skipped); + }); }; const retryTest = withSum(actions.retryTest); @@ -51,14 +52,23 @@ export const Runner = (rootSuite: Suite, params: UrlParams, callbacks: Callbacks reporter.done(); // make it easy to restart at this test stopTest(); - } else if (params.retry < retries) { - retryTest(params.retry + 1); } else { - loadNextTest(); + if (params.retry < retries) { + // post all results except the failure, and retry + reporter.retry().then(() => { + retryTest(params.retry + 1); + }); + } else { + // post the failure to the server and move on + reporter.waitForResults().then(() => { + // wait for 2 seconds for the purposes of showing the failure on video + setTimeout(loadNextTest, 2000); + }); + } } }; - const init = (): Promise => { + const init = async (): Promise => { // Filter the tests to ensure we have an accurate total test count filterOnly(rootSuite); numTests = countTests(rootSuite); @@ -66,24 +76,31 @@ export const Runner = (rootSuite: Suite, params: UrlParams, callbacks: Callbacks // Render the initial UI ui.render(params.offset, numTests, actions.restartTests, retryTest, loadNextTest); - // delay this ajax call until after the reporter status elements are in the page - const keepAliveTimer = setInterval(() => { - callbacks.sendKeepAlive(params.session).catch(() => { - // if the server shuts down stop trying to send messages - clearInterval(keepAliveTimer); - }); - }, KEEP_ALIVE_INTERVAL); - - return callbacks.sendInit(params.session).then(() => callbacks.loadHarness()); + return Promise.all([callbacks.sendInit(params.session), callbacks.loadHarness()]).then(([_, harness]) => { + // we don't need a keep-alive timer in auto mode, + if (harness.mode === 'manual') { + // delay this ajax call until after the reporter status elements are in the page + const keepAliveTimer = setInterval(() => { + callbacks.sendKeepAlive(params.session).catch(() => { + // if the server shuts down stop trying to send messages + clearInterval(keepAliveTimer); + }); + }, KEEP_ALIVE_INTERVAL); + } + + return harness; + }); }; - const run = (chunk: number, retries: number, timeout: number, stopOnFailure: boolean): Promise => { + const run = (chunk: number, retries: number, timeout: number, stopOnFailure: boolean, auto: boolean): Promise => { const runState: RunState = { totalTests: numTests, offset: params.offset, chunk, timeout, - testCount: 0 + testCount: 0, + checkSiblings: () => ui.siblings(), + auto }; const runActions: RunActions = { @@ -116,4 +133,4 @@ export const Runner = (rootSuite: Suite, params: UrlParams, callbacks: Callbacks init, run }; -}; \ No newline at end of file +}; diff --git a/modules/runner/src/main/ts/runner/TestRun.ts b/modules/runner/src/main/ts/runner/TestRun.ts index 6f0e1dce..9d06debb 100644 --- a/modules/runner/src/main/ts/runner/TestRun.ts +++ b/modules/runner/src/main/ts/runner/TestRun.ts @@ -1,5 +1,4 @@ import { Failure, LoggedError, RunnableState, Suite, Test } from '@ephox/bedrock-common'; -import Promise from '@ephox/wrap-promise-polyfill'; import * as Context from '../core/Context'; import { InternalError, isInternalError, SkipError } from '../errors/Errors'; import { Reporter, TestReporter } from '../reporter/Reporter'; @@ -16,6 +15,8 @@ export interface RunState { readonly chunk: number; readonly timeout: number; testCount: number; + readonly checkSiblings: () => string[]; + readonly auto: boolean; } export interface RunActions { @@ -26,7 +27,7 @@ export interface RunActions { readonly runNextChunk: (offset: number) => void; } -const runTestWithRetry = (test: Test, state: RunState, report: TestReporter, retryCount: number): Promise => { +const runTestWithRetry = (test: Test, state: RunState, testReport: TestReporter, retryCount: number): Promise => { if (test.isSkipped()) { return Promise.reject(new SkipError()); } else { @@ -38,9 +39,12 @@ const runTestWithRetry = (test: Test, state: RunState, report: TestReporter, ret // Ensure we run the afterEach hooks no matter if the test failed .then(runAfterHooks(false), runAfterHooks(true)) .catch((e: LoggedError | InternalError) => { + // This is unique to `this.retries()` within a test, not the general page reload to retry system if (retryCount < test.retries() && !isInternalError(e)) { test.setResult(RunnableState.NotRun); - return report.retry().then(() => runTestWithRetry(test, state, report, retryCount + 1)); + testReport.retry(); + // don't fail the page + return runTestWithRetry(test, state, testReport, retryCount + 1); } else { return Promise.reject(e); } @@ -52,17 +56,23 @@ export const runTest = (test: Test, state: RunState, actions: RunActions, report const fail = (report: TestReporter, e: LoggedError) => { test.setResult(RunnableState.Failed, e); console.error(e); - return report.fail(e).then(actions.onFailure).then(() => Promise.reject()); + report.fail(e); + // this is where the page reloads if the global retry system is active + actions.onFailure(); + // Test failures must be an empty reject, otherwise the error management thinks it's a bedrock error + return Promise.reject(); }; - const skip = (report: TestReporter) => { + const skip = (testReport: TestReporter) => { test.setResult(RunnableState.Skipped); - return report.skip(test.title).then(actions.onSkip); + testReport.skip(test.title); + actions.onSkip(); }; - const pass = (report: TestReporter) => { + const pass = (testReport: TestReporter) => { test.setResult(RunnableState.Passed); - return report.pass().then(actions.onPass); + testReport.pass(); + actions.onPass(); }; state.testCount++; @@ -71,27 +81,60 @@ export const runTest = (test: Test, state: RunState, actions: RunActions, report } else if (state.testCount > state.offset + state.chunk) { actions.runNextChunk(state.offset + state.chunk); // Reject so no other tests are run + // Test failures must be an empty reject, otherwise the error management thinks it's a bedrock error return Promise.reject(); } else { - const report = reporter.test(test.file || 'Unknown', test.fullTitle(), state.totalTests); + const testReport = reporter.test(test.file || 'Unknown', test.fullTitle(), state.totalTests); actions.onStart(test); - return report.start() - .then(() => runTestWithRetry(test, state, report, 0)) - .then(() => pass(report), (e: LoggedError | InternalError) => { + if (!state.auto) { + console.log(`Starting test ${state.testCount} of ${state.totalTests}: ${test.fullTitle()} (${test.file})`); + } + testReport.start(); + return runTestWithRetry(test, state, testReport, 0) + .then(() => pass(testReport), (e: LoggedError | InternalError) => { if (e instanceof SkipError) { - return skip(report); + return skip(testReport); } else { - return fail(report, Failure.prepFailure(e)); + return fail(testReport, Failure.prepFailure(e)); } }); } }; -export const runTests = (tests: Test[], state: RunState, actions: RunActions, reporter: RunReporter): Promise => { +const runTests = (tests: Test[], state: RunState, actions: RunActions, reporter: RunReporter): Promise => { return loop(tests, (test) => runTest(test, state, actions, reporter)); }; +const checkSiblings = (suite: Suite, state: RunState, actions: RunActions, reporter: RunReporter) => () => { + // we only want to verify this for things directly in the root test suite. + // test files often have nested context blocks; shared state is allowed within a single file. + if (!suite.root) { + return Promise.resolve(); + } + // if there are no siblings, that's a pass + const siblings = state.checkSiblings(); + if (siblings.length === 0) { + return Promise.resolve(); + } + + // rogue sibling elements have been detected, generate a fake test and then report it as failed + const tests = suite.tests.length === 0 ? suite.suites[0].tests : suite.tests; + const filename = tests[0]?.file || 'Unknown'; + const fakeReporter = reporter.test(filename.substring(filename.lastIndexOf('/') + 1) + ' DOM validation', suite.fullTitle(), state.totalTests + 1); + fakeReporter.start(); + const fakeFailure = LoggedError.loggedError(new Error(`File ${filename} did not clean up after itself, extra elements were left in the DOM`), siblings); + + // This is mostly duplicate of `fakeReporter.fail()`, but the "test" doesn't really exist + // so the UI gets confused and shows two errors if we do that + console.error(fakeFailure); + fakeReporter.fail(fakeFailure); + // this is where the page reloads if the global retry system is active + actions.onFailure(); + // Test failures must be an empty reject, otherwise the error management thinks it's a bedrock error + return Promise.reject(); +}; + export const runSuite = (suite: Suite, state: RunState, actions: RunActions, reporter: RunReporter): Promise => { const numTests = countTests(suite); if (state.testCount + numTests <= state.offset) { @@ -104,11 +147,11 @@ export const runSuite = (suite: Suite, state: RunState, actions: RunActions, rep return Hooks.runBefore(suite) .then(() => runTests(suite.tests, state, actions, reporter)) .then(() => runSuites(suite.suites, state, actions, reporter)) + .then(checkSiblings(suite, state, actions, reporter)) // Ensure we run the after hooks no matter if the tests/suites fail .then(runAfterHooks(false), runAfterHooks(true)); } }; -export const runSuites = (suites: Suite[], state: RunState, actions: RunActions, reporter: RunReporter): Promise => { - return loop(suites, (suite) => runSuite(suite, state, actions, reporter)); -}; \ No newline at end of file +const runSuites = (suites: Suite[], state: RunState, actions: RunActions, reporter: RunReporter): Promise => + loop(suites, (suite) => runSuite(suite, state, actions, reporter)); diff --git a/modules/runner/src/main/ts/runner/Utils.ts b/modules/runner/src/main/ts/runner/Utils.ts index be4dcfcd..84bff409 100644 --- a/modules/runner/src/main/ts/runner/Utils.ts +++ b/modules/runner/src/main/ts/runner/Utils.ts @@ -1,5 +1,4 @@ import { Suite, Test } from '@ephox/bedrock-common'; -import Promise from '@ephox/wrap-promise-polyfill'; export const countTests = (suite: Suite): number => suite.tests.length + suite.suites.reduce((acc, suite) => acc + countTests(suite), 0); @@ -46,4 +45,4 @@ export const filterOnly = (suite: Suite): void => { suite.suites.forEach(filterOnly); suite.suites = onlySuites; } -}; \ No newline at end of file +}; diff --git a/modules/runner/src/main/ts/ui/Ui.ts b/modules/runner/src/main/ts/ui/Ui.ts index 7b562376..3ef473da 100644 --- a/modules/runner/src/main/ts/ui/Ui.ts +++ b/modules/runner/src/main/ts/ui/Ui.ts @@ -11,6 +11,7 @@ export interface Ui extends ReporterUi { readonly render: (offset: number, totalNumTests: number, onRestart: () => void, onRetry: () => void, onSkip: () => void) => void; readonly error: (e: LoggedError) => void; readonly setStopOnFailure: (stopOnFailure: boolean) => void; + readonly siblings: () => string[]; } declare const $: JQueryStatic; @@ -24,7 +25,7 @@ export const Ui = (container: JQuery): Ui => { let skipBtn: JQuery; const render = (offset: number, totalNumTests: number, onRestart: () => void, onRetry: () => void, onSkip: () => void) => { - ui = $('
'); + ui = $('
'); current = $('').addClass('progress').text(offset); restartBtn = $('