diff --git a/src/JestExt/process-listeners.ts b/src/JestExt/process-listeners.ts index c5332d22..e9a70ee1 100644 --- a/src/JestExt/process-listeners.ts +++ b/src/JestExt/process-listeners.ts @@ -1,7 +1,7 @@ import * as vscode from 'vscode'; import { JestTotalResults, RunnerEvent } from 'jest-editor-support'; import { cleanAnsi, toErrorString } from '../helpers'; -import { JestProcess } from '../JestProcessManagement'; +import { JestProcess, ProcessStatus } from '../JestProcessManagement'; import { ListenerSession, ListTestFilesCallback } from './process-session'; import { Logging } from '../logging'; import { JestRunEvent } from './types'; @@ -279,10 +279,7 @@ export class RunTestListener extends AbstractProcessListener { // watch process should not exit unless we request it to be closed private handleWatchProcessCrash(process: JestProcess): string | undefined { - if ( - (process.request.type === 'watch-tests' || process.request.type === 'watch-all-tests') && - process.stopReason !== 'on-demand' - ) { + if (process.isWatchMode && process.status !== ProcessStatus.Cancelled) { const msg = `Jest process "${process.request.type}" ended unexpectedly`; this.logging('warn', msg); diff --git a/src/JestProcessManagement/JestProcess.ts b/src/JestProcessManagement/JestProcess.ts index 99191b39..3ab6fb68 100644 --- a/src/JestProcessManagement/JestProcess.ts +++ b/src/JestProcessManagement/JestProcess.ts @@ -4,7 +4,7 @@ import { Runner, RunnerEvent, Options } from 'jest-editor-support'; import { JestExtContext, WatchMode } from '../JestExt/types'; import { extensionId } from '../appGlobals'; import { Logging } from '../logging'; -import { JestProcessInfo, JestProcessRequest, UserDataType } from './types'; +import { JestProcessInfo, JestProcessRequest, ProcessStatus, UserDataType } from './types'; import { requestString } from './helper'; import { toFilePath, removeSurroundingQuote, escapeRegExp, shellQuote } from '../helpers'; @@ -23,20 +23,18 @@ interface RunnerTask { reject: (reason: unknown) => unknown; runner: Runner; } -export type StopReason = 'on-demand' | 'process-end'; let SEQ = 0; export class JestProcess implements JestProcessInfo { - static readonly stopHangTimeout = 500; - private task?: RunnerTask; private extContext: JestExtContext; private logging: Logging; - private _stopReason?: StopReason; public readonly id: string; private desc: string; public readonly request: JestProcessRequest; + public _status: ProcessStatus; + private autoStopTimer?: NodeJS.Timeout; constructor( extContext: JestExtContext, @@ -48,10 +46,11 @@ export class JestProcess implements JestProcessInfo { this.logging = extContext.loggingFactory.create(`JestProcess ${request.type}`); this.id = `${request.type}-${SEQ++}`; this.desc = `id: ${this.id}, request: ${requestString(request)}`; + this._status = ProcessStatus.Pending; } - public get stopReason(): StopReason | undefined { - return this._stopReason; + public get status(): ProcessStatus { + return this._status; } private get watchMode(): WatchMode { @@ -64,15 +63,39 @@ export class JestProcess implements JestProcessInfo { return WatchMode.None; } + public get isWatchMode(): boolean { + return this.watchMode !== WatchMode.None; + } + public toString(): string { - return `JestProcess: ${this.desc}; stopReason: ${this.stopReason}`; + return `JestProcess: ${this.desc}; status: "${this.status}"`; } - public start(): Promise { - this._stopReason = undefined; - return this.startRunner(); + + /** + * To prevent zombie process, this method will automatically stops the Jest process if it is running for too long. The process will be marked as "Cancelled" and stopped. + * Warning: This should only be called when you are certain the process should end soon, for example a non-watch mode process should end after the test results have been processed. + * @param delay The delay in milliseconds after which the process will be considered hung and stopped. Default is 30000 milliseconds (30 seconds ). + */ + public autoStop(delay = 30000, onStop?: (process: JestProcessInfo) => void): void { + if (this.status === ProcessStatus.Running) { + if (this.autoStopTimer) { + clearTimeout(this.autoStopTimer); + } + this.autoStopTimer = setTimeout(() => { + if (this.status === ProcessStatus.Running) { + console.warn( + `Jest Process "${this.id}": will be force closed due to the autoStop Timer (${delay} msec) ` + ); + this.stop(); + onStop?.(this); + } + }, delay); + } } + public stop(): Promise { - this._stopReason = 'on-demand'; + this._status = ProcessStatus.Cancelled; + if (!this.task) { this.logging('debug', 'nothing to stop, no pending runner/promise'); this.taskDone(); @@ -99,12 +122,19 @@ export class JestProcess implements JestProcessInfo { return `"${removeSurroundingQuote(aString)}"`; } - private startRunner(): Promise { + public start(): Promise { + if (this.status === ProcessStatus.Cancelled) { + this.logging('warn', `the runner task has been cancelled!`); + return Promise.resolve(); + } + if (this.task) { this.logging('warn', `the runner task has already started!`); return this.task.promise; } + this._status = ProcessStatus.Running; + const options: Options = { noColor: false, reporters: ['default', `"${this.getReporterPath()}"`], @@ -196,7 +226,13 @@ export class JestProcess implements JestProcessInfo { if (event === 'processClose' || event === 'processExit') { this.task?.resolve(); this.task = undefined; - this._stopReason = this._stopReason ?? 'process-end'; + + clearTimeout(this.autoStopTimer); + this.autoStopTimer = undefined; + + if (this._status !== ProcessStatus.Cancelled) { + this._status = ProcessStatus.Done; + } } this.request.listener.onEvent(this, event, ...args); } diff --git a/src/JestProcessManagement/JestProcessManager.ts b/src/JestProcessManagement/JestProcessManager.ts index 9e5119af..d4076d4c 100644 --- a/src/JestProcessManagement/JestProcessManager.ts +++ b/src/JestProcessManagement/JestProcessManager.ts @@ -6,6 +6,7 @@ import { Task, JestProcessInfo, UserDataType, + ProcessStatus, } from './types'; import { Logging } from '../logging'; import { createTaskQueue, TaskQueue } from './task-queue'; @@ -78,11 +79,13 @@ export class JestProcessManager implements TaskArrayFunctions { return; } const process = task.data; - try { - const promise = process.start(); - this.extContext.onRunEvent.fire({ type: 'process-start', process }); - await promise; + // process could be cancelled before it starts, so check before starting + if (process.status === ProcessStatus.Pending) { + const promise = process.start(); + this.extContext.onRunEvent.fire({ type: 'process-start', process }); + await promise; + } } catch (e) { this.logging('error', `${queue.name}: process failed to start:`, process, e); this.extContext.onRunEvent.fire({ diff --git a/src/JestProcessManagement/types.ts b/src/JestProcessManagement/types.ts index 115ac196..2981a9e0 100644 --- a/src/JestProcessManagement/types.ts +++ b/src/JestProcessManagement/types.ts @@ -15,12 +15,25 @@ export interface UserDataType { testError?: boolean; testItem?: vscode.TestItem; } +export enum ProcessStatus { + Pending = 'pending', + Running = 'running', + Cancelled = 'cancelled', + // process exited not because of cancellation + Done = 'done', +} + export interface JestProcessInfo { readonly id: string; readonly request: JestProcessRequest; // user data is a way to store data that is outside of the process managed by the processManager. // subsequent use of this data is up to the user but should be aware that multiple components might contribute to this data. userData?: UserDataType; + stop: () => Promise; + status: ProcessStatus; + isWatchMode: boolean; + // starting a timer to automatically kill the process after x milliseconds if the process is still running. + autoStop: (delay?: number, onStop?: (process: JestProcessInfo) => void) => void; } export type TaskStatus = 'running' | 'pending'; diff --git a/src/test-provider/jest-test-run.ts b/src/test-provider/jest-test-run.ts index 1671ec50..bb154707 100644 --- a/src/test-provider/jest-test-run.ts +++ b/src/test-provider/jest-test-run.ts @@ -1,6 +1,7 @@ import * as vscode from 'vscode'; import { JestExtOutput, JestOutputTerminal, OutputOptions } from '../JestExt/output-terminal'; import { JestTestProviderContext } from './test-provider-context'; +import { JestProcessInfo } from '../JestProcessManagement'; export type TestRunProtocol = Pick< vscode.TestRun, @@ -8,12 +9,16 @@ export type TestRunProtocol = Pick< >; export type CreateTestRun = (request: vscode.TestRunRequest, name: string) => vscode.TestRun; -export type EndProcessOption = { pid: string; delay?: number; reason?: string }; +export type EndProcessOption = { process: JestProcessInfo; delay?: number; reason?: string }; export type EndOption = EndProcessOption | { reason: string }; const isEndProcessOption = (arg?: EndOption): arg is EndProcessOption => - arg != null && 'pid' in arg; + arg != null && 'process' in arg; let SEQ = 0; +interface ProcessInfo { + process: JestProcessInfo; + timeoutId?: NodeJS.Timeout; +} /** * A wrapper class for vscode.TestRun to support * 1. JIT creation of TestRun @@ -23,11 +28,12 @@ let SEQ = 0; export class JestTestRun implements JestExtOutput, TestRunProtocol { private output: JestOutputTerminal; private _run?: vscode.TestRun; - private processes: Map; + private processes: Map; private verbose: boolean; private runCount = 0; public readonly name: string; private ignoreSkipped = false; + private isCancelled = false; constructor( name: string, @@ -50,16 +56,16 @@ export class JestTestRun implements JestExtOutput, TestRunProtocol { return !this._run; } - public addProcess(pid: string): void { - if (!this.processes.has(pid)) { - this.processes.set(pid, undefined); + public addProcess(process: JestProcessInfo): void { + if (!this.processes.has(process.id)) { + this.processes.set(process.id, { process }); } } /** * returns the underlying vscode.TestRun, if no run then create one. **/ - private vscodeRun(): vscode.TestRun { - if (!this._run) { + private vscodeRun(): vscode.TestRun | undefined { + if (!this._run && !this.isCancelled) { const runName = `${this.name} (${this.runCount++})`; this._run = this.createRun(this.request, runName); @@ -78,10 +84,10 @@ export class JestTestRun implements JestExtOutput, TestRunProtocol { // TestRunProtocol public enqueued = (test: vscode.TestItem): void => { - this.vscodeRun().enqueued(test); + this.vscodeRun()?.enqueued(test); }; public started = (test: vscode.TestItem): void => { - this.vscodeRun().started(test); + this.vscodeRun()?.started(test); }; public errored = ( test: vscode.TestItem, @@ -89,7 +95,7 @@ export class JestTestRun implements JestExtOutput, TestRunProtocol { duration?: number | undefined ): void => { const _msg = this.context.ext.settings.runMode.config.showInlineError ? message : []; - this.vscodeRun().errored(test, _msg, duration); + this.vscodeRun()?.errored(test, _msg, duration); }; public failed = ( test: vscode.TestItem, @@ -97,14 +103,14 @@ export class JestTestRun implements JestExtOutput, TestRunProtocol { duration?: number | undefined ): void => { const _msg = this.context.ext.settings.runMode.config.showInlineError ? message : []; - this.vscodeRun().failed(test, _msg, duration); + this.vscodeRun()?.failed(test, _msg, duration); }; public passed = (test: vscode.TestItem, duration?: number | undefined): void => { - this.vscodeRun().passed(test, duration); + this.vscodeRun()?.passed(test, duration); }; public skipped = (test: vscode.TestItem): void => { if (!this.ignoreSkipped) { - this.vscodeRun().skipped(test); + this.vscodeRun()?.skipped(test); } }; public end = (options?: EndOption): void => { @@ -113,10 +119,11 @@ export class JestTestRun implements JestExtOutput, TestRunProtocol { } const runName = this._run.name; if (isEndProcessOption(options)) { - const { pid, delay, reason } = options; - let timeoutId = this.processes.get(pid); - if (timeoutId) { - clearTimeout(timeoutId); + const { process, delay, reason } = options; + const pid = process.id; + const pInfo = this.processes.get(pid); + if (pInfo?.timeoutId) { + clearTimeout(pInfo?.timeoutId); } if (!delay) { @@ -125,7 +132,7 @@ export class JestTestRun implements JestExtOutput, TestRunProtocol { console.log(`JestTestRun "${runName}": process "${pid}" ended because: ${reason}`); } } else { - timeoutId = setTimeout(() => { + const timeoutId = setTimeout(() => { if (this.verbose) { console.log( `JestTestRun "${runName}": process "${pid}" ended after ${delay} msec delay because: ${reason}` @@ -136,7 +143,7 @@ export class JestTestRun implements JestExtOutput, TestRunProtocol { reason: `last process "${pid}" ended by ${reason}`, }); }, delay); - this.processes.set(pid, timeoutId); + this.processes.set(pid, { process, timeoutId }); if (this.verbose) { console.log( `JestTestRun "${runName}": starting a ${delay} msec timer to end process "${pid}" because: ${reason}` @@ -148,14 +155,42 @@ export class JestTestRun implements JestExtOutput, TestRunProtocol { if (this.processes.size > 0) { return; } - this._run.end(); - this._run = undefined; + this.endVscodeRun(options?.reason ?? 'all processes are done'); + }; + + endVscodeRun(reason: string): void { + /* istanbul ignore next */ + if (!this._run) { + return; + } if (this.verbose) { - console.log(`JestTestRun "${runName}": TestRun ended because: ${options?.reason}.`); + console.log(`JestTestRun "${this._run.name}": TestRun ended because: ${reason}.`); } - }; + this._run.end(); + this._run = undefined; + } + // set request for next time the underlying run needed to be created updateRequest(request: vscode.TestRunRequest): void { this.request = request; } + cancel(): void { + if (!this._run) { + return; + } + this.write(`\r\nTestRun "${this._run.name}" cancelled\r\n`, 'warn'); + + // close all processes and timer associated with this testRun + for (const p of this.processes.values()) { + p.process.stop(); + console.log(`process ${p.process.id} stopped because of user cancellation`); + + if (p.timeoutId) { + clearTimeout(p.timeoutId); + } + } + this.processes.clear(); + this.isCancelled = true; + this.endVscodeRun('user cancellation'); + } } diff --git a/src/test-provider/test-item-data.ts b/src/test-provider/test-item-data.ts index 4bfa4af7..57b9a1d0 100644 --- a/src/test-provider/test-item-data.ts +++ b/src/test-provider/test-item-data.ts @@ -11,7 +11,7 @@ import { TestSuitChangeEvent } from '../TestResults/test-result-events'; import { Debuggable, ItemCommand, TestItemData } from './types'; import { JestTestProviderContext } from './test-provider-context'; import { JestTestRun } from './jest-test-run'; -import { JestProcessInfo } from '../JestProcessManagement'; +import { JestProcessInfo, ProcessStatus } from '../JestProcessManagement'; import { GENERIC_ERROR, LONG_RUNNING_TESTS, getExitErrorDef } from '../errors'; import { tiContextManager } from './test-item-context-manager'; import { runModeDescription } from '../JestExt/run-mode'; @@ -84,7 +84,7 @@ abstract class TestItemDataBase implements TestItemData, JestRunnable, WithUri { run.write(msg, 'error'); run.end({ reason: 'failed to schedule test' }); } else { - run.addProcess(process.id); + run.addProcess(process); } } @@ -258,10 +258,23 @@ export class WorkspaceRoot extends TestItemDataBase { this.item.canResolveChildren = false; }; + // prevent a jest non-watch mode runs failed to stop, which could block the process queue from running other tests. + // by default it will wait 10 seconds before killing the process + private preventZombieProcess = (process: JestProcessInfo, delay = 10000): void => { + if (process.status === ProcessStatus.Running && !process.isWatchMode) { + process.autoStop(delay, () => { + this.context.output.write( + `Zombie jest process "${process.id}" is killed. Please investigate the root cause or file an issue.`, + 'warn' + ); + }); + } + }; + /** * invoked when external test result changed, this could be caused by the watch-mode or on-demand test run, includes vscode's runTest. - * We will try to find the run based on the event's id, if found, means a vscode runTest initiated such run, will use that run to - * ask all touched DocumentRoot to refresh both the test items and their states. + * We will use either existing run or creating a new one if none exist yet, + * and ask all touched DocumentRoot to refresh both the test items and their states. * * @param event */ @@ -279,7 +292,9 @@ export class WorkspaceRoot extends TestItemDataBase { } else { event.files.forEach((f) => this.addTestFile(f, (testRoot) => testRoot.discoverTest(run))); } - run.end({ pid: event.process.id, delay: 1000, reason: 'assertions-updated' }); + run.end({ process: event.process, delay: 1000, reason: 'assertions-updated' }); + this.preventZombieProcess(event.process); + break; } case 'result-matched': { @@ -365,19 +380,16 @@ export class WorkspaceRoot extends TestItemDataBase { private getJestRun(event: TypedRunEvent, createIfMissing?: false): JestTestRun | undefined; // istanbul ignore next private getJestRun(event: TypedRunEvent, createIfMissing = false): JestTestRun | undefined { - if (event.process.userData?.run) { - return event.process.userData.run; - } + let run = event.process.userData?.run; - if (createIfMissing) { + if (!run && createIfMissing) { const name = (event.process.userData?.run?.name ?? event.process.id) + `:${event.type}`; const testItem = this.getItemFromProcess(event.process) ?? this.item; - const run = this.createRun(name, testItem); - run.addProcess(event.process.id); + run = this.createRun(name, testItem); event.process.userData = { ...event.process.userData, run, testItem }; - - return run; } + run?.addProcess(event.process); + return run; } private runLog(type: string): void { @@ -419,7 +431,7 @@ export class WorkspaceRoot extends TestItemDataBase { event.process.userData = { ...(event.process.userData ?? {}), execError: true }; } this.runLog('finished'); - run.end({ pid: event.process.id, delay: 30000, reason: 'process end' }); + run.end({ process: event.process, delay: 30000, reason: 'process end' }); break; } case 'exit': { @@ -435,7 +447,7 @@ export class WorkspaceRoot extends TestItemDataBase { } } this.runLog('exited'); - run.end({ pid: event.process.id, delay: 1000, reason: 'process exit' }); + run.end({ process: event.process, delay: 1000, reason: 'process exit' }); break; } case 'long-run': { @@ -599,10 +611,7 @@ abstract class TestResultData extends TestItemDataBase { } createLocation(uri: vscode.Uri, zeroBasedLine = 0): vscode.Location { - return new vscode.Location( - uri, - new vscode.Range(new vscode.Position(zeroBasedLine, 0), new vscode.Position(zeroBasedLine, 0)) - ); + return new vscode.Location(uri, new vscode.Range(zeroBasedLine, 0, zeroBasedLine, 0)); } forEachChild(onTestData: (child: TestData) => void): void { diff --git a/src/test-provider/test-provider.ts b/src/test-provider/test-provider.ts index 2a4369f5..5a1cdf2a 100644 --- a/src/test-provider/test-provider.ts +++ b/src/test-provider/test-provider.ts @@ -163,6 +163,11 @@ export class JestTestProvider { const run = this.context.createTestRun(req, { name: `runTest: ${this.controller.id}`, }); + + cancelToken?.onCancellationRequested(() => { + run.cancel(); + }); + const tests = (req.include ?? this.getAllItems()).filter((t) => !req.exclude?.includes(t)); for (const test of tests) { diff --git a/tests/JestExt/process-listeners.test.ts b/tests/JestExt/process-listeners.test.ts index f13487c8..a95a0f60 100644 --- a/tests/JestExt/process-listeners.test.ts +++ b/tests/JestExt/process-listeners.test.ts @@ -11,6 +11,8 @@ import { } from '../../src/JestExt/process-listeners'; import { cleanAnsi, toErrorString } from '../../src/helpers'; import { extensionName } from '../../src/appGlobals'; +import { ProcessStatus } from '../../src/JestProcessManagement/types'; +import { JestTestProcessType } from '../../src/Settings'; class DummyListener extends AbstractProcessListener { constructor(session) { @@ -21,6 +23,16 @@ class DummyListener extends AbstractProcessListener { } } +const initMockProcess = (requestType: JestTestProcessType) => { + return { + id: `${requestType}-0`, + request: { type: requestType }, + stop: jest.fn(), + isWatchMode: requestType === 'watch-tests' || requestType === 'watch-all-tests', + status: ProcessStatus.Pending, + }; +}; + describe('jest process listeners', () => { let mockSession: any; let mockProcess; @@ -46,7 +58,7 @@ describe('jest process listeners', () => { onRunEvent: { fire: jest.fn() }, }, }; - mockProcess = { request: { type: 'watch' } }; + mockProcess = initMockProcess('watch-tests'); (cleanAnsi as jest.Mocked).mockImplementation((s) => s); }); afterEach(() => { @@ -62,7 +74,6 @@ describe('jest process listeners', () => { ${'executableOutput'} | ${false} ${'terminalError'} | ${true} `('listening for runner event $event, will log=$log', ({ event, log }) => { - mockProcess = { id: 'all-tests-0', request: { type: 'all-tests' } }; const listener = new AbstractProcessListener(mockSession); listener.onEvent(mockProcess, event, jest.fn(), jest.fn()); if (log) { @@ -86,7 +97,6 @@ describe('jest process listeners', () => { ${7} | ${'/bin/sh: react-scripts: command not found'} | ${false} ${8} | ${'/bin/sh: react-scripts: No such file or directory'} | ${false} `('case $case', ({ data, CmdNotFoundEnv }) => { - mockProcess = { id: 'all-tests-0', request: { type: 'all-tests' } }; const listener = new AbstractProcessListener(mockSession); listener.onEvent(mockProcess, 'executableStdErr', data, ''); expect((listener as any).CmdNotFoundEnv).toEqual(CmdNotFoundEnv); @@ -102,7 +112,6 @@ describe('jest process listeners', () => { ${5} | ${false} | ${136} | ${true} | ${true} ${6} | ${false} | ${1} | ${true} | ${false} `('case $case', ({ useLoginShell, exitCode, hasEnvIssue, retry }) => { - mockProcess = { id: 'all-tests-0', request: { type: 'all-tests' } }; mockSession.context.settings.shell.useLoginShell = useLoginShell; const listener = new DummyListener(mockSession); if (hasEnvIssue) { @@ -233,7 +242,6 @@ describe('jest process listeners', () => { show: jest.fn(), }; mockSession.context.updateWithData = jest.fn(); - mockProcess = { request: { type: 'watch-tests' } }; }); describe('can handle test result', () => { @@ -245,7 +253,6 @@ describe('jest process listeners', () => { expect.hasAssertions(); const listener = new RunTestListener(mockSession); const mockData = {}; - mockProcess = { id: 'mock-id' }; listener.onEvent(mockProcess, 'executableJSON', mockData); expect(mockSession.context.updateWithData).toHaveBeenCalledWith(mockData, mockProcess); }); @@ -461,8 +468,7 @@ describe('jest process listeners', () => { 'can detect and switch from watch to watch-all: #$seq', ({ processType, output, expectToRestart }) => { expect.hasAssertions(); - mockProcess.stop = jest.fn(); - mockProcess.request.type = processType; + mockProcess = initMockProcess(processType); const listener = new RunTestListener(mockSession); listener.onEvent(mockProcess, 'executableStdErr', Buffer.from(output)); @@ -478,7 +484,7 @@ describe('jest process listeners', () => { describe('upon process exit', () => { it('not report error if not a watch process', () => { expect.hasAssertions(); - mockProcess.request = { type: 'all-tests' }; + mockProcess = initMockProcess('all-tests'); const listener = new RunTestListener(mockSession); @@ -492,8 +498,7 @@ describe('jest process listeners', () => { }); it('not report error if watch run exit due to on-demand stop', () => { expect.hasAssertions(); - mockProcess.request = { type: 'watch-tests' }; - mockProcess.stopReason = 'on-demand'; + mockProcess.status = ProcessStatus.Cancelled; const listener = new RunTestListener(mockSession); @@ -508,7 +513,6 @@ describe('jest process listeners', () => { describe('if watch exit not caused by on-demand stop', () => { beforeEach(() => { mockSession.context.workspace = { name: 'workspace-xyz' }; - mockProcess.request = { type: 'watch-tests' }; }); it('will fire exit with error for watch run', () => { expect.hasAssertions(); @@ -526,7 +530,6 @@ describe('jest process listeners', () => { it('will always file error if error code > 1, regardless of request type', () => { expect.hasAssertions(); - mockProcess.request = { type: 'all-tests' }; const listener = new RunTestListener(mockSession); listener.onEvent(mockProcess, 'processClose', 127); @@ -547,7 +550,7 @@ describe('jest process listeners', () => { ${3} | ${false} | ${136} | ${true} ${4} | ${true} | ${127} | ${false} ${5} | ${'never'} | ${127} | ${false} - `('will retry with login-shell', ({ useLoginShell, exitCode, willRetry }) => { + `('case $case', ({ useLoginShell, exitCode, willRetry }) => { mockSession.context.settings.shell.useLoginShell = useLoginShell; const listener = new RunTestListener(mockSession); @@ -558,14 +561,12 @@ describe('jest process listeners', () => { expect(mockSession.context.onRunEvent.fire).not.toHaveBeenCalledWith( expect.objectContaining({ type: 'exit', - error: expect.anything(), }) ); } else { expect(mockSession.context.onRunEvent.fire).toHaveBeenCalledWith( expect.objectContaining({ type: 'exit', - error: expect.anything(), }) ); } diff --git a/tests/JestProcessManagement/JestProcess.test.ts b/tests/JestProcessManagement/JestProcess.test.ts index d18c83fa..d459c934 100644 --- a/tests/JestProcessManagement/JestProcess.test.ts +++ b/tests/JestProcessManagement/JestProcess.test.ts @@ -13,7 +13,7 @@ import { JestProcess, RunnerEvents } from '../../src/JestProcessManagement/JestP import { EventEmitter } from 'events'; import { mockProcessRequest, mockJestExtContext } from '../test-helper'; import { normalize } from 'path'; -import { JestProcessRequest } from '../../src/JestProcessManagement/types'; +import { JestProcessRequest, ProcessStatus } from '../../src/JestProcessManagement/types'; import { JestTestProcessType } from '../../src/Settings'; jest.unmock('path'); jest.mock('vscode', () => ({ @@ -59,7 +59,7 @@ describe('JestProcess', () => { jestProcess = new JestProcess(extContext, request); expect(`${jestProcess}`).toEqual(jestProcess.toString()); expect(jestProcess.toString()).toMatchInlineSnapshot( - `"JestProcess: id: all-tests-0, request: {"type":"all-tests","schedule":{"queue":"blocking"},"listener":"function"}; stopReason: undefined"` + `"JestProcess: id: all-tests-0, request: {"type":"all-tests","schedule":{"queue":"blocking"},"listener":"function"}; status: "pending""` ); }); describe('when creating', () => { @@ -67,7 +67,7 @@ describe('JestProcess', () => { const request = mockProcessRequest('all-tests'); jestProcess = new JestProcess(extContext, request); expect(jestProcess.request).toEqual(request); - expect(jestProcess.stopReason).toBeUndefined(); + expect(jestProcess.status).toEqual(ProcessStatus.Pending); }); it('uses loggingFactory to create logging', async () => { const request = mockProcessRequest('all-tests'); @@ -80,6 +80,23 @@ describe('JestProcess', () => { jestProcess = new JestProcess(extContext, request); expect(RunnerClassMock).not.toHaveBeenCalled(); }); + describe('isWatchMode', () => { + it.each` + requestType | isWatchMode + ${'all-tests'} | ${false} + ${'watch-tests'} | ${true} + ${'watch-all-tests'} | ${true} + ${'by-file'} | ${false} + ${'by-file-test'} | ${false} + ${'by-file-pattern'} | ${false} + ${'by-file-test-pattern'} | ${false} + ${'not-test'} | ${false} + `('for $requestType isWatchMode=$isWatchMode', ({ requestType, isWatchMode }) => { + const request = mockProcessRequest(requestType); + jestProcess = new JestProcess(extContext, request); + expect(jestProcess.isWatchMode).toEqual(isWatchMode); + }); + }); }); describe('when start', () => { it('returns a promise that resolved when process closed', async () => { @@ -88,23 +105,23 @@ describe('JestProcess', () => { const jp = new JestProcess(extContext, request); const p = jp.start(); + expect(jp.status).toEqual(ProcessStatus.Running); expect(RunnerClassMock).toHaveBeenCalled(); closeRunner(); await expect(p).resolves.not.toThrow(); - expect(jp.stopReason).toEqual('process-end'); + expect(jp.status).toEqual(ProcessStatus.Done); }); - it.each` - event | willEndProcess - ${'processClose'} | ${true} - ${'processExit'} | ${false} - ${'executableJSON'} | ${false} - ${'executableStdErr'} | ${false} - ${'executableOutput'} | ${false} - ${'terminalError'} | ${false} - `( - 'register and propagate $event to the request.listener', - async ({ event, willEndProcess }) => { + describe('register and propagate the following event to the request.listener', () => { + it.each` + event | willEndProcess + ${'processClose'} | ${true} + ${'processExit'} | ${false} + ${'executableJSON'} | ${false} + ${'executableStdErr'} | ${false} + ${'executableOutput'} | ${false} + ${'terminalError'} | ${false} + `('$event', async ({ event, willEndProcess }) => { expect.hasAssertions(); const request = mockRequest('all-tests'); const jp = new JestProcess(extContext, request); @@ -124,49 +141,51 @@ describe('JestProcess', () => { } await expect(p).resolves.not.toThrow(); - } - ); + }); + }); - it.each` - type | extraProperty | startArgs | includeReporter | extraRunnerOptions - ${'all-tests'} | ${undefined} | ${[false, false]} | ${true} | ${undefined} - ${'watch-tests'} | ${undefined} | ${[true, false]} | ${true} | ${undefined} - ${'watch-all-tests'} | ${undefined} | ${[true, true]} | ${true} | ${undefined} - ${'by-file'} | ${{ testFileName: '"c:\\a\\b.ts"' }} | ${[false, false]} | ${true} | ${{ args: { args: ['--runTestsByPath'] }, testFileNamePattern: '"C:\\a\\b.ts"' }} - ${'by-file'} | ${{ testFileName: '"c:\\a\\b.ts"', notTestFile: true }} | ${[false, false]} | ${true} | ${{ args: { args: ['--findRelatedTests', '"C:\\a\\b.ts"'] } }} - ${'by-file-test'} | ${{ testFileName: '"/a/b.js"', testNamePattern: 'a test' }} | ${[false, false]} | ${true} | ${{ args: { args: ['--runTestsByPath'] }, testFileNamePattern: '"/a/b.js"', testNamePattern: 'a\\ test' }} - ${'by-file-pattern'} | ${{ testFileNamePattern: '"c:\\a\\b.ts"' }} | ${[false, false]} | ${true} | ${{ args: { args: ['--testPathPattern', '"c:\\\\a\\\\b\\.ts"'] } }} - ${'by-file-test-pattern'} | ${{ testFileNamePattern: '/a/b.js', testNamePattern: 'a test' }} | ${[false, false]} | ${true} | ${{ args: { args: ['--testPathPattern', '"/a/b\\.js"'] }, testNamePattern: 'a\\ test' }} - ${'not-test'} | ${{ args: ['--listTests', '--watchAll=false'] }} | ${[false, false]} | ${false} | ${{ args: { args: ['--listTests'], replace: true } }} - `( - 'supports jest process request: $type', - async ({ type, extraProperty, startArgs, includeReporter, extraRunnerOptions }) => { - expect.hasAssertions(); - const request = mockRequest(type, extraProperty); - jestProcess = new JestProcess(extContext, request); - const p = jestProcess.start(); - const [, options] = RunnerClassMock.mock.calls[0]; - if (includeReporter) { - expect(options.reporters).toEqual([ - 'default', - `"${normalize('/my/vscode/extensions/out/reporter.js')}"`, - ]); - } else { - expect(options.reporters).toBeUndefined(); - } + describe('Supports the following jest process request type', () => { + it.each` + type | extraProperty | startArgs | includeReporter | extraRunnerOptions + ${'all-tests'} | ${undefined} | ${[false, false]} | ${true} | ${undefined} + ${'watch-tests'} | ${undefined} | ${[true, false]} | ${true} | ${undefined} + ${'watch-all-tests'} | ${undefined} | ${[true, true]} | ${true} | ${undefined} + ${'by-file'} | ${{ testFileName: '"c:\\a\\b.ts"' }} | ${[false, false]} | ${true} | ${{ args: { args: ['--runTestsByPath'] }, testFileNamePattern: '"C:\\a\\b.ts"' }} + ${'by-file'} | ${{ testFileName: '"c:\\a\\b.ts"', notTestFile: true }} | ${[false, false]} | ${true} | ${{ args: { args: ['--findRelatedTests', '"C:\\a\\b.ts"'] } }} + ${'by-file-test'} | ${{ testFileName: '"/a/b.js"', testNamePattern: 'a test' }} | ${[false, false]} | ${true} | ${{ args: { args: ['--runTestsByPath'] }, testFileNamePattern: '"/a/b.js"', testNamePattern: 'a\\ test' }} + ${'by-file-pattern'} | ${{ testFileNamePattern: '"c:\\a\\b.ts"' }} | ${[false, false]} | ${true} | ${{ args: { args: ['--testPathPattern', '"c:\\\\a\\\\b\\.ts"'] } }} + ${'by-file-test-pattern'} | ${{ testFileNamePattern: '/a/b.js', testNamePattern: 'a test' }} | ${[false, false]} | ${true} | ${{ args: { args: ['--testPathPattern', '"/a/b\\.js"'] }, testNamePattern: 'a\\ test' }} + ${'not-test'} | ${{ args: ['--listTests', '--watchAll=false'] }} | ${[false, false]} | ${false} | ${{ args: { args: ['--listTests'], replace: true } }} + `( + '$type', + async ({ type, extraProperty, startArgs, includeReporter, extraRunnerOptions }) => { + expect.hasAssertions(); + const request = mockRequest(type, extraProperty); + jestProcess = new JestProcess(extContext, request); + const p = jestProcess.start(); + const [, options] = RunnerClassMock.mock.calls[0]; + if (includeReporter) { + expect(options.reporters).toEqual([ + 'default', + `"${normalize('/my/vscode/extensions/out/reporter.js')}"`, + ]); + } else { + expect(options.reporters).toBeUndefined(); + } - if (extraRunnerOptions) { - const { args, ...restOptions } = extraRunnerOptions; - expect(options).toEqual(expect.objectContaining(restOptions)); - const { args: flags, replace } = args; - expect(options.args.replace).toEqual(replace); - expect(options.args.args).toEqual(expect.arrayContaining(flags)); + if (extraRunnerOptions) { + const { args, ...restOptions } = extraRunnerOptions; + expect(options).toEqual(expect.objectContaining(restOptions)); + const { args: flags, replace } = args; + expect(options.args.replace).toEqual(replace); + expect(options.args.args).toEqual(expect.arrayContaining(flags)); + } + expect(mockRunner.start).toHaveBeenCalledWith(...startArgs); + closeRunner(); + await p; } - expect(mockRunner.start).toHaveBeenCalledWith(...startArgs); - closeRunner(); - await p; - } - ); + ); + }); describe('common flags', () => { it.each` type | extraProperty | excludeWatch | withColors @@ -235,6 +254,16 @@ describe('JestProcess', () => { expect(mockRunner.start).toHaveBeenCalledTimes(1); expect(p1).toBe(p2); }); + it('starting a cancelled process will resolved immediate', async () => { + expect.hasAssertions(); + const request = mockRequest('all-tests'); + + jestProcess = new JestProcess(extContext, request); + jestProcess.stop(); + expect(jestProcess.status).toEqual(ProcessStatus.Cancelled); + await expect(jestProcess.start()).resolves.not.toThrow(); + expect(RunnerClassMock).not.toHaveBeenCalled(); + }); describe('can prepare testNamePattern for used in corresponding spawned shell', () => { it.each` platform | shell | testNamePattern | expected @@ -314,7 +343,7 @@ describe('JestProcess', () => { expect(mockRunner.closeProcess).toHaveBeenCalledTimes(1); expect(startDone).toBeFalsy(); expect(stopDone).toBeFalsy(); - expect(jestProcess.stopReason).toEqual('on-demand'); + expect(jestProcess.status).toEqual(ProcessStatus.Cancelled); closeRunner(); @@ -323,12 +352,86 @@ describe('JestProcess', () => { expect(startDone).toBeTruthy(); expect(stopDone).toBeTruthy(); - expect(jestProcess.stopReason).toEqual('on-demand'); + expect(jestProcess.status).toEqual(ProcessStatus.Cancelled); }); it('call stop before start will resolve right away', async () => { expect.hasAssertions(); await expect(jestProcess.stop()).resolves.not.toThrow(); - expect(jestProcess.stopReason).toEqual('on-demand'); + expect(jestProcess.status).toEqual(ProcessStatus.Cancelled); + }); + }); + describe('autoStop', () => { + let clearTimeoutSpy; + let stopSpy; + beforeAll(() => { + jest.useFakeTimers(); + }); + beforeEach(() => { + const request = mockRequest('all-tests'); + jestProcess = new JestProcess(extContext, request); + stopSpy = jest.spyOn(jestProcess, 'stop'); + clearTimeoutSpy = jest.spyOn(global, 'clearTimeout'); + }); + it('should stop the process after a delay', () => { + jestProcess.start(); + jestProcess.autoStop(30000); + + expect(jestProcess.status).toEqual(ProcessStatus.Running); + + jest.advanceTimersByTime(30000); + + expect(stopSpy).toHaveBeenCalled(); + expect(jestProcess.status).toEqual(ProcessStatus.Cancelled); + }); + + it('should call the onStop callback when the process is force closed', () => { + const onStopMock = jest.fn(); + jestProcess.start(); + jestProcess.autoStop(30000, onStopMock); + + expect(jestProcess.status).toEqual(ProcessStatus.Running); + + jest.advanceTimersByTime(30000); + + expect(stopSpy).toHaveBeenCalled(); + expect(jestProcess.status).toEqual(ProcessStatus.Cancelled); + expect(onStopMock).toHaveBeenCalledWith(jestProcess); + }); + + it('should not stop the process if it is not running', () => { + jestProcess.autoStop(30000); + + expect(jestProcess.status).not.toEqual(ProcessStatus.Running); + + jest.advanceTimersByTime(30000); + + expect(jestProcess.status).not.toEqual(ProcessStatus.Cancelled); + expect(stopSpy).not.toHaveBeenCalled(); + }); + + it('will clear previous timer if called again', () => { + jestProcess.start(); + jestProcess.autoStop(30000); + + expect(jestProcess.status).toEqual(ProcessStatus.Running); + expect(clearTimeoutSpy).not.toHaveBeenCalled(); + + jestProcess.autoStop(10000); + expect(clearTimeoutSpy).toHaveBeenCalled(); + expect(jestProcess.status).toEqual(ProcessStatus.Running); + + jest.advanceTimersByTime(10000); + + expect(jestProcess.status).toEqual(ProcessStatus.Cancelled); + expect(stopSpy).toHaveBeenCalledTimes(1); + }); + it('if process ends before the autoStop timer, it will clear the timer', () => { + jestProcess.start(); + jestProcess.autoStop(30000); + closeRunner(); + expect(clearTimeoutSpy).toHaveBeenCalledTimes(1); + jest.advanceTimersByTime(30000); + expect(stopSpy).not.toHaveBeenCalled(); }); }); }); diff --git a/tests/JestProcessManagement/JestProcessManager.test.ts b/tests/JestProcessManagement/JestProcessManager.test.ts index 099398be..25209357 100644 --- a/tests/JestProcessManagement/JestProcessManager.test.ts +++ b/tests/JestProcessManagement/JestProcessManager.test.ts @@ -7,7 +7,7 @@ import { JestProcessManager } from '../../src/JestProcessManagement/JestProcessM import { JestProcess } from '../../src/JestProcessManagement/JestProcess'; import { mockJestProcessContext, mockProcessRequest } from '../test-helper'; import * as taskQueue from '../../src/JestProcessManagement/task-queue'; -import { ScheduleStrategy } from '../../src/JestProcessManagement/types'; +import { ProcessStatus, ScheduleStrategy } from '../../src/JestProcessManagement/types'; interface ProcessState { inQ?: boolean; @@ -44,6 +44,7 @@ describe('JestProcessManager', () => { request, start: jest.fn().mockReturnValueOnce(promise), stop: jest.fn().mockImplementation(() => resolve('requested to stop')), + status: 'pending', resolve, reject, }; @@ -320,7 +321,7 @@ describe('JestProcessManager', () => { expect(getState(pm, process2)).toEqual({ inQ: true, started: false, qSize: 2 }); expect(getState(pm, process3)).toEqual({ inQ: false, qSize: 2 }); }); - it('will not schedule if there is pending process with the same content', async () => { + it('will not schedule if there is pending process with the same content', () => { expect.hasAssertions(); const schedule: ScheduleStrategy = { queue: 'blocking', @@ -339,29 +340,66 @@ describe('JestProcessManager', () => { // schedule the first process: no problem const process1 = mockJestProcess(request1); - let scheduled = await pm.scheduleJestProcess(request1); + let scheduled = pm.scheduleJestProcess(request1); expect(scheduled.id).toContain(request1.type); expect(getState(pm, process1)).toEqual({ inQ: true, started: true, qSize: 1 }); // schedule the 2nd process with request1, fine because process1 is running, not pending const process2 = mockJestProcess(request1); - scheduled = await pm.scheduleJestProcess(request1); + scheduled = pm.scheduleJestProcess(request1); expect(scheduled.id).toContain(request1.type); expect(getState(pm, process2)).toEqual({ inQ: true, started: false, qSize: 2 }); // schedule the 3rd one with different request2, should be fine, no dup const process3 = mockJestProcess(request2); - scheduled = await pm.scheduleJestProcess(request2); + scheduled = pm.scheduleJestProcess(request2); expect(scheduled.id).toContain(request2.type); expect(getState(pm, process3)).toEqual({ inQ: true, started: false, qSize: 3 }); // schedule the 4th one with request1, should be rejected as there is already one request pending const process4 = mockJestProcess(request1); - scheduled = await pm.scheduleJestProcess(request1); + scheduled = pm.scheduleJestProcess(request1); expect(scheduled).toBeUndefined(); expect(getState(pm, process4)).toEqual({ inQ: false, qSize: 3 }); }); }); + describe('handling cancelled process in queues', () => { + it('if a scheduled process is cancelled, it will be removed from the queue upon executing', async () => { + expect.hasAssertions(); + + const process1 = mockJestProcess( + mockProcessRequest('all-tests', { schedule: { queue: 'blocking-2' } }) + ); + const process2 = mockJestProcess( + mockProcessRequest('by-file', { schedule: { queue: 'blocking-2' } }) + ); + + const pm = new JestProcessManager(extContext); + pm.scheduleJestProcess(process1.request); + pm.scheduleJestProcess(process2.request); + expect(pm.numberOfProcesses()).toEqual(2); + + // only the first process should be running + expect(process1.start).toHaveBeenCalled(); + expect(process2.start).not.toHaveBeenCalled(); + expect(extContext.onRunEvent.fire).toHaveBeenCalledTimes(1); + + extContext.onRunEvent.fire.mockClear(); + + // cancel the 2nd process sitting in the queue + process2.status = ProcessStatus.Cancelled; + + // finish the first process + await process1.resolve(); + + // the queue will be zero but process2 was never run + expect(pm.numberOfProcesses()).toEqual(0); + expect(process2.start).not.toHaveBeenCalled(); + + // no runEvent should be fired either + expect(extContext.onRunEvent.fire).not.toHaveBeenCalled(); + }); + }); }); describe('stop processes', () => { @@ -486,10 +524,15 @@ describe('JestProcessManager', () => { const nonBlockingRequests = [ mockProcessRequest('not-test', { schedule: nonBlockingSchedule }), ]; + const pm = new JestProcessManager(extContext); - blockingRequests.forEach((r) => pm.scheduleJestProcess(r)); - blockingRequests2.forEach((r) => pm.scheduleJestProcess(r)); - nonBlockingRequests.forEach((r) => pm.scheduleJestProcess(r)); + // mock jest process and then schedule them + [blockingRequests, blockingRequests2, nonBlockingRequests].forEach((requests) => { + requests.forEach((r) => { + mockJestProcess(r); + pm.scheduleJestProcess(r); + }); + }); expect(pm.numberOfProcesses()).toEqual(4); }); diff --git a/tests/manual-mocks.ts b/tests/manual-mocks.ts index dfacd1ac..9b4050d6 100644 --- a/tests/manual-mocks.ts +++ b/tests/manual-mocks.ts @@ -9,23 +9,9 @@ jest.mock('../src/output-manager', () => ({ outputManager: { clearOutputOnRun: jest.fn() }, })); +import { mockRun } from './test-provider/test-helper'; jest.mock('../src/test-provider/jest-test-run', () => { return { - JestTestRun: jest.fn().mockImplementation((name: string) => { - return { - name, - enqueued: jest.fn(), - started: jest.fn(), - errored: jest.fn(), - failed: jest.fn(), - passed: jest.fn(), - skipped: jest.fn(), - end: jest.fn(), - write: jest.fn(), - addProcess: jest.fn(), - isClosed: jest.fn(() => false), - updateRequest: jest.fn(), - }; - }), + JestTestRun: jest.fn().mockImplementation((name) => mockRun({}, name)), }; }); diff --git a/tests/test-provider/jest-test-runt.test.ts b/tests/test-provider/jest-test-runt.test.ts index b80ece42..02719e67 100644 --- a/tests/test-provider/jest-test-runt.test.ts +++ b/tests/test-provider/jest-test-runt.test.ts @@ -5,6 +5,7 @@ jest.unmock('./test-helper'); import * as vscode from 'vscode'; import { JestTestRun } from '../../src/test-provider/jest-test-run'; +import { mockJestProcess } from './test-helper'; jest.useFakeTimers(); jest.spyOn(global, 'setTimeout'); @@ -208,8 +209,10 @@ describe('JestTestRun', () => { expect(run.end).toHaveBeenCalledTimes(1); }); it('can only close a run after all processes are done', () => { - jestRun.addProcess('p1'); - jestRun.addProcess('p2'); + const p1 = mockJestProcess('p1'); + const p2 = mockJestProcess('p2'); + jestRun.addProcess(p1); + jestRun.addProcess(p2); jestRun.enqueued(mockTestItem); expect(mockCreateTestRun).toHaveBeenCalledTimes(1); @@ -219,29 +222,29 @@ describe('JestTestRun', () => { expect(jestRun.isClosed()).toBe(false); expect(run.end).toHaveBeenCalledTimes(0); - jestRun.end({ pid: 'p1' }); + jestRun.end({ process: p1 }); expect(jestRun.isClosed()).toBe(false); expect(run.end).toHaveBeenCalledTimes(0); // when the last process is closed, the whole run is then closed - jestRun.end({ pid: 'p2' }); + jestRun.end({ process: p2 }); expect(jestRun.isClosed()).toBe(true); expect(run.end).toHaveBeenCalledTimes(1); }); it('with verbose, more information will be logged', () => { - const pid = '123'; + const process = mockJestProcess('123'); const consoleSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); mockContext.ext.settings.debugMode = true; jestRun = new JestTestRun('test', mockContext, mockRequest, mockCreateTestRun); - jestRun.addProcess(pid); + jestRun.addProcess(process); expect(mockCreateTestRun).toHaveBeenCalledTimes(0); jestRun.started(mockTestItem); expect(mockCreateTestRun).toHaveBeenCalledTimes(1); - jestRun.end({ pid, reason: 'testReason' }); - expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining(pid)); + jestRun.end({ process, reason: 'testReason' }); + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining(process.id)); expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('testReason')); }); describe('when close the process-run with delayed', () => { @@ -252,7 +255,7 @@ describe('JestTestRun', () => { expect(jestRun.isClosed()).toBe(false); // close with 1000 msec delay - jestRun.end({ pid: 'whatever', delay: 1000 }); + jestRun.end({ process: mockJestProcess('whatever'), delay: 1000 }); expect(jestRun.isClosed()).toBe(false); expect(run.end).not.toHaveBeenCalled(); @@ -266,16 +269,16 @@ describe('JestTestRun', () => { }); it('the subsequent end will cancel any running timer earlier', () => { - const pid = '123'; + const process = mockJestProcess('123'); jest.spyOn(global, 'clearTimeout'); - jestRun.addProcess(pid); + jestRun.addProcess(process); jestRun.started(mockTestItem); expect(mockCreateTestRun).toHaveBeenCalledTimes(1); const run = mockCreateTestRun.mock.results[0].value; - jestRun.end({ pid, delay: 30000 }); + jestRun.end({ process, delay: 30000 }); expect(jestRun.isClosed()).toBe(false); // advance timer by 1000 msec, the run is still not closed @@ -283,7 +286,7 @@ describe('JestTestRun', () => { expect(jestRun.isClosed()).toBe(false); // another end with 1000 msec delay, will cancel the previous 30000 msec delay - jestRun.end({ pid, delay: 1000 }); + jestRun.end({ process, delay: 1000 }); expect(global.clearTimeout).toHaveBeenCalledTimes(1); expect(jestRun.isClosed()).toBe(false); @@ -294,21 +297,21 @@ describe('JestTestRun', () => { expect(run.end).toHaveBeenCalledTimes(1); }); it('with verbose, more information will be logged', () => { - const pid = '123'; + const process = mockJestProcess('123'); const consoleSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); mockContext.ext.settings.debugMode = true; jestRun = new JestTestRun('test', mockContext, mockRequest, mockCreateTestRun); - jestRun.addProcess(pid); + jestRun.addProcess(process); expect(mockCreateTestRun).toHaveBeenCalledTimes(0); jestRun.started(mockTestItem); expect(mockCreateTestRun).toHaveBeenCalledTimes(1); - jestRun.end({ pid, delay: 1000, reason: 'testReason' }); + jestRun.end({ process, delay: 1000, reason: 'testReason' }); jest.runAllTimers(); - expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining(pid)); + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining(process.id)); expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('testReason')); }); }); @@ -368,4 +371,81 @@ describe('JestTestRun', () => { expect(mockCreateTestRun.mock.calls[1][0]).toBe(newRequest); }); }); + describe('cancel', () => { + it('should cancel the run', () => { + jestRun.started({} as any); + expect(mockCreateTestRun).toHaveBeenCalledTimes(1); + const run = mockCreateTestRun.mock.results[0].value; + expect(run.started).toHaveBeenCalled(); + + jestRun.cancel(); + expect(run.started).toHaveBeenCalled(); + expect(run.end).toHaveBeenCalled(); + }); + it('will stops all processes and timers associated with the run', () => { + const clearTimeoutSpy = jest.spyOn(global, 'clearTimeout'); + + const process1 = mockJestProcess('123'); + const process2 = mockJestProcess('456'); + jestRun.started({} as any); + jestRun.addProcess(process1); + jestRun.addProcess(process2); + expect(process1.stop).not.toHaveBeenCalled(); + expect(process2.stop).not.toHaveBeenCalled(); + clearTimeoutSpy.mockClear(); + + // a timer will be created + jestRun.end({ process: process1, delay: 1000 }); + expect(clearTimeoutSpy).toHaveBeenCalledTimes(0); + + jestRun.cancel(); + expect(process1.stop).toHaveBeenCalled(); + expect(process2.stop).toHaveBeenCalled(); + expect(clearTimeoutSpy).toHaveBeenCalledTimes(1); + }); + it('cancel a already cancelled run will do nothing', () => { + jestRun.started({} as any); + expect(mockCreateTestRun).toHaveBeenCalledTimes(1); + const run = mockCreateTestRun.mock.results[0].value; + expect(run.started).toHaveBeenCalled(); + + jestRun.cancel(); + expect(run.end).toHaveBeenCalledTimes(1); + + jestRun.cancel(); + expect(run.end).toHaveBeenCalledTimes(1); + }); + it('call run methods after cancel will do nothing', () => { + jestRun.started({} as any); + expect(mockCreateTestRun).toHaveBeenCalled(); + const run = mockCreateTestRun.mock.results[0].value; + expect(run.started).toHaveBeenCalled(); + run.started.mockClear(); + mockCreateTestRun.mockClear(); + + jestRun.cancel(); + expect(run.end).toHaveBeenCalled(); + + jestRun.started({} as any); + expect(run.started).not.toHaveBeenCalled(); + + jestRun.errored({} as any, {} as any); + expect(run.errored).not.toHaveBeenCalled(); + + jestRun.failed({} as any, {} as any); + expect(run.failed).not.toHaveBeenCalled(); + + jestRun.enqueued({} as any); + expect(run.enqueued).not.toHaveBeenCalled(); + + jestRun.passed({} as any); + expect(run.passed).not.toHaveBeenCalled(); + + jestRun.skipped({} as any); + expect(run.skipped).not.toHaveBeenCalled(); + + // no new run should be created + expect(mockCreateTestRun).not.toHaveBeenCalled(); + }); + }); }); diff --git a/tests/test-provider/test-helper.ts b/tests/test-provider/test-helper.ts index e5dc849d..42854efe 100644 --- a/tests/test-provider/test-helper.ts +++ b/tests/test-provider/test-helper.ts @@ -58,6 +58,10 @@ export const mockRun = (request?: any, name?: any): any => ({ enqueued: jest.fn(), appendOutput: jest.fn(), end: jest.fn(), + cancel: jest.fn(), + write: jest.fn(), + addProcess: jest.fn(), + updateRequest: jest.fn(), token: { onCancellationRequested: jest.fn() }, }); export const mockController = (): any => { @@ -92,3 +96,13 @@ export const mockController = (): any => { items: new TestItemCollectionMock(), }; }; + +export const mockJestProcess = (id: string, extra?: any): any => { + return { + id, + start: jest.fn(), + stop: jest.fn(), + status: 'pending', + ...(extra ?? {}), + }; +}; diff --git a/tests/test-provider/test-item-data.test.ts b/tests/test-provider/test-item-data.test.ts index a0c682e3..20c505d8 100644 --- a/tests/test-provider/test-item-data.test.ts +++ b/tests/test-provider/test-item-data.test.ts @@ -48,11 +48,12 @@ import { buildSourceContainer, } from '../../src/TestResults/match-by-context'; import * as path from 'path'; -import { mockController, mockExtExplorerContext } from './test-helper'; +import { mockController, mockExtExplorerContext, mockJestProcess } from './test-helper'; import * as errors from '../../src/errors'; import { ItemCommand } from '../../src/test-provider/types'; import { RunMode } from '../../src/JestExt/run-mode'; import { VirtualWorkspaceFolder } from '../../src/virtual-workspace-folder'; +import { ProcessStatus } from '../../src/JestProcessManagement'; const mockPathSep = (newSep: string) => { (path as jest.Mocked).setSep(newSep); @@ -70,7 +71,7 @@ const getChildItem = (item: vscode.TestItem, partialId: string): vscode.TestItem }; const mockScheduleProcess = (context, id = 'whatever') => { - const process: any = { id, request: { type: 'watch-tests' } }; + const process: any = mockJestProcess(id, { request: { type: 'watch-tests' } }); context.ext.session.scheduleProcess.mockImplementation((request, userData) => { process.request = request; process.userData = userData; @@ -185,7 +186,6 @@ describe('test-item-data', () => { (tiContextManager.setItemContext as jest.Mocked).mockClear(); (vscode.Location as jest.Mocked).mockReturnValue({}); - (toAbsoluteRootPath as jest.Mocked).mockImplementation((p) => p.uri.fsPath); }); describe('discover children', () => { @@ -317,6 +317,11 @@ describe('test-item-data', () => { (vscode.Range as jest.Mocked).mockImplementation((n1, n2, n3, n4) => ({ args: [n1, n2, n3, n4], })); + (vscode.Location as jest.Mocked).mockImplementation((uri, range) => ({ + uri, + range, + })); + (vscode.TestMessage as jest.Mocked).mockImplementation((message) => ({ message, })); @@ -382,6 +387,7 @@ describe('test-item-data', () => { // assertions are available now const a1 = helper.makeAssertion('test-a', 'KnownFail', [], [1, 0], { message: 'test error', + line: 2, }); const assertionContainer = buildAssertionContainer([a1]); const testSuiteResult: any = { @@ -394,9 +400,15 @@ describe('test-item-data', () => { // triggers testSuiteChanged event listener contextCreateTestRunSpy.mockClear(); mockedJestTestRun.mockClear(); + + // mock a non-watch process that is still running + const process = { + id: 'whatever', + request: { type: 'watch-tests' }, + }; context.ext.testResultProvider.events.testSuiteChanged.event.mock.calls[0][0]({ type: 'assertions-updated', - process: { id: 'whatever', request: { type: 'watch-tests' } }, + process, files: ['/ws-1/a.test.ts'], }); const run = mockedJestTestRun.mock.results[0].value; @@ -417,8 +429,44 @@ describe('test-item-data', () => { expect(docItem.children.size).toEqual(1); const tItem = getChildItem(docItem, 'test-a'); expect(tItem).not.toBeUndefined(); - expect(run.failed).toHaveBeenCalledWith(tItem, { message: a1.message }); expect(tItem.range).toEqual({ args: [1, 0, 1, 0] }); + + // error location within message + expect(run.failed).toHaveBeenCalledWith(tItem, { + message: a1.message, + location: expect.objectContaining({ range: { args: [1, 0, 1, 0] } }), + }); + }); + describe('will auto stop zombie process', () => { + it.each` + case | processStatus | isWatchMode | autoStopCalled + ${1} | ${ProcessStatus.Running} | ${true} | ${false} + ${2} | ${ProcessStatus.Running} | ${false} | ${true} + ${3} | ${ProcessStatus.Done} | ${false} | ${false} + ${4} | ${ProcessStatus.Done} | ${true} | ${false} + ${5} | ${ProcessStatus.Cancelled} | ${false} | ${false} + `('case $case', ({ processStatus, isWatchMode, autoStopCalled }) => { + const wsRoot = new WorkspaceRoot(context); + expect(wsRoot.item.children.size).toBe(0); + + const run = createTestRun(); + const process = { + id: 'whatever', + request: { type: 'watch-tests' }, + status: processStatus, + isWatchMode, + userData: { run }, + autoStop: jest.fn(), + }; + context.ext.testResultProvider.events.testSuiteChanged.event.mock.calls[0][0]({ + type: 'assertions-updated', + process, + files: [], + }); + // no tests items to be added + expect(run.end).toHaveBeenCalled(); + expect(process.autoStop).toHaveBeenCalledTimes(autoStopCalled ? 1 : 0); + }); }); it('if nothing is updated, output the message', () => { const wsRoot = new WorkspaceRoot(context); @@ -1579,7 +1627,7 @@ describe('test-item-data', () => { const endOption1 = jestRun.end.mock.calls[0][0]; expect(endOption1).toEqual( expect.objectContaining({ - pid: env.process.id, + process: env.process, delay: expect.anything(), reason: expect.anything(), }) @@ -1596,7 +1644,7 @@ describe('test-item-data', () => { const endOption2 = jestRun.end.mock.calls[1][0]; expect(endOption2).toEqual( expect.objectContaining({ - pid: env.process.id, + process: env.process, delay: expect.anything(), reason: expect.anything(), }) @@ -1610,7 +1658,7 @@ describe('test-item-data', () => { const endOption3 = jestRun.end.mock.calls[2][0]; expect(endOption3).toEqual( expect.objectContaining({ - pid: env.process.id, + process: env.process, delay: expect.anything(), reason: expect.anything(), }) diff --git a/tests/test-provider/test-provider.test.ts b/tests/test-provider/test-provider.test.ts index ff2d1ff2..dd01db88 100644 --- a/tests/test-provider/test-provider.test.ts +++ b/tests/test-provider/test-provider.test.ts @@ -216,7 +216,7 @@ describe('JestTestProvider', () => { return itemDataList; }; beforeEach(() => { - cancelToken = { onCancellationRequested: jest.fn() }; + cancelToken = { onCancellationRequested: jest.fn(), isCancellationRequested: false }; }); describe('debug tests', () => { let debugDone; @@ -319,7 +319,7 @@ describe('JestTestProvider', () => { // the run will be closed expect(jestRun.end).toHaveBeenCalled(); }); - it('cancellation means skip the rest of tests', async () => { + it('cancellation means stop the run and skip the rest of tests', async () => { expect.hasAssertions(); extExplorerContextMock.debugTests.mockImplementation(controlled); @@ -336,6 +336,7 @@ describe('JestTestProvider', () => { // a run is created expect(JestTestRun).toHaveBeenCalledTimes(1); const jestRun = mockedJestTestRun.mock.results[0].value; + const onCancel = cancelToken.onCancellationRequested.mock.calls[0][0]; expect(extExplorerContextMock.debugTests).toHaveBeenCalledTimes(1); @@ -344,6 +345,8 @@ describe('JestTestProvider', () => { // cancel the run during 2nd debug, the 3rd one should be skipped cancelToken.isCancellationRequested = true; + onCancel(); + expect(jestRun.cancel).toHaveBeenCalled(); await finishDebug(); expect(extExplorerContextMock.debugTests).toHaveBeenCalledTimes(2); @@ -386,14 +389,13 @@ describe('JestTestProvider', () => { const resolveSchedule = (_r, resolve) => { resolve(); }; - it.each` - scheduleTest | isCancelled | state - ${resolveSchedule} | ${false} | ${undefined} - ${resolveSchedule} | ${true} | ${'skipped'} - ${throwError} | ${false} | ${'errored'} - `( - 'run test should always resolve: schedule test pid = $pid, isCancelled=$isCancelled => state? $state', - async ({ scheduleTest, isCancelled, state }) => { + describe('run test should update test item status', () => { + it.each` + case | scheduleTest | isCancelled | state + ${1} | ${resolveSchedule} | ${false} | ${undefined} + ${2} | ${resolveSchedule} | ${true} | ${'skipped'} + ${3} | ${throwError} | ${false} | ${'errored'} + `('case $case', async ({ scheduleTest, isCancelled, state }) => { expect.hasAssertions(); const testProvider = new JestTestProvider(extExplorerContextMock); @@ -436,8 +438,8 @@ describe('JestTestProvider', () => { expect('unhandled state type').toBeUndefined(); break; } - } - ); + }); + }); it('running tests in parallel', async () => { expect.hasAssertions(); @@ -468,7 +470,7 @@ describe('JestTestProvider', () => { expect(jestRun.end).toHaveBeenCalledTimes(itemDataList.length + 1); }); - it('cancellation is passed to the itemData to handle', async () => { + it('cancellation will cancel all testRun and items', async () => { expect.hasAssertions(); const testProvider = new JestTestProvider(extExplorerContextMock); @@ -480,13 +482,16 @@ describe('JestTestProvider', () => { profile: { kind: vscode.TestRunProfileKind.Run }, }; const p = testProvider.runTests(request, cancelToken); + const onCancel = cancelToken.onCancellationRequested.mock.calls[0][0]; - // cancel after run + // cancel the run cancelToken.isCancellationRequested = true; + onCancel(); // a run is already created expect(JestTestRun).toHaveBeenCalledTimes(1); const jestRun = mockedJestTestRun.mock.results[0].value; + expect(jestRun.cancel).toHaveBeenCalled(); itemDataList.forEach((d) => { expect(d.scheduleTest).toHaveBeenCalled();