diff --git a/src/browser-pool/basic-pool.ts b/src/browser-pool/basic-pool.ts index b2725a145..69069ac83 100644 --- a/src/browser-pool/basic-pool.ts +++ b/src/browser-pool/basic-pool.ts @@ -14,6 +14,7 @@ export class BasicPool implements Pool { private _emitter: AsyncEmitter; private _activeSessions: Record; private _cancelled: boolean; + private _cancelError: Error | null; private _wdPool: WebdriverPool; private _observer?: PoolObserver; log: debug.Debugger; @@ -29,11 +30,16 @@ export class BasicPool implements Pool { this._activeSessions = {}; this._cancelled = false; + this._cancelError = null; this._wdPool = new WebdriverPool(); this._observer = isPoolObserver(observer) ? observer : undefined; } async getBrowser(id: string, opts: BrowserOpts = {}): Promise { + if (this._cancelled) { + throw this._cancelError ?? new CancelledError(); + } + const operation = this._observer?.start("browser.session.create", { browserId: id }); let browser: NewBrowser | undefined; @@ -51,7 +57,7 @@ export class BasicPool implements Pool { await this._emit(MasterEvents.SESSION_START, browser); if (this._cancelled) { - throw new CancelledError(); + throw this._cancelError ?? new CancelledError(); } await browser.reset(); @@ -111,10 +117,11 @@ export class BasicPool implements Pool { }); } - cancel(): void { + cancel(err?: Error): void { this._cancelled = true; + this._cancelError ??= err ?? new CancelledError(); - _.forEach(this._activeSessions, browser => browser.quit()); + _.forEach(this._activeSessions, browser => browser.quit(this._cancelError!)); this._activeSessions = {}; } diff --git a/src/browser-pool/caching-pool.ts b/src/browser-pool/caching-pool.ts index 33a35f980..66fab6d62 100644 --- a/src/browser-pool/caching-pool.ts +++ b/src/browser-pool/caching-pool.ts @@ -115,9 +115,9 @@ export class CachingPool implements Pool { return cache.push(browser); } - cancel(): void { + cancel(err?: Error): void { this.log("cancel"); - this.underlyingPool.cancel(); + this.underlyingPool.cancel(err); } } diff --git a/src/browser/new-browser.ts b/src/browser/new-browser.ts index c9c593d3c..3acc10a3d 100644 --- a/src/browser/new-browser.ts +++ b/src/browser/new-browser.ts @@ -57,6 +57,9 @@ const headlessBrowserOptions: HeadlessBrowserOptions = { export class NewBrowser extends Browser { private _onExit: (err?: Error) => Promise = async () => {}; + private _initPromise: Promise | null = null; + private _quitPromise: Promise | null = null; + private _killPromise: Promise | null = null; constructor(config: Config, opts: BrowserOpts) { super(config, opts); @@ -65,7 +68,13 @@ export class NewBrowser extends Browser { signalHandler.on("exit", this._onExit); } - async init(): Promise { + init(): Promise { + this._initPromise ??= this._init(); + + return this._initPromise; + } + + private async _init(): Promise { this._session = await this._createSession(); this._addCommands(); @@ -79,11 +88,21 @@ export class NewBrowser extends Browser { return Promise.resolve(); } - async quit(err?: Error): Promise { + quit(err?: Error): Promise { + if (this._quitPromise) { + return this._quitPromise; + } + this._exitError = err; signalHandler.off("exit", this._onExit); + this._quitPromise = this._quit(); + + return this._quitPromise; + } + private async _quit(): Promise { try { + await this._initPromise; this.setHttpTimeout(this._config.sessionQuitTimeout); await this._session!.deleteSession(); this._wdProcess?.free(); @@ -95,7 +114,17 @@ export class NewBrowser extends Browser { } } - async kill(): Promise { + kill(): Promise { + if (this._killPromise) { + return this._killPromise; + } + + this._killPromise = this._kill(); + + return this._killPromise; + } + + private async _kill(): Promise { try { await this._session!.deleteSession(); this._wdProcess?.kill(); diff --git a/src/runner/browser-runner.ts b/src/runner/browser-runner.ts index c7d208038..c0b8eed31 100644 --- a/src/runner/browser-runner.ts +++ b/src/runner/browser-runner.ts @@ -26,6 +26,7 @@ export class BrowserRunner extends CancelableEmitter { private workers: Workers; private running: PromiseGroup; private profiler: ProfilerRuntimeLike; + private cancelError: Error | null; constructor( browserId: string, @@ -44,6 +45,7 @@ export class BrowserRunner extends CancelableEmitter { this.workers = workers; this.running = new PromiseGroup(); this.profiler = profiler; + this.cancelError = null; } get browserId(): string { @@ -72,6 +74,10 @@ export class BrowserRunner extends CancelableEmitter { }); const runner = TestRunner.create(test, this.config, browserAgent, this.profiler); + if (this.cancelError && typeof runner.cancel === "function") { + runner.cancel(this.cancelError); + } + runner.on(MasterEvents.TEST_BEGIN, (test: Test) => { this.suiteMonitor.testBegin(test); }); @@ -96,6 +102,7 @@ export class BrowserRunner extends CancelableEmitter { } cancel(error: Error): void { + this.cancelError ??= error; this.activeTestRunners.forEach(runner => runner.cancel(error)); } diff --git a/src/runner/index.ts b/src/runner/index.ts index d7710946d..05415e673 100644 --- a/src/runner/index.ts +++ b/src/runner/index.ts @@ -58,6 +58,7 @@ export class MainRunner extends RunnableEmitter { protected workersRegistry: WorkersRegistry; protected workers: Workers | null; protected profiler: ProfilerRuntimeLike; + private _cancelError: Error | null; constructor(config: Config, interceptors: Interceptor[], profiler: ProfilerRuntimeLike = noopProfilerRuntime) { super(); @@ -71,6 +72,7 @@ export class MainRunner extends RunnableEmitter { this.running = new PromiseGroup(); this.runned = false; this.cancelled = false; + this._cancelError = null; this.profiler = profiler; this.workersRegistry = WorkersRegistry.create(this.config, this.profiler); @@ -97,6 +99,10 @@ export class MainRunner extends RunnableEmitter { this.workers = this.registerWorkers(require.resolve("../worker"), ["runTest", "cancel"] as const) as Workers; this.browserPool = pool.create(this.config, this, this.profiler); + if (this._cancelError) { + this.browserPool.cancel(this._cancelError); + } + this.once(MasterEvents.EXIT, () => this.workersRegistry.shutdown()); eventsUtils.passthroughEvent(this, this.workersRegistry, MasterEvents.EXIT); @@ -112,7 +118,7 @@ export class MainRunner extends RunnableEmitter { try { await this.emitAndWait(MasterEvents.RUNNER_START, this); this.emit(MasterEvents.BEGIN); - !this.cancelled && (await this._runTests(testCollection, opts)); + await this._runTests(testCollection, opts); } finally { this.emit(MasterEvents.END); await this.emitAndWait(MasterEvents.RUNNER_END, stats.getResult()).catch(logger.warn); @@ -163,6 +169,10 @@ export class MainRunner extends RunnableEmitter { protected _createBrowserRunner(browserId: string): BrowserRunner { const runner = BrowserRunner.create(browserId, this.config, this.browserPool, this.workers, this.profiler); + if (this._cancelError) { + runner.cancel(this._cancelError); + } + eventsUtils.passthroughEvent(runner, this, this.getEventsToPassthrough()); this.interceptEvents(runner, this.getEventsToIntercept()); @@ -214,9 +224,10 @@ export class MainRunner extends RunnableEmitter { cancel(error: Error): void { this.cancelled = true; - this.browserPool?.cancel(error); + this._cancelError ??= error; + this.browserPool?.cancel(this._cancelError); - this.activeBrowserRunners.forEach(runner => runner.cancel(error)); + this.activeBrowserRunners.forEach(runner => runner.cancel(this._cancelError!)); this.workers?.cancel().catch(() => { /* we can just ignore the error thrown, because we don't care about cleanup at this point */ diff --git a/src/runner/test-runner/insistant-test-runner.js b/src/runner/test-runner/insistant-test-runner.js index 2a679656d..226fd1af6 100644 --- a/src/runner/test-runner/insistant-test-runner.js +++ b/src/runner/test-runner/insistant-test-runner.js @@ -21,6 +21,8 @@ module.exports = class InsistantTestRunner extends RunnableEmitter { this._retriesPerformed = 0; this._cancelled = false; + this._cancelError = null; + this._activeRunner = null; } async run(workers) { @@ -40,10 +42,16 @@ module.exports = class InsistantTestRunner extends RunnableEmitter { } }, ); + this._activeRunner = runner; + + if (this._cancelError) { + runner.cancel(this._cancelError); + } passthroughEvent(runner, this, [MasterEvents.TEST_BEGIN, MasterEvents.TEST_PASS, MasterEvents.TEST_END]); await runner.run(workers, this._retriesPerformed); + this._activeRunner = null; if (retry) { ++this._retriesPerformed; @@ -78,7 +86,9 @@ module.exports = class InsistantTestRunner extends RunnableEmitter { return this._browserConfig.retry - this._retriesPerformed; } - cancel() { + cancel(error) { this._cancelled = true; + this._cancelError = this._cancelError || error; + this._activeRunner?.cancel(error); } }; diff --git a/src/runner/test-runner/regular-test-runner.js b/src/runner/test-runner/regular-test-runner.js index c2305ee20..a1c42a7ce 100644 --- a/src/runner/test-runner/regular-test-runner.js +++ b/src/runner/test-runner/regular-test-runner.js @@ -17,6 +17,8 @@ module.exports = class RegularTestRunner extends RunnableEmitter { this._test = test.clone(); this._browserAgent = browserAgent; this._browser = null; + this._cancelError = null; + this._cancelBrowserRequest = null; this._profiler = profiler || noopProfilerRuntime; this._profilerSanitizer = this._profiler.isEnabled(2) ? new ProfilerSanitizer() : null; } @@ -85,7 +87,7 @@ module.exports = class RegularTestRunner extends RunnableEmitter { this._emit(MasterEvents.TEST_PASS); } catch (error) { - this._test.err = this._browser?.exitError || error; + this._test.err = this._cancelError || this._browser?.exitError || error; this._applyTestResults(this._test.err); @@ -106,6 +108,10 @@ module.exports = class RegularTestRunner extends RunnableEmitter { } async _runTest(workers, attempt, attemptId, profileSessionId) { + if (this._cancelError) { + throw this._cancelError; + } + if (!this._browser) { throw this._test.err; } @@ -182,14 +188,28 @@ module.exports = class RegularTestRunner extends RunnableEmitter { .slice(0, 12)}`; } + cancel(error) { + this._cancelError = this._cancelError || error; + this._cancelBrowserRequest?.(this._cancelError); + } + async _getBrowser() { + if (this._cancelError) { + this._test.err = this._cancelError; + return; + } + + const cancelPromise = new Promise((_, reject) => { + this._cancelBrowserRequest = reject; + }); + try { const state = { testXReqId: crypto.randomUUID(), traceparent: this._getTraceparent(), }; - this._browser = await this._browserAgent.getBrowser({ state }); + this._browser = await Promise.race([this._browserAgent.getBrowser({ state }), cancelPromise]); // TODO: move logic to caching pool (in order to use correct state for cached browsers) if ( @@ -203,7 +223,9 @@ module.exports = class RegularTestRunner extends RunnableEmitter { return this._browser; } catch (error) { - this._test.err = error; + this._test.err = this._cancelError || error; + } finally { + this._cancelBrowserRequest = null; } } diff --git a/src/testplane.ts b/src/testplane.ts index 7d03b760c..aa16a1226 100644 --- a/src/testplane.ts +++ b/src/testplane.ts @@ -108,6 +108,7 @@ export class Testplane extends BaseTestplane { protected viteServer: ViteServer | null; private _filesToRemove: string[]; + private _haltError: Error | null; protected testsTracker: TestsTracker | null; constructor(config?: string | ConfigInput) { @@ -119,6 +120,7 @@ export class Testplane extends BaseTestplane { this.viteServer = null; this._filesToRemove = []; + this._haltError = null; this.testsTracker = null; @@ -190,6 +192,7 @@ export class Testplane extends BaseTestplane { reporters = [], }: Partial, ): Promise { + this._haltError = null; validateUnknownBrowsers(browsers!, _.keys(this._config.browsers)); RuntimeConfig.getInstance().extend({ @@ -210,6 +213,10 @@ export class Testplane extends BaseTestplane { const runner = RunnerClass.create(this._config, this._interceptors, this._profiler.runtime); this.runner = runner; + if (this._haltError) { + runner.cancel(this._haltError); + } + this.on(MasterEvents.TEST_FAIL, res => { this._fail(); this._addFailedTest(res); @@ -454,6 +461,9 @@ export class Testplane extends BaseTestplane { message: this._profiler.sanitizeMessage(err?.message ?? "Testplane run was aborted"), }); + this._haltError = err; + signalHandler.emit(MasterEvents.EXIT, err); + if (timeout > 0) { setTimeout(() => { logger.error("Forcing shutdown..."); @@ -466,8 +476,6 @@ export class Testplane extends BaseTestplane { this.viteServer.close(); } - if (this.runner) { - this.runner.cancel(err); - } + this.runner?.cancel(err); } } diff --git a/test/src/browser-pool/basic-pool.js b/test/src/browser-pool/basic-pool.js index 3fe5fc427..bc037897b 100644 --- a/test/src/browser-pool/basic-pool.js +++ b/test/src/browser-pool/basic-pool.js @@ -227,6 +227,16 @@ describe("browser-pool/basic-pool", () => { await assert.isRejected(pool.getBrowser(), CancelledError); }); + it("should reject subsequent browser requests with passed cancel error", async () => { + const error = new Error("Tests were stopped by the user"); + const pool = mkPool_(); + + pool.cancel(error); + + await assert.isRejected(pool.getBrowser(), error); + assert.notCalled(NewBrowser.create); + }); + it("should quit browser once if it was launched after cancel", async () => { const browser = stubBrowser(); NewBrowser.create.returns(browser); diff --git a/test/src/browser/new-browser.ts b/test/src/browser/new-browser.ts index b576894c4..bd61508b6 100644 --- a/test/src/browser/new-browser.ts +++ b/test/src/browser/new-browser.ts @@ -493,10 +493,34 @@ describe("NewBrowser", () => { assert.called(session.deleteSession); }); + it("should finalize webdriver.io session only once", async () => { + const browser = await mkBrowser_().init(); + const error = new Error("Tests were stopped by the user"); + + await Promise.all([browser.quit(error), browser.quit(error)]); + + assert.calledOnce(session.deleteSession); + assert.strictEqual(browser.exitError, error); + }); + + it("should wait for session creation before finalizing it", async () => { + let resolveSession: (browserSession: unknown) => void; + webdriverioRemoteStub.returns(new Promise(resolve => (resolveSession = resolve))); + const browser = mkBrowser_(); + const initPromise = browser.init(); + const quitPromise = browser.quit(new Error("Tests were stopped by the user")); + + assert.notCalled(session.deleteSession); + resolveSession!(session); + await Promise.all([initPromise, quitPromise]); + + assert.calledOnce(session.deleteSession); + }); + it("should finalize session on global exit event", async () => { await mkBrowser_().init(); - signalHandler.emitAndWait("exit"); + await signalHandler.emitAndWait("exit"); assert.called(session.deleteSession); }); @@ -565,6 +589,15 @@ describe("NewBrowser", () => { assert.notCalled(wdProcess.free); assert.calledOnce(wdProcess.kill); }); + + it("should kill webdriver.io session only once", async () => { + const browser = await mkBrowser_().init(); + + await browser.kill(); + await browser.kill(); + + assert.calledOnce(session.deleteSession); + }); }); describe("sessionId", () => { diff --git a/test/src/runner/browser-runner.js b/test/src/runner/browser-runner.js index f073305ed..52f4fdfd8 100644 --- a/test/src/runner/browser-runner.js +++ b/test/src/runner/browser-runner.js @@ -154,6 +154,16 @@ describe("runner/browser-runner", () => { }); describe("cancel", () => { + it("should cancel a test runner created after browser runner cancellation", async () => { + const error = new Error("Tests were stopped by the user"); + const runner = mkRunner_(); + + runner.cancel(error); + await run_({ runner }); + + assert.calledOnceWith(TestRunner.prototype.cancel, error); + }); + it("should cancel all executing test runners", async () => { stubTestCollection_([Test.create({}), Test.create({})]); diff --git a/test/src/runner/index.js b/test/src/runner/index.js index 08f54a204..5cb933d6b 100644 --- a/test/src/runner/index.js +++ b/test/src/runner/index.js @@ -704,14 +704,15 @@ describe("NodejsEnvRunner", () => { assert.notCalled(BrowserRunner.prototype.cancel); }); - it("shuld not run tests in browser runners if cancelled", async () => { - const runner = new Runner(makeConfigStub()).on(RunnerEvents.RUNNER_START, () => runner.cancel()); + it("should pass cancelled tests to browser runners to emit test results", async () => { + const error = new Error("Tests were stopped by the user"); + const runner = new Runner(makeConfigStub()).on(RunnerEvents.RUNNER_START, () => runner.cancel(error)); await run_({ runner }); - assert.notCalled(BrowserRunner.prototype.addTestToRun); - assert.notCalled(BrowserRunner.prototype.waitTestsCompletion); - assert.notCalled(BrowserRunner.prototype.cancel); + assert.calledOnce(BrowserRunner.prototype.addTestToRun); + assert.calledOnce(BrowserRunner.prototype.waitTestsCompletion); + assert.calledOnceWith(BrowserRunner.prototype.cancel, error); }); it("should cancel all executing workers", async () => { diff --git a/test/src/runner/test-runner/insistant-test-runner.js b/test/src/runner/test-runner/insistant-test-runner.js index d7b58a4fe..6459e8625 100644 --- a/test/src/runner/test-runner/insistant-test-runner.js +++ b/test/src/runner/test-runner/insistant-test-runner.js @@ -66,6 +66,17 @@ describe("runner/test-runner/insistant-test-runner", () => { afterEach(() => sandbox.restore()); describe("run", () => { + it("should pass cancel error to a regular runner created later", async () => { + const error = new Error("Tests were stopped by the user"); + const cancelSpy = sandbox.spy(RegularTestRunner.prototype, "cancel"); + const runner = mkRunner_(); + + runner.cancel(error); + await run_({ runner }); + + assert.calledOnceWith(cancelSpy, error); + }); + it("should run test in regular test runner", async () => { const test = new Test({}); const config = makeConfigStub(); diff --git a/test/src/runner/test-runner/regular-test-runner.js b/test/src/runner/test-runner/regular-test-runner.js index 002638af3..3544a9e94 100644 --- a/test/src/runner/test-runner/regular-test-runner.js +++ b/test/src/runner/test-runner/regular-test-runner.js @@ -338,6 +338,38 @@ describe("runner/test-runner/regular-test-runner", () => { }); describe("TEST_FAIL event", () => { + it("should stop waiting for browser and emit cancel error", async () => { + const error = new Error("Tests were stopped by the user"); + const onFail = sinon.stub().named("onFail"); + let resolveBrowser; + BrowserAgent.prototype.getBrowser.returns(new Promise(resolve => (resolveBrowser = resolve))); + const runner = mkRunner_().on(Events.TEST_FAIL, onFail); + const workers = mkWorkers_(); + const runPromise = run_({ runner, workers }); + + await Promise.resolve(); + runner.cancel(error); + await runPromise; + resolveBrowser(stubBrowser_()); + + assert.notCalled(workers.runTest); + assert.calledOnceWith(onFail, sinon.match({ err: error })); + }); + + it("should be emitted with cancel error without running test in worker", async () => { + const error = new Error("Tests were stopped by the user"); + const onFail = sinon.stub().named("onFail"); + const runner = mkRunner_() + .on(Events.TEST_BEGIN, () => runner.cancel(error)) + .on(Events.TEST_FAIL, onFail); + const workers = mkWorkers_(); + + await run_({ runner, workers }); + + assert.notCalled(workers.runTest); + assert.calledOnceWith(onFail, sinon.match({ err: error })); + }); + it("should be emitted on test fail with test data", async () => { const test = new Test({}); const onFail = sinon.stub().named("onFail"); diff --git a/test/src/testplane.js b/test/src/testplane.js index 6531b332b..ab7762406 100644 --- a/test/src/testplane.js +++ b/test/src/testplane.js @@ -966,14 +966,14 @@ describe("testplane", () => { assert.notCalled(MainRunner.prototype.cancel); }); - it("should cancel test runner", async () => { + it("should cancel test runner immediately", async () => { + const err = new Error("Tests were stopped by the user"); testplane.on(RunnerEvents.RUNNER_START, () => { - testplane.halt(new Error("test error")); + testplane.halt(err, 0); + assert.calledOnceWith(MainRunner.prototype.cancel, err); }); - return testplane.run().finally(() => { - assert.calledOnce(MainRunner.prototype.cancel); - }); + return testplane.run(); }); it("should mark test run as failed", async () => {