From fdeba7d869aeb247aa5b26d7f02fc5b5c1b392df Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Fri, 20 May 2022 13:40:39 -0700 Subject: [PATCH] fix task status issues (#149976) --- .../tasks/browser/taskTerminalStatus.ts | 39 +++--- .../test/browser/taskTerminalStatus.test.ts | 117 ++++++++++++++++++ 2 files changed, 140 insertions(+), 16 deletions(-) create mode 100644 src/vs/workbench/contrib/tasks/test/browser/taskTerminalStatus.test.ts diff --git a/src/vs/workbench/contrib/tasks/browser/taskTerminalStatus.ts b/src/vs/workbench/contrib/tasks/browser/taskTerminalStatus.ts index ced0fd8cf8f..3c6dc5ca458 100644 --- a/src/vs/workbench/contrib/tasks/browser/taskTerminalStatus.ts +++ b/src/vs/workbench/contrib/tasks/browser/taskTerminalStatus.ts @@ -5,10 +5,10 @@ import * as nls from 'vs/nls'; import { Codicon } from 'vs/base/common/codicons'; -import { Disposable } from 'vs/base/common/lifecycle'; +import { Disposable, IDisposable } from 'vs/base/common/lifecycle'; import Severity from 'vs/base/common/severity'; import { AbstractProblemCollector, StartStopProblemCollector } from 'vs/workbench/contrib/tasks/common/problemCollectors'; -import { TaskEvent, TaskEventKind } from 'vs/workbench/contrib/tasks/common/tasks'; +import { TaskEvent, TaskEventKind, TaskRunType } from 'vs/workbench/contrib/tasks/common/tasks'; import { ITaskService, Task } from 'vs/workbench/contrib/tasks/common/taskService'; import { ITerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminal'; import { ITerminalStatus } from 'vs/workbench/contrib/terminal/browser/terminalStatusList'; @@ -17,15 +17,18 @@ import { spinningLoading } from 'vs/platform/theme/common/iconRegistry'; interface TerminalData { terminal: ITerminalInstance; + task: Task; status: ITerminalStatus; problemMatcher: AbstractProblemCollector; + taskRunEnded: boolean; + disposeListener?: IDisposable; } const TASK_TERMINAL_STATUS_ID = 'task_terminal_status'; -const ACTIVE_TASK_STATUS: ITerminalStatus = { id: TASK_TERMINAL_STATUS_ID, icon: spinningLoading, severity: Severity.Info, tooltip: nls.localize('taskTerminalStatus.active', "Task is running") }; -const SUCCEEDED_TASK_STATUS: ITerminalStatus = { id: TASK_TERMINAL_STATUS_ID, icon: Codicon.check, severity: Severity.Info, tooltip: nls.localize('taskTerminalStatus.succeeded', "Task succeeded") }; +export const ACTIVE_TASK_STATUS: ITerminalStatus = { id: TASK_TERMINAL_STATUS_ID, icon: spinningLoading, severity: Severity.Info, tooltip: nls.localize('taskTerminalStatus.active', "Task is running") }; +export const SUCCEEDED_TASK_STATUS: ITerminalStatus = { id: TASK_TERMINAL_STATUS_ID, icon: Codicon.check, severity: Severity.Info, tooltip: nls.localize('taskTerminalStatus.succeeded', "Task succeeded") }; const SUCCEEDED_INACTIVE_TASK_STATUS: ITerminalStatus = { id: TASK_TERMINAL_STATUS_ID, icon: Codicon.check, severity: Severity.Info, tooltip: nls.localize('taskTerminalStatus.succeededInactive', "Task succeeded and waiting...") }; -const FAILED_TASK_STATUS: ITerminalStatus = { id: TASK_TERMINAL_STATUS_ID, icon: Codicon.error, severity: Severity.Error, tooltip: nls.localize('taskTerminalStatus.errors', "Task has errors") }; +export const FAILED_TASK_STATUS: ITerminalStatus = { id: TASK_TERMINAL_STATUS_ID, icon: Codicon.error, severity: Severity.Error, tooltip: nls.localize('taskTerminalStatus.errors', "Task has errors") }; const FAILED_INACTIVE_TASK_STATUS: ITerminalStatus = { id: TASK_TERMINAL_STATUS_ID, icon: Codicon.error, severity: Severity.Error, tooltip: nls.localize('taskTerminalStatus.errorsInactive', "Task has errors and is waiting...") }; const WARNING_TASK_STATUS: ITerminalStatus = { id: TASK_TERMINAL_STATUS_ID, icon: Codicon.warning, severity: Severity.Warning, tooltip: nls.localize('taskTerminalStatus.warnings', "Task has warnings") }; const WARNING_INACTIVE_TASK_STATUS: ITerminalStatus = { id: TASK_TERMINAL_STATUS_ID, icon: Codicon.warning, severity: Severity.Warning, tooltip: nls.localize('taskTerminalStatus.warningsInactive', "Task has warnings and is waiting...") }; @@ -33,7 +36,7 @@ const INFO_TASK_STATUS: ITerminalStatus = { id: TASK_TERMINAL_STATUS_ID, icon: C const INFO_INACTIVE_TASK_STATUS: ITerminalStatus = { id: TASK_TERMINAL_STATUS_ID, icon: Codicon.info, severity: Severity.Info, tooltip: nls.localize('taskTerminalStatus.infosInactive', "Task has infos and is waiting...") }; export class TaskTerminalStatus extends Disposable { - private terminalMap: Map = new Map(); + private terminalMap: Map = new Map(); constructor(taskService: ITaskService) { super(); @@ -50,15 +53,15 @@ export class TaskTerminalStatus extends Disposable { addTerminal(task: Task, terminal: ITerminalInstance, problemMatcher: AbstractProblemCollector) { const status: ITerminalStatus = { id: TASK_TERMINAL_STATUS_ID, severity: Severity.Info }; terminal.statusList.add(status); - this.terminalMap.set(task, { terminal, status, problemMatcher }); + this.terminalMap.set(task._id, { terminal, task, status, problemMatcher, taskRunEnded: false }); } private terminalFromEvent(event: TaskEvent): TerminalData | undefined { - if (!event.__task || !this.terminalMap.get(event.__task)) { + if (!event.__task) { return undefined; } - return this.terminalMap.get(event.__task); + return this.terminalMap.get(event.__task._id); } private eventEnd(event: TaskEvent) { @@ -66,13 +69,11 @@ export class TaskTerminalStatus extends Disposable { if (!terminalData) { return; } - - this.terminalMap.delete(event.__task!); - + terminalData.taskRunEnded = true; terminalData.terminal.statusList.remove(terminalData.status); if ((event.exitCode === 0) && (terminalData.problemMatcher.numberOfMatches === 0)) { terminalData.terminal.statusList.add(SUCCEEDED_TASK_STATUS); - } else if (terminalData.problemMatcher.maxMarkerSeverity === MarkerSeverity.Error) { + } else if (event.exitCode || terminalData.problemMatcher.maxMarkerSeverity === MarkerSeverity.Error) { terminalData.terminal.statusList.add(FAILED_TASK_STATUS); } else if (terminalData.problemMatcher.maxMarkerSeverity === MarkerSeverity.Warning) { terminalData.terminal.statusList.add(WARNING_TASK_STATUS); @@ -83,7 +84,7 @@ export class TaskTerminalStatus extends Disposable { private eventInactive(event: TaskEvent) { const terminalData = this.terminalFromEvent(event); - if (!terminalData || !terminalData.problemMatcher) { + if (!terminalData || !terminalData.problemMatcher || terminalData.taskRunEnded) { return; } terminalData.terminal.statusList.remove(terminalData.status); @@ -103,10 +104,16 @@ export class TaskTerminalStatus extends Disposable { if (!terminalData) { return; } - + if (!terminalData.disposeListener) { + terminalData.disposeListener = terminalData.terminal.onDisposed(() => { + this.terminalMap.delete(event.__task?._id!); + terminalData.disposeListener?.dispose(); + }); + } + terminalData.taskRunEnded = false; terminalData.terminal.statusList.remove(terminalData.status); // We don't want to show an infinite status for a background task that doesn't have a problem matcher. - if ((terminalData.problemMatcher instanceof StartStopProblemCollector) || (terminalData.problemMatcher?.problemMatchers.length > 0)) { + if ((terminalData.problemMatcher instanceof StartStopProblemCollector) || (terminalData.problemMatcher?.problemMatchers.length > 0) || event.runType === TaskRunType.SingleRun) { terminalData.terminal.statusList.add(ACTIVE_TASK_STATUS); } } diff --git a/src/vs/workbench/contrib/tasks/test/browser/taskTerminalStatus.test.ts b/src/vs/workbench/contrib/tasks/test/browser/taskTerminalStatus.test.ts new file mode 100644 index 00000000000..3b00f83eaf5 --- /dev/null +++ b/src/vs/workbench/contrib/tasks/test/browser/taskTerminalStatus.test.ts @@ -0,0 +1,117 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { ok } from 'assert'; +import { Emitter, Event } from 'vs/base/common/event'; +import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; +import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; +import { ACTIVE_TASK_STATUS, FAILED_TASK_STATUS, SUCCEEDED_TASK_STATUS, TaskTerminalStatus } from 'vs/workbench/contrib/tasks/browser/taskTerminalStatus'; +import { AbstractProblemCollector } from 'vs/workbench/contrib/tasks/common/problemCollectors'; +import { CommonTask, TaskEvent, TaskEventKind, TaskRunType } from 'vs/workbench/contrib/tasks/common/tasks'; +import { ITaskService, Task } from 'vs/workbench/contrib/tasks/common/taskService'; +import { ITerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminal'; +import { ITerminalStatus, ITerminalStatusList, TerminalStatusList } from 'vs/workbench/contrib/terminal/browser/terminalStatusList'; + +class TestTaskService implements Partial { + private readonly _onDidStateChange: Emitter = new Emitter(); + public get onDidStateChange(): Event { + return this._onDidStateChange.event; + } + public triggerStateChange(event: TaskEvent): void { + this._onDidStateChange.fire(event); + } +} + +class TestTerminal implements Partial { + statusList: TerminalStatusList = new TerminalStatusList(new TestConfigurationService()); +} + +class TestTask extends CommonTask { + protected getFolderId(): string | undefined { + throw new Error('Method not implemented.'); + } + protected fromObject(object: any): Task { + throw new Error('Method not implemented.'); + } +} + +class TestProblemCollector implements Partial { + +} + +suite('Task Terminal Status', () => { + let instantiationService: TestInstantiationService; + let taskService: TestTaskService; + let taskTerminalStatus: TaskTerminalStatus; + let testTerminal: ITerminalInstance; + let testTask: Task; + let problemCollector: AbstractProblemCollector; + setup(() => { + instantiationService = new TestInstantiationService(); + taskService = new TestTaskService(); + taskTerminalStatus = instantiationService.createInstance(TaskTerminalStatus, taskService); + testTerminal = instantiationService.createInstance(TestTerminal); + testTask = instantiationService.createInstance(TestTask); + problemCollector = instantiationService.createInstance(TestProblemCollector); + }); + test('Should add failed status when there is an exit code on task end', async () => { + taskTerminalStatus.addTerminal(testTask, testTerminal, problemCollector); + taskService.triggerStateChange({ kind: TaskEventKind.ProcessStarted }); + assertStatus(testTerminal.statusList, ACTIVE_TASK_STATUS); + taskService.triggerStateChange({ kind: TaskEventKind.Inactive }); + assertStatus(testTerminal.statusList, SUCCEEDED_TASK_STATUS); + taskService.triggerStateChange({ kind: TaskEventKind.End, exitCode: 2 }); + await poll(async () => Promise.resolve(), () => testTerminal?.statusList.primary?.id === FAILED_TASK_STATUS.id, 'terminal status should be updated'); + }); + test('Should add active status when a non-background task is run for a second time in the same terminal', async () => { + taskTerminalStatus.addTerminal(testTask, testTerminal, problemCollector); + taskService.triggerStateChange({ kind: TaskEventKind.ProcessStarted }); + assertStatus(testTerminal.statusList, ACTIVE_TASK_STATUS); + taskService.triggerStateChange({ kind: TaskEventKind.Inactive }); + assertStatus(testTerminal.statusList, SUCCEEDED_TASK_STATUS); + taskService.triggerStateChange({ kind: TaskEventKind.ProcessStarted, runType: TaskRunType.SingleRun }); + assertStatus(testTerminal.statusList, ACTIVE_TASK_STATUS); + taskService.triggerStateChange({ kind: TaskEventKind.Inactive }); + assertStatus(testTerminal.statusList, SUCCEEDED_TASK_STATUS); + }); +}); + +function assertStatus(actual: ITerminalStatusList, expected: ITerminalStatus): void { + ok(actual.statuses.length === 1, '# of statuses'); + ok(actual.primary?.id === expected.id, 'ID'); + ok(actual.primary?.severity === expected.severity, 'Severity'); +} + +async function poll( + fn: () => Thenable, + acceptFn: (result: T) => boolean, + timeoutMessage: string, + retryCount: number = 200, + retryInterval: number = 10 // millis +): Promise { + let trial = 1; + let lastError: string = ''; + + while (true) { + if (trial > retryCount) { + throw new Error(`Timeout: ${timeoutMessage} after ${(retryCount * retryInterval) / 1000} seconds.\r${lastError}`); + } + + let result; + try { + result = await fn(); + if (acceptFn(result)) { + return result; + } else { + lastError = 'Did not pass accept function'; + } + } catch (e: any) { + lastError = Array.isArray(e.stack) ? e.stack.join('\n') : e.stack; + } + + await new Promise(resolve => setTimeout(resolve, retryInterval)); + trial++; + } +}