Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
27 changes: 17 additions & 10 deletions src/vs/workbench/contrib/testing/common/testResultService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export class TestResultService extends Disposable implements ITestResultService
declare _serviceBrand: undefined;
private changeResultEmitter = this._register(new Emitter<ResultChangeEvent>());
private _results: ITestResult[] = [];
private readonly _resultsDisposables: DisposableStore[] = [];
private readonly _resultsDisposables = new Map<ITestResult, DisposableStore>();
Comment thread
SimonSiefke marked this conversation as resolved.
Outdated
private testChangeEmitter = this._register(new Emitter<TestResultItemChange>());
private insertOrderCounter = 0;

Expand Down Expand Up @@ -115,7 +115,7 @@ export class TestResultService extends Disposable implements ITestResultService
@ITelemetryService private readonly telemetryService: ITelemetryService,
) {
super();
this._register(toDisposable(() => dispose(this._resultsDisposables)));
this._register(toDisposable(() => dispose(this._resultsDisposables.values())));
this.isRunning = TestingContextKeys.isRunning.bindTo(contextKeyService);
this.hasAnyResults = TestingContextKeys.hasAnyResults.bindTo(contextKeyService);
}
Expand Down Expand Up @@ -172,6 +172,14 @@ export class TestResultService extends Disposable implements ITestResultService
* @inheritdoc
*/
public push<T extends ITestResult>(result: T): T {
if (result instanceof LiveTestResult) {
const ds = new DisposableStore();
this._resultsDisposables.set(result, ds);
ds.add(result);
ds.add(result.onComplete(() => this.onComplete(result)));
ds.add(result.onChange(this.testChangeEmitter.fire, this.testChangeEmitter));
}

if (result.completedAt === undefined) {
this.results.unshift(result);
} else {
Expand All @@ -182,17 +190,12 @@ export class TestResultService extends Disposable implements ITestResultService

this.hasAnyResults.set(true);
if (this.results.length > RETAIN_MAX_RESULTS) {
this.results.pop();
this._resultsDisposables.pop()?.dispose();
const removed = this.results.pop()!;
this._resultsDisposables.get(removed)?.dispose();
this._resultsDisposables.delete(removed);
Comment thread
SimonSiefke marked this conversation as resolved.
Outdated
}

const ds = new DisposableStore();
this._resultsDisposables.push(ds);

if (result instanceof LiveTestResult) {
ds.add(result);
ds.add(result.onComplete(() => this.onComplete(result)));
ds.add(result.onChange(this.testChangeEmitter.fire, this.testChangeEmitter));
this.isRunning.set(true);
this.changeResultEmitter.fire({ started: result });
} else {
Expand Down Expand Up @@ -237,6 +240,10 @@ export class TestResultService extends Disposable implements ITestResultService
}

this._results = keep;
for (const result of removed) {
this._resultsDisposables.get(result)?.dispose();
this._resultsDisposables.delete(result);
}
this.persistScheduler.schedule();
if (keep.length === 0) {
this.hasAnyResults.set(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ suite('Workbench - Test Results Service', () => {
let insertCounter = 0;

class TestLiveTestResult extends LiveTestResult {
public disposed = false;
Comment thread
SimonSiefke marked this conversation as resolved.

constructor(
id: string,
persist: boolean,
Expand All @@ -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();
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () {
Expand Down