From e887486e4d12d774074a1556a4c0e25b822b2e26 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Wed, 9 Oct 2019 19:11:09 -0400 Subject: [PATCH 1/8] Fixes #48513 - buffers terminal onData events --- .../api/browser/mainThreadTerminalService.ts | 17 +++++++- .../api/common/extHostTerminalService.ts | 17 ++++++-- .../contrib/terminal/common/terminal.ts | 40 +++++++++++++++++++ 3 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index cce3bcfa8cc..db16317f241 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { DisposableStore, Disposable } from 'vs/base/common/lifecycle'; -import { IShellLaunchConfig, ITerminalProcessExtHostProxy, ISpawnExtHostProcessRequest, ITerminalDimensions, EXT_HOST_CREATION_DELAY, IAvailableShellsRequest, IDefaultShellAndArgsRequest, IStartExtensionTerminalRequest } from 'vs/workbench/contrib/terminal/common/terminal'; +import { IShellLaunchConfig, ITerminalProcessExtHostProxy, ISpawnExtHostProcessRequest, ITerminalDimensions, EXT_HOST_CREATION_DELAY, IAvailableShellsRequest, IDefaultShellAndArgsRequest, IStartExtensionTerminalRequest, getTerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminal'; import { ExtHostContext, ExtHostTerminalServiceShape, MainThreadTerminalServiceShape, MainContext, IExtHostContext, IShellLaunchConfigDto, TerminalLaunchConfig, ITerminalDimensionsDto } from 'vs/workbench/api/common/extHost.protocol'; import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; import { URI } from 'vs/base/common/uri'; @@ -337,6 +337,19 @@ class TerminalDataEventTracker extends Disposable { } private _registerInstance(instance: ITerminalInstance): void { - this._register(instance.onData(e => this._callback(instance.id, e))); + const bufferer = getTerminalDataBufferer(instance.id, this._callback); + const disposables = [bufferer, instance.onData(bufferer.onData)]; + + for (const d of disposables) { + this._register(d); + } + + this._register(this._terminalService.onInstanceDisposed(i => { + if (i.id === instance.id) { + for (const d of disposables) { + d.dispose(); + } + } + })); } } diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index 368f057d8d8..cfd8e90b8e1 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -4,12 +4,13 @@ *--------------------------------------------------------------------------------------------*/ import * as vscode from 'vscode'; +import { IDisposable } from 'vs/base/common/lifecycle'; import { Event, Emitter } from 'vs/base/common/event'; import { ExtHostTerminalServiceShape, MainContext, MainThreadTerminalServiceShape, IShellLaunchConfigDto, IShellDefinitionDto, IShellAndArgsDto, ITerminalDimensionsDto } from 'vs/workbench/api/common/extHost.protocol'; import { ExtHostConfigProvider } from 'vs/workbench/api/common/extHostConfiguration'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { URI, UriComponents } from 'vs/base/common/uri'; -import { EXT_HOST_CREATION_DELAY, ITerminalChildProcess, ITerminalDimensions } from 'vs/workbench/contrib/terminal/common/terminal'; +import { EXT_HOST_CREATION_DELAY, ITerminalChildProcess, ITerminalDimensions, getTerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminal'; import { timeout } from 'vs/base/common/async'; import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService'; @@ -464,8 +465,12 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ protected _setupExtHostProcessListeners(id: number, p: ITerminalChildProcess): void { p.onProcessReady((e: { pid: number, cwd: string }) => this._proxy.$sendProcessReady(id, e.pid, e.cwd)); p.onProcessTitleChanged(title => this._proxy.$sendProcessTitle(id, title)); - p.onProcessData(data => this._proxy.$sendProcessData(id, data)); - p.onProcessExit(exitCode => this._onProcessExit(id, exitCode)); + + const bufferer = getTerminalDataBufferer(id, this._proxy.$sendProcessData); + const disposables = [bufferer, p.onProcessData(bufferer.onData)]; + + p.onProcessExit(exitCode => this._onProcessExit(id, exitCode, disposables)); + if (p.onProcessOverrideDimensions) { p.onProcessOverrideDimensions(e => this._proxy.$sendOverrideDimensions(id, e)); } @@ -503,7 +508,11 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ return id; } - private _onProcessExit(id: number, exitCode: number): void { + private _onProcessExit(id: number, exitCode: number, disposables: IDisposable[]): void { + for (const d of disposables) { + d.dispose(); + } + // Remove process reference delete this._terminalProcesses[id]; diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index dc364d04f15..bbcab2f07b3 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -471,3 +471,43 @@ export const enum TERMINAL_COMMAND_ID { NAVIGATION_MODE_FOCUS_NEXT = 'workbench.action.terminal.navigationModeFocusNext', NAVIGATION_MODE_FOCUS_PREVIOUS = 'workbench.action.terminal.navigationModeFocusPrevious' } + +interface TerminalDataBuffer { + timeoutId: any; + data: string[]; +} + +export interface TerminalDataBufferer extends IDisposable { + onData: (e: string) => void; +} + +const terminalBufferMap = new Map(); + +export function getTerminalDataBufferer(id: number, callback: (id: number, data: string) => void): TerminalDataBufferer { + return { + dispose: () => { + const buffer = terminalBufferMap.get(id); + if (buffer) { + clearTimeout(buffer.timeoutId); + terminalBufferMap.delete(id); + } + }, + onData: (e: string) => { + let buffer = terminalBufferMap.get(id); + if (buffer) { + buffer.data.push(e); + + return; + } + + buffer = { + data: [e], + timeoutId: setTimeout(() => { + terminalBufferMap.delete(id); + callback(id, buffer!.data.join('')); + }, 5) + }; + terminalBufferMap.set(id, buffer); + } + }; +} From 1ab47af535de89c9d8cceb78b321c91eb3abb9c4 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Fri, 11 Oct 2019 14:55:37 -0400 Subject: [PATCH 2/8] Fixes #48513 - updated from review comments --- .../api/browser/mainThreadTerminalService.ts | 27 +-- .../api/common/extHostTerminalService.ts | 21 +- .../contrib/terminal/common/terminal.ts | 40 ---- .../terminal/common/terminalDataBuffering.ts | 57 ++++++ .../test/common/terminalDataBuffering.test.ts | 193 ++++++++++++++++++ 5 files changed, 272 insertions(+), 66 deletions(-) create mode 100644 src/vs/workbench/contrib/terminal/common/terminalDataBuffering.ts create mode 100644 src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index db16317f241..de12c271b51 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { DisposableStore, Disposable } from 'vs/base/common/lifecycle'; -import { IShellLaunchConfig, ITerminalProcessExtHostProxy, ISpawnExtHostProcessRequest, ITerminalDimensions, EXT_HOST_CREATION_DELAY, IAvailableShellsRequest, IDefaultShellAndArgsRequest, IStartExtensionTerminalRequest, getTerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminal'; +import { IShellLaunchConfig, ITerminalProcessExtHostProxy, ISpawnExtHostProcessRequest, ITerminalDimensions, EXT_HOST_CREATION_DELAY, IAvailableShellsRequest, IDefaultShellAndArgsRequest, IStartExtensionTerminalRequest } from 'vs/workbench/contrib/terminal/common/terminal'; import { ExtHostContext, ExtHostTerminalServiceShape, MainThreadTerminalServiceShape, MainContext, IExtHostContext, IShellLaunchConfigDto, TerminalLaunchConfig, ITerminalDimensionsDto } from 'vs/workbench/api/common/extHost.protocol'; import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; import { URI } from 'vs/base/common/uri'; @@ -12,6 +12,7 @@ import { StopWatch } from 'vs/base/common/stopwatch'; import { ITerminalInstanceService, ITerminalService, ITerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminal'; import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { TerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminalDataBuffering'; @extHostNamedCustomer(MainContext.MainThreadTerminalService) export class MainThreadTerminalService implements MainThreadTerminalServiceShape { @@ -327,29 +328,23 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape * listeners are removed. */ class TerminalDataEventTracker extends Disposable { + private readonly _bufferer: TerminalDataBufferer; + constructor( private readonly _callback: (id: number, data: string) => void, - @ITerminalService private readonly _terminalService: ITerminalService + @ITerminalService private readonly _terminalService: ITerminalService, ) { super(); + + this._register(this._bufferer = new TerminalDataBufferer()); + this._terminalService.terminalInstances.forEach(instance => this._registerInstance(instance)); this._register(this._terminalService.onInstanceCreated(instance => this._registerInstance(instance))); + this._register(this._terminalService.onInstanceDisposed(instance => this._bufferer.stopBuffering(instance.id))); } private _registerInstance(instance: ITerminalInstance): void { - const bufferer = getTerminalDataBufferer(instance.id, this._callback); - const disposables = [bufferer, instance.onData(bufferer.onData)]; - - for (const d of disposables) { - this._register(d); - } - - this._register(this._terminalService.onInstanceDisposed(i => { - if (i.id === instance.id) { - for (const d of disposables) { - d.dispose(); - } - } - })); + // Buffer data events to reduce the amount of messages going to the extension host + this._register(this._bufferer.startBuffering(instance.id, instance.onData, this._callback)); } } diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index cfd8e90b8e1..16a1cafd977 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -4,15 +4,15 @@ *--------------------------------------------------------------------------------------------*/ import * as vscode from 'vscode'; -import { IDisposable } from 'vs/base/common/lifecycle'; import { Event, Emitter } from 'vs/base/common/event'; import { ExtHostTerminalServiceShape, MainContext, MainThreadTerminalServiceShape, IShellLaunchConfigDto, IShellDefinitionDto, IShellAndArgsDto, ITerminalDimensionsDto } from 'vs/workbench/api/common/extHost.protocol'; import { ExtHostConfigProvider } from 'vs/workbench/api/common/extHostConfiguration'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { URI, UriComponents } from 'vs/base/common/uri'; -import { EXT_HOST_CREATION_DELAY, ITerminalChildProcess, ITerminalDimensions, getTerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminal'; +import { EXT_HOST_CREATION_DELAY, ITerminalChildProcess, ITerminalDimensions } from 'vs/workbench/contrib/terminal/common/terminal'; import { timeout } from 'vs/base/common/async'; import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService'; +import { TerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminalDataBuffering'; export interface IExtHostTerminalService extends ExtHostTerminalServiceShape { @@ -287,9 +287,13 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ protected readonly _onDidWriteTerminalData: Emitter; public get onDidWriteTerminalData(): Event { return this._onDidWriteTerminalData && this._onDidWriteTerminalData.event; } + private readonly _bufferer: TerminalDataBufferer; + constructor( @IExtHostRpcService extHostRpc: IExtHostRpcService ) { + this._bufferer = new TerminalDataBufferer(); + this._proxy = extHostRpc.getProxy(MainContext.MainThreadTerminalService); this._onDidWriteTerminalData = new Emitter({ onFirstListenerAdd: () => this._proxy.$startSendingDataEvents(), @@ -466,10 +470,9 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ p.onProcessReady((e: { pid: number, cwd: string }) => this._proxy.$sendProcessReady(id, e.pid, e.cwd)); p.onProcessTitleChanged(title => this._proxy.$sendProcessTitle(id, title)); - const bufferer = getTerminalDataBufferer(id, this._proxy.$sendProcessData); - const disposables = [bufferer, p.onProcessData(bufferer.onData)]; - - p.onProcessExit(exitCode => this._onProcessExit(id, exitCode, disposables)); + // Buffer data events to reduce the amount of messages going to the renderer + this._bufferer.startBuffering(id, p.onProcessData, this._proxy.$sendProcessData); + p.onProcessExit(exitCode => this._onProcessExit(id, exitCode)); if (p.onProcessOverrideDimensions) { p.onProcessOverrideDimensions(e => this._proxy.$sendOverrideDimensions(id, e)); @@ -508,10 +511,8 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ return id; } - private _onProcessExit(id: number, exitCode: number, disposables: IDisposable[]): void { - for (const d of disposables) { - d.dispose(); - } + private _onProcessExit(id: number, exitCode: number): void { + this._bufferer.stopBuffering(id); // Remove process reference delete this._terminalProcesses[id]; diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index bbcab2f07b3..dc364d04f15 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -471,43 +471,3 @@ export const enum TERMINAL_COMMAND_ID { NAVIGATION_MODE_FOCUS_NEXT = 'workbench.action.terminal.navigationModeFocusNext', NAVIGATION_MODE_FOCUS_PREVIOUS = 'workbench.action.terminal.navigationModeFocusPrevious' } - -interface TerminalDataBuffer { - timeoutId: any; - data: string[]; -} - -export interface TerminalDataBufferer extends IDisposable { - onData: (e: string) => void; -} - -const terminalBufferMap = new Map(); - -export function getTerminalDataBufferer(id: number, callback: (id: number, data: string) => void): TerminalDataBufferer { - return { - dispose: () => { - const buffer = terminalBufferMap.get(id); - if (buffer) { - clearTimeout(buffer.timeoutId); - terminalBufferMap.delete(id); - } - }, - onData: (e: string) => { - let buffer = terminalBufferMap.get(id); - if (buffer) { - buffer.data.push(e); - - return; - } - - buffer = { - data: [e], - timeoutId: setTimeout(() => { - terminalBufferMap.delete(id); - callback(id, buffer!.data.join('')); - }, 5) - }; - terminalBufferMap.set(id, buffer); - } - }; -} diff --git a/src/vs/workbench/contrib/terminal/common/terminalDataBuffering.ts b/src/vs/workbench/contrib/terminal/common/terminalDataBuffering.ts new file mode 100644 index 00000000000..8b2008bf29b --- /dev/null +++ b/src/vs/workbench/contrib/terminal/common/terminalDataBuffering.ts @@ -0,0 +1,57 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Event } from 'vs/base/common/event'; +import { IDisposable } from 'vs/base/common/lifecycle'; + +interface TerminalDataBuffer extends IDisposable { + data: string[]; + timeoutId: any; +} + +export class TerminalDataBufferer implements IDisposable { + private readonly _terminalBufferMap = new Map(); + + dispose() { + for (const buffer of this._terminalBufferMap.values()) { + buffer.dispose(); + } + } + + startBuffering(id: number, event: Event, callback: (id: number, data: string) => void, throttleBy: number = 5): IDisposable { + let disposable: IDisposable; + disposable = event((e: string) => { + let buffer = this._terminalBufferMap.get(id); + if (buffer) { + buffer.data.push(e); + + return; + } + + const timeoutId = setTimeout(() => { + this._terminalBufferMap.delete(id); + callback(id, buffer!.data.join('')); + }, throttleBy); + buffer = { + data: [e], + timeoutId: timeoutId, + dispose: () => { + clearTimeout(timeoutId); + this._terminalBufferMap.delete(id); + disposable.dispose(); + } + }; + this._terminalBufferMap.set(id, buffer); + }); + return disposable; + } + + stopBuffering(id: number) { + const buffer = this._terminalBufferMap.get(id); + if (buffer) { + buffer.dispose(); + } + } +} diff --git a/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts b/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts new file mode 100644 index 00000000000..298923d2c23 --- /dev/null +++ b/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts @@ -0,0 +1,193 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { Emitter } from 'vs/base/common/event'; +import { TerminalDataBufferer } from 'vs/workbench/contrib/terminal/common/terminalDataBuffering'; + +const wait = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); + +suite('Workbench - TerminalDataBufferer', () => { + let bufferer: TerminalDataBufferer; + + setup(async () => { + bufferer = new TerminalDataBufferer(); + }); + + suite('buffering', () => { + test('start', async () => { + let terminalOnData = new Emitter(); + let counter = 0; + let data: string | undefined; + + bufferer.startBuffering(1, terminalOnData.event, (id, e) => { + counter++; + data = e; + }, 0); + + terminalOnData.fire('1'); + terminalOnData.fire('2'); + terminalOnData.fire('3'); + + await wait(5); + + terminalOnData.fire('4'); + + assert.equal(counter, 1); + assert.equal(data, '123'); + + await wait(5); + + assert.equal(counter, 2); + assert.equal(data, '4'); + }); + + test('start 2', async () => { + let terminal1OnData = new Emitter(); + let terminal1Counter = 0; + let terminal1Data: string | undefined; + + bufferer.startBuffering(1, terminal1OnData.event, (id, e) => { + terminal1Counter++; + terminal1Data = e; + }, 0); + + let terminal2OnData = new Emitter(); + let terminal2Counter = 0; + let terminal2Data: string | undefined; + + bufferer.startBuffering(2, terminal2OnData.event, (id, e) => { + terminal2Counter++; + terminal2Data = e; + }, 0); + + terminal1OnData.fire('1'); + terminal2OnData.fire('4'); + terminal1OnData.fire('2'); + terminal2OnData.fire('5'); + terminal1OnData.fire('3'); + terminal2OnData.fire('6'); + terminal2OnData.fire('7'); + + assert.equal(terminal1Counter, 0); + assert.equal(terminal1Data, undefined); + assert.equal(terminal2Counter, 0); + assert.equal(terminal2Data, undefined); + + await wait(5); + + assert.equal(terminal1Counter, 1); + assert.equal(terminal1Data, '123'); + assert.equal(terminal2Counter, 1); + assert.equal(terminal2Data, '4567'); + }); + + test('stop', async () => { + let terminalOnData = new Emitter(); + let counter = 0; + let data: string | undefined; + + bufferer.startBuffering(1, terminalOnData.event, (id, e) => { + counter++; + data = e; + }, 0); + + terminalOnData.fire('1'); + terminalOnData.fire('2'); + terminalOnData.fire('3'); + + bufferer.stopBuffering(1); + + await wait(5); + + assert.equal(counter, 0); + assert.equal(data, undefined); + }); + }); + + test('start 2 stop 1', async () => { + let terminal1OnData = new Emitter(); + let terminal1Counter = 0; + let terminal1Data: string | undefined; + + bufferer.startBuffering(1, terminal1OnData.event, (id, e) => { + terminal1Counter++; + terminal1Data = e; + }, 5); + + let terminal2OnData = new Emitter(); + let terminal2Counter = 0; + let terminal2Data: string | undefined; + + bufferer.startBuffering(2, terminal2OnData.event, (id, e) => { + terminal2Counter++; + terminal2Data = e; + }, 5); + + + terminal1OnData.fire('1'); + terminal2OnData.fire('4'); + terminal1OnData.fire('2'); + terminal2OnData.fire('5'); + terminal1OnData.fire('3'); + terminal2OnData.fire('6'); + terminal2OnData.fire('7'); + + assert.equal(terminal1Counter, 0); + assert.equal(terminal1Data, undefined); + assert.equal(terminal2Counter, 0); + assert.equal(terminal2Data, undefined); + + bufferer.stopBuffering(1); + await wait(10); + + assert.equal(terminal1Counter, 0); + assert.equal(terminal1Data, undefined); + assert.equal(terminal2Counter, 1); + assert.equal(terminal2Data, '4567'); + }); + + test('dispose', async () => { + let terminal1OnData = new Emitter(); + let terminal1Counter = 0; + let terminal1Data: string | undefined; + + bufferer.startBuffering(1, terminal1OnData.event, (id, e) => { + terminal1Counter++; + terminal1Data = e; + }, 5); + + let terminal2OnData = new Emitter(); + let terminal2Counter = 0; + let terminal2Data: string | undefined; + + bufferer.startBuffering(2, terminal2OnData.event, (id, e) => { + terminal2Counter++; + terminal2Data = e; + }, 5); + + + terminal1OnData.fire('1'); + terminal2OnData.fire('4'); + terminal1OnData.fire('2'); + terminal2OnData.fire('5'); + terminal1OnData.fire('3'); + terminal2OnData.fire('6'); + terminal2OnData.fire('7'); + + assert.equal(terminal1Counter, 0); + assert.equal(terminal1Data, undefined); + assert.equal(terminal2Counter, 0); + assert.equal(terminal2Data, undefined); + + bufferer.dispose(); + await wait(10); + + assert.equal(terminal1Counter, 0); + assert.equal(terminal1Data, undefined); + assert.equal(terminal2Counter, 0); + assert.equal(terminal2Data, undefined); + }); +}); From 000006f0d14292e063f5a77dffb95e82027e1730 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Thu, 17 Oct 2019 17:19:42 -0400 Subject: [PATCH 3/8] Fixes terminal integration test timing --- .../src/singlefolder-tests/terminal.test.ts | 64 +++++++++++-------- 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts index 1cde9257d52..cc986d26c5d 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts @@ -176,10 +176,30 @@ suite('window namespace tests', () => { const reg2 = window.onDidWriteTerminalData(e => dataEvents.push({ name: e.terminal.name, data: e.data })); const reg3 = window.onDidCloseTerminal(e => { closeEvents.push(e.name); + + if (closeEvents.length === 1) { + const term2Write = new EventEmitter(); + const term2Close = new EventEmitter(); + window.createTerminal({ + name: 'test2', pty: { + onDidWrite: term2Write.event, + onDidClose: term2Close.event, + open: () => { + term2Write.fire('write2'); + // Need to wait to ensure the data will be fired, because it will be buffered for 5ms (currently) + setTimeout(() => term2Close.fire(), 100); + }, + close: () => { } + } + }); + return; + } + if (closeEvents.length === 2) { - deepEqual(openEvents, [ 'test1', 'test2' ]); - deepEqual(dataEvents, [ { name: 'test1', data: 'write1' }, { name: 'test2', data: 'write2' } ]); - deepEqual(closeEvents, [ 'test1', 'test2' ]); + deepEqual(openEvents, ['test1', 'test2']); + deepEqual(dataEvents, [{ name: 'test1', data: 'write1' }, { name: 'test2', data: 'write2' }]); + deepEqual(closeEvents, ['test1', 'test2']); + reg1.dispose(); reg2.dispose(); reg3.dispose(); @@ -189,26 +209,18 @@ suite('window namespace tests', () => { const term1Write = new EventEmitter(); const term1Close = new EventEmitter(); - window.createTerminal({ name: 'test1', pty: { - onDidWrite: term1Write.event, - onDidClose: term1Close.event, - open: () => { - term1Write.fire('write1'); - term1Close.fire(); - const term2Write = new EventEmitter(); - const term2Close = new EventEmitter(); - window.createTerminal({ name: 'test2', pty: { - onDidWrite: term2Write.event, - onDidClose: term2Close.event, - open: () => { - term2Write.fire('write2'); - term2Close.fire(); - }, - close: () => {} - }}); - }, - close: () => {} - }}); + window.createTerminal({ + name: 'test1', pty: { + onDidWrite: term1Write.event, + onDidClose: term1Close.event, + open: () => { + term1Write.fire('write1'); + // Need to wait to ensure the data will be fired, because it will be buffered for 5ms (currently) + setTimeout(() => term1Close.fire(), 100); + }, + close: () => { } + } + }); }); }); @@ -225,8 +237,8 @@ suite('window namespace tests', () => { }); const pty: Pseudoterminal = { onDidWrite: new EventEmitter().event, - open: () => {}, - close: () => {} + open: () => { }, + close: () => { } }; window.createTerminal({ name: 'c', pty }); }); @@ -303,7 +315,7 @@ suite('window namespace tests', () => { open: () => { overrideDimensionsEmitter.fire({ columns: 10, rows: 5 }); }, - close: () => {} + close: () => { } }; const terminal = window.createTerminal({ name: 'foo', pty }); }); From 618b9aa0221c11439e64e76da702a48200036888 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Fri, 18 Oct 2019 16:55:27 -0400 Subject: [PATCH 4/8] Reduces terminal buffering test wait to 0 Removes unneeded suite --- .../api/browser/mainThreadTerminalService.ts | 2 +- .../test/common/terminalDataBuffering.test.ts | 134 +++++++++--------- 2 files changed, 67 insertions(+), 69 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index de12c271b51..8344b323c7e 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -332,7 +332,7 @@ class TerminalDataEventTracker extends Disposable { constructor( private readonly _callback: (id: number, data: string) => void, - @ITerminalService private readonly _terminalService: ITerminalService, + @ITerminalService private readonly _terminalService: ITerminalService ) { super(); diff --git a/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts b/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts index 298923d2c23..70a54735f1f 100644 --- a/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts +++ b/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts @@ -16,95 +16,93 @@ suite('Workbench - TerminalDataBufferer', () => { bufferer = new TerminalDataBufferer(); }); - suite('buffering', () => { - test('start', async () => { - let terminalOnData = new Emitter(); - let counter = 0; - let data: string | undefined; + test('start', async () => { + let terminalOnData = new Emitter(); + let counter = 0; + let data: string | undefined; - bufferer.startBuffering(1, terminalOnData.event, (id, e) => { - counter++; - data = e; - }, 0); + bufferer.startBuffering(1, terminalOnData.event, (id, e) => { + counter++; + data = e; + }, 0); - terminalOnData.fire('1'); - terminalOnData.fire('2'); - terminalOnData.fire('3'); + terminalOnData.fire('1'); + terminalOnData.fire('2'); + terminalOnData.fire('3'); - await wait(5); + await wait(0); - terminalOnData.fire('4'); + terminalOnData.fire('4'); - assert.equal(counter, 1); - assert.equal(data, '123'); + assert.equal(counter, 1); + assert.equal(data, '123'); - await wait(5); + await wait(0); - assert.equal(counter, 2); - assert.equal(data, '4'); - }); + assert.equal(counter, 2); + assert.equal(data, '4'); + }); - test('start 2', async () => { - let terminal1OnData = new Emitter(); - let terminal1Counter = 0; - let terminal1Data: string | undefined; + test('start 2', async () => { + let terminal1OnData = new Emitter(); + let terminal1Counter = 0; + let terminal1Data: string | undefined; - bufferer.startBuffering(1, terminal1OnData.event, (id, e) => { - terminal1Counter++; - terminal1Data = e; - }, 0); + bufferer.startBuffering(1, terminal1OnData.event, (id, e) => { + terminal1Counter++; + terminal1Data = e; + }, 0); - let terminal2OnData = new Emitter(); - let terminal2Counter = 0; - let terminal2Data: string | undefined; + let terminal2OnData = new Emitter(); + let terminal2Counter = 0; + let terminal2Data: string | undefined; - bufferer.startBuffering(2, terminal2OnData.event, (id, e) => { - terminal2Counter++; - terminal2Data = e; - }, 0); + bufferer.startBuffering(2, terminal2OnData.event, (id, e) => { + terminal2Counter++; + terminal2Data = e; + }, 0); - terminal1OnData.fire('1'); - terminal2OnData.fire('4'); - terminal1OnData.fire('2'); - terminal2OnData.fire('5'); - terminal1OnData.fire('3'); - terminal2OnData.fire('6'); - terminal2OnData.fire('7'); + terminal1OnData.fire('1'); + terminal2OnData.fire('4'); + terminal1OnData.fire('2'); + terminal2OnData.fire('5'); + terminal1OnData.fire('3'); + terminal2OnData.fire('6'); + terminal2OnData.fire('7'); - assert.equal(terminal1Counter, 0); - assert.equal(terminal1Data, undefined); - assert.equal(terminal2Counter, 0); - assert.equal(terminal2Data, undefined); + assert.equal(terminal1Counter, 0); + assert.equal(terminal1Data, undefined); + assert.equal(terminal2Counter, 0); + assert.equal(terminal2Data, undefined); - await wait(5); + await wait(5); - assert.equal(terminal1Counter, 1); - assert.equal(terminal1Data, '123'); - assert.equal(terminal2Counter, 1); - assert.equal(terminal2Data, '4567'); - }); + assert.equal(terminal1Counter, 1); + assert.equal(terminal1Data, '123'); + assert.equal(terminal2Counter, 1); + assert.equal(terminal2Data, '4567'); + }); - test('stop', async () => { - let terminalOnData = new Emitter(); - let counter = 0; - let data: string | undefined; + test('stop', async () => { + let terminalOnData = new Emitter(); + let counter = 0; + let data: string | undefined; - bufferer.startBuffering(1, terminalOnData.event, (id, e) => { - counter++; - data = e; - }, 0); + bufferer.startBuffering(1, terminalOnData.event, (id, e) => { + counter++; + data = e; + }, 0); - terminalOnData.fire('1'); - terminalOnData.fire('2'); - terminalOnData.fire('3'); + terminalOnData.fire('1'); + terminalOnData.fire('2'); + terminalOnData.fire('3'); - bufferer.stopBuffering(1); + bufferer.stopBuffering(1); - await wait(5); + await wait(5); - assert.equal(counter, 0); - assert.equal(data, undefined); - }); + assert.equal(counter, 0); + assert.equal(data, undefined); }); test('start 2 stop 1', async () => { From 95b1f7c3c04701bf85596536d910d5c08e602564 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Tue, 22 Oct 2019 15:42:00 -0400 Subject: [PATCH 5/8] Reworks the test to not be timing specific --- .../src/singlefolder-tests/terminal.test.ts | 57 ++++++++++++------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts index cc986d26c5d..a588c1d750f 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts @@ -173,29 +173,24 @@ suite('window namespace tests', () => { const dataEvents: { name: string, data: string }[] = []; const closeEvents: string[] = []; const reg1 = window.onDidOpenTerminal(e => openEvents.push(e.name)); - const reg2 = window.onDidWriteTerminalData(e => dataEvents.push({ name: e.terminal.name, data: e.data })); + + let resolveOnceDataWritten: (() => void) | undefined; + + const reg2 = window.onDidWriteTerminalData(e => { + dataEvents.push({ name: e.terminal.name, data: e.data }); + + resolveOnceDataWritten!(); + }); + const reg3 = window.onDidCloseTerminal(e => { closeEvents.push(e.name); if (closeEvents.length === 1) { - const term2Write = new EventEmitter(); - const term2Close = new EventEmitter(); - window.createTerminal({ - name: 'test2', pty: { - onDidWrite: term2Write.event, - onDidClose: term2Close.event, - open: () => { - term2Write.fire('write2'); - // Need to wait to ensure the data will be fired, because it will be buffered for 5ms (currently) - setTimeout(() => term2Close.fire(), 100); - }, - close: () => { } - } - }); - return; + deepEqual(openEvents, ['test1']); + deepEqual(dataEvents, [{ name: 'test1', data: 'write1' }]); + deepEqual(closeEvents, ['test1']); } - - if (closeEvents.length === 2) { + else if (closeEvents.length === 2) { deepEqual(openEvents, ['test1', 'test2']); deepEqual(dataEvents, [{ name: 'test1', data: 'write1' }, { name: 'test2', data: 'write2' }]); deepEqual(closeEvents, ['test1', 'test2']); @@ -213,10 +208,30 @@ suite('window namespace tests', () => { name: 'test1', pty: { onDidWrite: term1Write.event, onDidClose: term1Close.event, - open: () => { + open: async () => { term1Write.fire('write1'); - // Need to wait to ensure the data will be fired, because it will be buffered for 5ms (currently) - setTimeout(() => term1Close.fire(), 100); + + // Wait until the data is written + await new Promise(resolve => { resolveOnceDataWritten = resolve; }); + term1Close.fire(); + + const term2Write = new EventEmitter(); + const term2Close = new EventEmitter(); + window.createTerminal({ + name: 'test2', pty: { + onDidWrite: term2Write.event, + onDidClose: term2Close.event, + open: async () => { + term2Write.fire('write2'); + + // Wait until the data is written + await new Promise(resolve => { resolveOnceDataWritten = resolve; }); + + term2Close.fire(); + }, + close: () => { } + } + }); }, close: () => { } } From a00301d0743d86987f4186c7e4422409cff93145 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 23 Oct 2019 16:54:04 -0700 Subject: [PATCH 6/8] Format/minimize diff --- .../src/singlefolder-tests/terminal.test.ts | 57 +++++++++---------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts index a588c1d750f..b8fbe9dfb9f 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts @@ -189,8 +189,7 @@ suite('window namespace tests', () => { deepEqual(openEvents, ['test1']); deepEqual(dataEvents, [{ name: 'test1', data: 'write1' }]); deepEqual(closeEvents, ['test1']); - } - else if (closeEvents.length === 2) { + } else if (closeEvents.length === 2) { deepEqual(openEvents, ['test1', 'test2']); deepEqual(dataEvents, [{ name: 'test1', data: 'write1' }, { name: 'test2', data: 'write2' }]); deepEqual(closeEvents, ['test1', 'test2']); @@ -204,38 +203,36 @@ suite('window namespace tests', () => { const term1Write = new EventEmitter(); const term1Close = new EventEmitter(); - window.createTerminal({ - name: 'test1', pty: { - onDidWrite: term1Write.event, - onDidClose: term1Close.event, - open: async () => { - term1Write.fire('write1'); + window.createTerminal({name: 'test1', pty: { + onDidWrite: term1Write.event, + onDidClose: term1Close.event, + open: async () => { + term1Write.fire('write1'); - // Wait until the data is written - await new Promise(resolve => { resolveOnceDataWritten = resolve; }); - term1Close.fire(); + // Wait until the data is written + await new Promise(resolve => { resolveOnceDataWritten = resolve; }); + term1Close.fire(); - const term2Write = new EventEmitter(); - const term2Close = new EventEmitter(); - window.createTerminal({ - name: 'test2', pty: { - onDidWrite: term2Write.event, - onDidClose: term2Close.event, - open: async () => { - term2Write.fire('write2'); + const term2Write = new EventEmitter(); + const term2Close = new EventEmitter(); + window.createTerminal({ + name: 'test2', pty: { + onDidWrite: term2Write.event, + onDidClose: term2Close.event, + open: async () => { + term2Write.fire('write2'); - // Wait until the data is written - await new Promise(resolve => { resolveOnceDataWritten = resolve; }); + // Wait until the data is written + await new Promise(resolve => { resolveOnceDataWritten = resolve; }); - term2Close.fire(); - }, - close: () => { } - } - }); - }, - close: () => { } - } - }); + term2Close.fire(); + }, + close: () => { } + } + }); + }, + close: () => { } + }}); }); }); From 4567067b084d2a012a713be297a6c9a69fc76437 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 23 Oct 2019 16:56:30 -0700 Subject: [PATCH 7/8] Remove > 0 timeouts in bufferer tests --- .../test/common/terminalDataBuffering.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts b/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts index 70a54735f1f..54a2e4be0ab 100644 --- a/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts +++ b/src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts @@ -75,7 +75,7 @@ suite('Workbench - TerminalDataBufferer', () => { assert.equal(terminal2Counter, 0); assert.equal(terminal2Data, undefined); - await wait(5); + await wait(0); assert.equal(terminal1Counter, 1); assert.equal(terminal1Data, '123'); @@ -99,7 +99,7 @@ suite('Workbench - TerminalDataBufferer', () => { bufferer.stopBuffering(1); - await wait(5); + await wait(0); assert.equal(counter, 0); assert.equal(data, undefined); @@ -113,7 +113,7 @@ suite('Workbench - TerminalDataBufferer', () => { bufferer.startBuffering(1, terminal1OnData.event, (id, e) => { terminal1Counter++; terminal1Data = e; - }, 5); + }, 0); let terminal2OnData = new Emitter(); let terminal2Counter = 0; @@ -122,7 +122,7 @@ suite('Workbench - TerminalDataBufferer', () => { bufferer.startBuffering(2, terminal2OnData.event, (id, e) => { terminal2Counter++; terminal2Data = e; - }, 5); + }, 0); terminal1OnData.fire('1'); @@ -139,7 +139,7 @@ suite('Workbench - TerminalDataBufferer', () => { assert.equal(terminal2Data, undefined); bufferer.stopBuffering(1); - await wait(10); + await wait(0); assert.equal(terminal1Counter, 0); assert.equal(terminal1Data, undefined); @@ -155,7 +155,7 @@ suite('Workbench - TerminalDataBufferer', () => { bufferer.startBuffering(1, terminal1OnData.event, (id, e) => { terminal1Counter++; terminal1Data = e; - }, 5); + }, 0); let terminal2OnData = new Emitter(); let terminal2Counter = 0; @@ -164,7 +164,7 @@ suite('Workbench - TerminalDataBufferer', () => { bufferer.startBuffering(2, terminal2OnData.event, (id, e) => { terminal2Counter++; terminal2Data = e; - }, 5); + }, 0); terminal1OnData.fire('1'); @@ -181,7 +181,7 @@ suite('Workbench - TerminalDataBufferer', () => { assert.equal(terminal2Data, undefined); bufferer.dispose(); - await wait(10); + await wait(0); assert.equal(terminal1Counter, 0); assert.equal(terminal1Data, undefined); From ebc27c441002e917cd69a074d58b641cd2698ba7 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 23 Oct 2019 16:57:22 -0700 Subject: [PATCH 8/8] More formatting --- .../src/singlefolder-tests/terminal.test.ts | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts index b8fbe9dfb9f..7138319d459 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts @@ -203,7 +203,7 @@ suite('window namespace tests', () => { const term1Write = new EventEmitter(); const term1Close = new EventEmitter(); - window.createTerminal({name: 'test1', pty: { + window.createTerminal({ name: 'test1', pty: { onDidWrite: term1Write.event, onDidClose: term1Close.event, open: async () => { @@ -215,21 +215,19 @@ suite('window namespace tests', () => { const term2Write = new EventEmitter(); const term2Close = new EventEmitter(); - window.createTerminal({ - name: 'test2', pty: { - onDidWrite: term2Write.event, - onDidClose: term2Close.event, - open: async () => { - term2Write.fire('write2'); + window.createTerminal({ name: 'test2', pty: { + onDidWrite: term2Write.event, + onDidClose: term2Close.event, + open: async () => { + term2Write.fire('write2'); - // Wait until the data is written - await new Promise(resolve => { resolveOnceDataWritten = resolve; }); + // Wait until the data is written + await new Promise(resolve => { resolveOnceDataWritten = resolve; }); - term2Close.fire(); - }, - close: () => { } - } - }); + term2Close.fire(); + }, + close: () => { } + }}); }, close: () => { } }});