Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

easy no leak test cases #201017

Merged
merged 4 commits into from
Dec 15, 2023
Merged

easy no leak test cases #201017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
708222d
easy no leak cases
amunger Dec 15, 2023
2cad827
remove unused function
amunger Dec 15, 2023
c493210
reduce test disposable leaks
amunger Dec 15, 2023
d35738a
reduce test disposable leaks, remove check from still leaking test
amunger Dec 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 0 additions & 7 deletions .eslintrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,15 +254,8 @@
"src/vs/workbench/contrib/editSessions/test/browser/editSessions.test.ts",
"src/vs/workbench/contrib/extensions/test/common/extensionQuery.test.ts",
"src/vs/workbench/contrib/extensions/test/electron-sandbox/extension.test.ts",
"src/vs/workbench/contrib/notebook/test/browser/cellDnd.test.ts",
"src/vs/workbench/contrib/notebook/test/browser/contrib/contributedStatusBarItemController.test.ts",
"src/vs/workbench/contrib/notebook/test/browser/contrib/executionStatusBarItem.test.ts",
"src/vs/workbench/contrib/notebook/test/browser/contrib/layoutActions.test.ts",
"src/vs/workbench/contrib/notebook/test/browser/contrib/outputCopyTests.test.ts",
"src/vs/workbench/contrib/notebook/test/browser/notebookBrowser.test.ts",
"src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts",
"src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts",
"src/vs/workbench/contrib/notebook/test/browser/notebookTextModel.test.ts",
"src/vs/workbench/contrib/snippets/test/browser/snippetFile.test.ts",
"src/vs/workbench/contrib/snippets/test/browser/snippetsRegistry.test.ts",
"src/vs/workbench/contrib/snippets/test/browser/snippetsRewrite.test.ts",
Expand Down
3 changes: 3 additions & 0 deletions src/vs/workbench/contrib/notebook/test/browser/cellDnd.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { withTestNotebook } from 'vs/workbench/contrib/notebook/test/browser/testNotebookEditor';
import * as assert from 'assert';
import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';

interface IBeginningState {
startOrder: string[];
Expand Down Expand Up @@ -46,6 +47,8 @@ async function testCellDnd(beginning: IBeginningState, dragAction: IDragAction,
}

suite('cellDND', () => {
ensureNoDisposablesAreLeakedInTestSuite();

test('drag 1 cell', async () => {
await testCellDnd(
{
Expand Down
3 changes: 3 additions & 0 deletions ...orkbench/contrib/notebook/test/browser/contrib/contributedStatusBarItemController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { CancellationToken } from 'vs/base/common/cancellation';
import { Emitter, Event } from 'vs/base/common/event';
import { Disposable, DisposableStore } from 'vs/base/common/lifecycle';
import { URI } from 'vs/base/common/uri';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
import { ContributedStatusBarItemController } from 'vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/contributedStatusBarItemController';
import { INotebookCellStatusBarService } from 'vs/workbench/contrib/notebook/common/notebookCellStatusBarService';
import { CellKind, INotebookCellStatusBarItemProvider } from 'vs/workbench/contrib/notebook/common/notebookCommon';
Expand All @@ -20,6 +21,8 @@ suite('Notebook Statusbar', () => {
testDisposables.clear();
});

ensureNoDisposablesAreLeakedInTestSuite();

test('Calls item provider', async function () {
await withTestNotebook(
[
Expand Down
3 changes: 3 additions & 0 deletions src/vs/workbench/contrib/notebook/test/browser/contrib/executionStatusBarItem.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@
*--------------------------------------------------------------------------------------------*/

import * as assert from 'assert';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
import { formatCellDuration } from 'vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController';

suite('notebookBrowser', () => {
ensureNoDisposablesAreLeakedInTestSuite();

test('formatCellDuration', function () {
assert.strictEqual(formatCellDuration(0, false), '0.0s');
assert.strictEqual(formatCellDuration(0), '0ms');
Expand Down
2 changes: 2 additions & 0 deletions src/vs/workbench/contrib/notebook/test/browser/contrib/outputCopyTests.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ import * as assert from 'assert';
import { VSBuffer } from 'vs/base/common/buffer';
import { IOutputItemDto } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { copyCellOutput } from 'vs/workbench/contrib/notebook/browser/contrib/clipboard/cellOutputClipboard';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';

suite('Cell Output Clipboard Tests', () => {
ensureNoDisposablesAreLeakedInTestSuite();

class ClipboardService {
private _clipboardContent = '';
Expand Down
10 changes: 5 additions & 5 deletions src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ suite('NotebookExecutionService', () => {
let kernelService: INotebookKernelService;
let disposables: DisposableStore;

teardown(() => {
Copy link
Contributor

@rzhao271 rzhao271 Dec 15, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of disposables as a separate variable, ensureNoDisposablesAreLeakedInTestSuite returns a disposable store to use. Using that function would also render this teardown section unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ensureNoDisposablesAreLeakedInTestSuite isn't being called in this test yet. There's still a leak that @rebornix says he'll look into

disposables.dispose();
});

setup(function () {

disposables = new DisposableStore();
Expand Down Expand Up @@ -71,15 +75,11 @@ suite('NotebookExecutionService', () => {
}
});

kernelService = instantiationService.createInstance(NotebookKernelService);
kernelService = disposables.add(instantiationService.createInstance(NotebookKernelService));
instantiationService.set(INotebookKernelService, kernelService);
contextKeyService = instantiationService.get(IContextKeyService);
});

teardown(() => {
disposables.dispose();
});

async function withTestNotebook(cells: [string, string, CellKind, IOutputDto[], NotebookCellMetadata][], callback: (viewModel: NotebookViewModel, textModel: NotebookTextModel) => void | Promise<void>) {
return _withTestNotebook(cells, (editor, viewModel) => callback(viewModel, viewModel.notebookDocument));
}
Expand Down
14 changes: 7 additions & 7 deletions src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ suite('NotebookExecutionStateService', () => {
let disposables: DisposableStore;
let testNotebookModel: NotebookTextModel | undefined;

teardown(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of disposables as a separate variable, ensureNoDisposablesAreLeakedInTestSuite returns a disposable store to use. Using that function would also render this teardown section unnecessary.

disposables.dispose();
});

setup(function () {

disposables = new DisposableStore();
Expand All @@ -59,14 +63,10 @@ suite('NotebookExecutionStateService', () => {
}
});

kernelService = instantiationService.createInstance(NotebookKernelService);
kernelService = disposables.add(instantiationService.createInstance(NotebookKernelService));
instantiationService.set(INotebookKernelService, kernelService);
instantiationService.set(INotebookExecutionService, instantiationService.createInstance(NotebookExecutionService));
instantiationService.set(INotebookExecutionStateService, instantiationService.createInstance(NotebookExecutionStateService));
});

teardown(() => {
disposables.dispose();
instantiationService.set(INotebookExecutionService, disposables.add(instantiationService.createInstance(NotebookExecutionService)));
instantiationService.set(INotebookExecutionStateService, disposables.add(instantiationService.createInstance(NotebookExecutionStateService)));
});

async function withTestNotebook(cells: [string, string, CellKind, IOutputDto[], NotebookCellMetadata][], callback: (viewModel: NotebookViewModel, textModel: NotebookTextModel) => void | Promise<void>) {
Expand Down