diff --git a/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsViewContent.ts b/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsViewContent.ts index b60894a750254..69a37fc0a7357 100644 --- a/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsViewContent.ts +++ b/src/vs/workbench/contrib/testing/browser/testResultsView/testResultsViewContent.ts @@ -250,7 +250,7 @@ export class TestResultsViewContent extends Disposable { public fillBody(containerElement: HTMLElement): void { const initialSpitWidth = TestResultsViewContent.lastSplitWidth; - this.splitView = new SplitView(containerElement, { orientation: Orientation.HORIZONTAL }); + this.splitView = this._register(new SplitView(containerElement, { orientation: Orientation.HORIZONTAL })); const { historyVisible, showRevealLocationOnMessages } = this.options; const isInPeekView = this.editor !== undefined; diff --git a/src/vs/workbench/contrib/testing/common/testResultService.ts b/src/vs/workbench/contrib/testing/common/testResultService.ts index 87c3210aeff0f..6e0c10447cb02 100644 --- a/src/vs/workbench/contrib/testing/common/testResultService.ts +++ b/src/vs/workbench/contrib/testing/common/testResultService.ts @@ -7,7 +7,7 @@ import { findFirstIdxMonotonousOrArrLen } from '../../../../base/common/arraysFi import { RunOnceScheduler } from '../../../../base/common/async.js'; import { Emitter, Event } from '../../../../base/common/event.js'; import { createSingleCallFunction } from '../../../../base/common/functional.js'; -import { Disposable, DisposableStore, dispose, toDisposable } from '../../../../base/common/lifecycle.js'; +import { Disposable, DisposableMap, DisposableStore } from '../../../../base/common/lifecycle.js'; import { generateUuid } from '../../../../base/common/uuid.js'; import { IContextKey, IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; import { createDecorator } from '../../../../platform/instantiation/common/instantiation.js'; @@ -76,7 +76,7 @@ export class TestResultService extends Disposable implements ITestResultService declare _serviceBrand: undefined; private changeResultEmitter = this._register(new Emitter()); private _results: ITestResult[] = []; - private readonly _resultsDisposables: DisposableStore[] = []; + private readonly _resultsDisposables = this._register(new DisposableMap()); private testChangeEmitter = this._register(new Emitter()); private insertOrderCounter = 0; @@ -115,7 +115,6 @@ export class TestResultService extends Disposable implements ITestResultService @ITelemetryService private readonly telemetryService: ITelemetryService, ) { super(); - this._register(toDisposable(() => dispose(this._resultsDisposables))); this.isRunning = TestingContextKeys.isRunning.bindTo(contextKeyService); this.hasAnyResults = TestingContextKeys.hasAnyResults.bindTo(contextKeyService); } @@ -181,18 +180,25 @@ export class TestResultService extends Disposable implements ITestResultService } this.hasAnyResults.set(true); + let removed: ITestResult | undefined; if (this.results.length > RETAIN_MAX_RESULTS) { - this.results.pop(); - this._resultsDisposables.pop()?.dispose(); + removed = this.results.pop(); } const ds = new DisposableStore(); - this._resultsDisposables.push(ds); + this._resultsDisposables.set(result, ds); if (result instanceof LiveTestResult) { ds.add(result); ds.add(result.onComplete(() => this.onComplete(result))); ds.add(result.onChange(this.testChangeEmitter.fire, this.testChangeEmitter)); + } + + if (removed) { + this._resultsDisposables.deleteAndDispose(removed); + } + + if (result instanceof LiveTestResult) { this.isRunning.set(true); this.changeResultEmitter.fire({ started: result }); } else { @@ -237,6 +243,9 @@ export class TestResultService extends Disposable implements ITestResultService } this._results = keep; + for (const result of removed) { + this._resultsDisposables.deleteAndDispose(result); + } this.persistScheduler.schedule(); if (keep.length === 0) { this.hasAnyResults.set(false); diff --git a/src/vs/workbench/contrib/testing/test/common/testResultService.test.ts b/src/vs/workbench/contrib/testing/test/common/testResultService.test.ts index 272d02a8a836f..4faad9ae2ceac 100644 --- a/src/vs/workbench/contrib/testing/test/common/testResultService.test.ts +++ b/src/vs/workbench/contrib/testing/test/common/testResultService.test.ts @@ -44,6 +44,8 @@ suite('Workbench - Test Results Service', () => { let insertCounter = 0; class TestLiveTestResult extends LiveTestResult { + public disposed = false; + constructor( id: string, persist: boolean, @@ -56,6 +58,11 @@ suite('Workbench - Test Results Service', () => { public setAllToStatePublic(state: TestResultState, taskId: string, when: (task: ITestTaskState, item: TestResultItem) => boolean) { this.setAllToState(state, taskId, when); } + + public override dispose(): void { + this.disposed = true; + super.dispose(); + } } const ds = ensureNoDisposablesAreLeakedInTestSuite(); @@ -272,6 +279,7 @@ suite('Workbench - Test Results Service', () => { results.clear(); assert.deepStrictEqual(results.results, [r2]); + assert.strictEqual(r.disposed, true); }); test('keeps ongoing tests on top, restored order when done', async () => { @@ -331,6 +339,19 @@ suite('Workbench - Test Results Service', () => { results.push(hydrated2); assert.deepStrictEqual(results.results, [r, hydrated1, hydrated2]); }); + + test('disposes a completed result that is immediately evicted', async () => { + const newerCompletedAt = Date.now() + 1000; + for (let i = 0; i < 128; i++) { + results.push(await makeHydrated(newerCompletedAt + i)); + } + + const older = new TestLiveTestResult('older', false, defaultOpts([])); + older.markComplete(); + results.push(older); + + assert.deepStrictEqual({ retained: results.results.includes(older), disposed: older.disposed }, { retained: false, disposed: true }); + }); }); test('resultItemParents', function () {