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

Simplfy notebook diff tests disposable tracking #192909

Merged
merged 1 commit into from
Sep 13, 2023
Merged

Simplfy notebook diff tests disposable tracking #192909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 25 additions & 37 deletions src/vs/workbench/contrib/notebook/test/browser/notebookDiff.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import * as assert from 'assert';
import { VSBuffer } from 'vs/base/common/buffer';
import { ISequence, LcsDiff } from 'vs/base/common/diff/diff';
import { DisposableStore } from 'vs/base/common/lifecycle';
import { Mimes } from 'vs/base/common/mime';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService';
Expand All @@ -30,27 +29,16 @@ class CellSequence implements ISequence {
}
}


suite('NotebookCommon', () => {
let disposables: DisposableStore;
const configurationService = new TestConfigurationService();

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

ensureNoDisposablesAreLeakedInTestSuite();

setup(() => {
disposables = new DisposableStore();
});

test('diff different source', async () => {
await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
['x', 'javascript', CellKind.Code, [{ outputId: 'someOtherId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([3])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 3 }],
], [
['y', 'javascript', CellKind.Code, [{ outputId: 'someOtherId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([3])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 3 }],
], (model, accessor) => {
], (model, disposables, accessor) => {
const diff = new LcsDiff(new CellSequence(model.original.notebook), new CellSequence(model.modified.notebook));
const diffResult = diff.ComputeDiff(false);
assert.strictEqual(diffResult.changes.length, 1);
Expand Down Expand Up @@ -83,13 +71,13 @@ suite('NotebookCommon', () => {
});

test('diff different output', async () => {
await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
['x', 'javascript', CellKind.Code, [{ outputId: 'someId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([5])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 5 }],
['', 'javascript', CellKind.Code, [], {}]
], [
['x', 'javascript', CellKind.Code, [{ outputId: 'someOtherId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([3])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 3 }],
['', 'javascript', CellKind.Code, [], {}]
], (model, accessor) => {
], (model, disposables, accessor) => {
const diff = new LcsDiff(new CellSequence(model.original.notebook), new CellSequence(model.modified.notebook));
const diffResult = diff.ComputeDiff(false);
assert.strictEqual(diffResult.changes.length, 1);
Expand Down Expand Up @@ -124,11 +112,11 @@ suite('NotebookCommon', () => {
});

test('diff test small source', async () => {
await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
['123456789', 'javascript', CellKind.Code, [], {}]
], [
['987654321', 'javascript', CellKind.Code, [], {}],
], (model, accessor) => {
], (model, disposables, accessor) => {
const diff = new LcsDiff(new CellSequence(model.original.notebook), new CellSequence(model.modified.notebook));
const diffResult = diff.ComputeDiff(false);
assert.strictEqual(diffResult.changes.length, 1);
Expand Down Expand Up @@ -162,7 +150,7 @@ suite('NotebookCommon', () => {
});

test('diff test data single cell', async () => {
await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
[[
'# This version has a bug\n',
'def mult(a, b):\n',
Expand All @@ -174,7 +162,7 @@ suite('NotebookCommon', () => {
' \'This version is debugged.\'\n',
' return a * b'
].join(''), 'javascript', CellKind.Code, [], {}],
], (model, accessor) => {
], (model, disposables, accessor) => {
const diff = new LcsDiff(new CellSequence(model.original.notebook), new CellSequence(model.modified.notebook));
const diffResult = diff.ComputeDiff(false);
assert.strictEqual(diffResult.changes.length, 1);
Expand Down Expand Up @@ -208,15 +196,15 @@ suite('NotebookCommon', () => {
});

test('diff foo/foe', async () => {
await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
[['def foe(x, y):\n', ' return x + y\n', 'foe(3, 2)'].join(''), 'javascript', CellKind.Code, [{ outputId: 'someId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([6])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 5 }],
[['def foo(x, y):\n', ' return x * y\n', 'foo(1, 2)'].join(''), 'javascript', CellKind.Code, [{ outputId: 'someId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([2])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 6 }],
['', 'javascript', CellKind.Code, [], {}]
], [
[['def foo(x, y):\n', ' return x * y\n', 'foo(1, 2)'].join(''), 'javascript', CellKind.Code, [{ outputId: 'someId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([6])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 5 }],
[['def foe(x, y):\n', ' return x + y\n', 'foe(3, 2)'].join(''), 'javascript', CellKind.Code, [{ outputId: 'someId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([2])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 6 }],
['', 'javascript', CellKind.Code, [], {}]
], (model, accessor) => {
], (model, disposables, accessor) => {
const diff = new LcsDiff(new CellSequence(model.original.notebook), new CellSequence(model.modified.notebook));
const diffResult = diff.ComputeDiff(false);
const eventDispatcher = disposables.add(new NotebookDiffEditorEventDispatcher());
Expand All @@ -239,15 +227,15 @@ suite('NotebookCommon', () => {
});

test('diff markdown', async () => {
await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
['This is a test notebook with only markdown cells', 'markdown', CellKind.Markup, [], {}],
['Lorem ipsum dolor sit amet', 'markdown', CellKind.Markup, [], {}],
['In other news', 'markdown', CellKind.Markup, [], {}],
], [
['This is a test notebook with markdown cells only', 'markdown', CellKind.Markup, [], {}],
['Lorem ipsum dolor sit amet', 'markdown', CellKind.Markup, [], {}],
['In the news', 'markdown', CellKind.Markup, [], {}],
], (model, accessor) => {
], (model, disposables, accessor) => {
const diff = new LcsDiff(new CellSequence(model.original.notebook), new CellSequence(model.modified.notebook));
const diffResult = diff.ComputeDiff(false);
const eventDispatcher = disposables.add(new NotebookDiffEditorEventDispatcher());
Expand All @@ -270,14 +258,14 @@ suite('NotebookCommon', () => {
});

test('diff insert', async () => {
await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
['var a = 1;', 'javascript', CellKind.Code, [], {}],
['var b = 2;', 'javascript', CellKind.Code, [], {}]
], [
['var h = 8;', 'javascript', CellKind.Code, [], {}],
['var a = 1;', 'javascript', CellKind.Code, [], {}],
['var b = 2;', 'javascript', CellKind.Code, [], {}]
], (model, accessor) => {
], (model, disposables, accessor) => {
const eventDispatcher = disposables.add(new NotebookDiffEditorEventDispatcher());
const diffResult = NotebookTextDiffEditor.computeDiff(accessor, configurationService, model, eventDispatcher, {
cellsDiff: {
Expand Down Expand Up @@ -308,7 +296,7 @@ suite('NotebookCommon', () => {

test('diff insert 2', async () => {

await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
['var a = 1;', 'javascript', CellKind.Code, [], {}],
['var b = 2;', 'javascript', CellKind.Code, [], {}],
['var c = 3;', 'javascript', CellKind.Code, [], {}],
Expand All @@ -325,7 +313,7 @@ suite('NotebookCommon', () => {
['var e = 5;', 'javascript', CellKind.Code, [], {}],
['var f = 6;', 'javascript', CellKind.Code, [], {}],
['var g = 7;', 'javascript', CellKind.Code, [], {}],
], async (model, accessor) => {
], async (model, disposables, accessor) => {
const eventDispatcher = disposables.add(new NotebookDiffEditorEventDispatcher());
const diffResult = NotebookTextDiffEditor.computeDiff(accessor, configurationService, model, eventDispatcher, {
cellsDiff: {
Expand Down Expand Up @@ -366,7 +354,7 @@ suite('NotebookCommon', () => {

test('diff insert 3', async () => {

await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
['var a = 1;', 'javascript', CellKind.Code, [], {}],
['var b = 2;', 'javascript', CellKind.Code, [], {}],
['var c = 3;', 'javascript', CellKind.Code, [], {}],
Expand All @@ -383,7 +371,7 @@ suite('NotebookCommon', () => {
['var e = 5;', 'javascript', CellKind.Code, [], {}],
['var f = 6;', 'javascript', CellKind.Code, [], {}],
['var g = 7;', 'javascript', CellKind.Code, [], {}],
], async (model, accessor) => {
], async (model, disposables, accessor) => {
const eventDispatcher = disposables.add(new NotebookDiffEditorEventDispatcher());
const diffResult = NotebookTextDiffEditor.computeDiff(accessor, configurationService, model, eventDispatcher, {
cellsDiff: {
Expand Down Expand Up @@ -418,7 +406,7 @@ suite('NotebookCommon', () => {
});

test('LCS', async () => {
await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
['# Description', 'markdown', CellKind.Markup, [], { custom: { metadata: {} } }],
['x = 3', 'javascript', CellKind.Code, [], { custom: { metadata: { collapsed: true } }, executionOrder: 1 }],
['x', 'javascript', CellKind.Code, [{ outputId: 'someId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([3])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 1 }],
Expand Down Expand Up @@ -451,7 +439,7 @@ suite('NotebookCommon', () => {
});

test('LCS 2', async () => {
await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
['# Description', 'markdown', CellKind.Markup, [], { custom: { metadata: {} } }],
['x = 3', 'javascript', CellKind.Code, [], { custom: { metadata: { collapsed: true } }, executionOrder: 1 }],
['x', 'javascript', CellKind.Code, [{ outputId: 'someId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([3])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 1 }],
Expand Down Expand Up @@ -502,7 +490,7 @@ suite('NotebookCommon', () => {
});

test('LCS 3', async () => {
await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
['var a = 1;', 'javascript', CellKind.Code, [], {}],
['var b = 2;', 'javascript', CellKind.Code, [], {}],
['var c = 3;', 'javascript', CellKind.Code, [], {}],
Expand Down Expand Up @@ -539,13 +527,13 @@ suite('NotebookCommon', () => {
});

test('diff output', async () => {
await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
['x', 'javascript', CellKind.Code, [{ outputId: 'someOtherId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([3])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 3 }],
['y', 'javascript', CellKind.Code, [{ outputId: 'someOtherId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([4])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 3 }],
], [
['x', 'javascript', CellKind.Code, [{ outputId: 'someOtherId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([3])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 3 }],
['y', 'javascript', CellKind.Code, [{ outputId: 'someOtherId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([5])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 3 }],
], (model, accessor) => {
], (model, disposables, accessor) => {
const diff = new LcsDiff(new CellSequence(model.original.notebook), new CellSequence(model.modified.notebook));
const diffResult = diff.ComputeDiff(false);
const eventDispatcher = disposables.add(new NotebookDiffEditorEventDispatcher());
Expand All @@ -568,13 +556,13 @@ suite('NotebookCommon', () => {
});

test('diff output fast check', async () => {
await withTestNotebookDiffModel(disposables, [
await withTestNotebookDiffModel([
['x', 'javascript', CellKind.Code, [{ outputId: 'someOtherId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([3])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 3 }],
['y', 'javascript', CellKind.Code, [{ outputId: 'someOtherId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([4])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 3 }],
], [
['x', 'javascript', CellKind.Code, [{ outputId: 'someOtherId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([3])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 3 }],
['y', 'javascript', CellKind.Code, [{ outputId: 'someOtherId', outputs: [{ mime: Mimes.text, data: VSBuffer.wrap(new Uint8Array([5])) }] }], { custom: { metadata: { collapsed: false } }, executionOrder: 3 }],
], (model, accessor) => {
], (model, disposables, accessor) => {
const diff = new LcsDiff(new CellSequence(model.original.notebook), new CellSequence(model.modified.notebook));
const diffResult = diff.ComputeDiff(false);
const eventDispatcher = disposables.add(new NotebookDiffEditorEventDispatcher());
Expand Down
5 changes: 3 additions & 2 deletions src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,8 @@ export function createTestNotebookEditor(instantiationService: TestInstantiation
return _createTestNotebookEditor(instantiationService, disposables, cells);
}

export async function withTestNotebookDiffModel<R = any>(disposables: DisposableStore, originalCells: [source: string, lang: string, kind: CellKind, output?: IOutputDto[], metadata?: NotebookCellMetadata][], modifiedCells: [source: string, lang: string, kind: CellKind, output?: IOutputDto[], metadata?: NotebookCellMetadata][], callback: (diffModel: INotebookDiffEditorModel, accessor: TestInstantiationService) => Promise<R> | R): Promise<R> {
export async function withTestNotebookDiffModel<R = any>(originalCells: [source: string, lang: string, kind: CellKind, output?: IOutputDto[], metadata?: NotebookCellMetadata][], modifiedCells: [source: string, lang: string, kind: CellKind, output?: IOutputDto[], metadata?: NotebookCellMetadata][], callback: (diffModel: INotebookDiffEditorModel, disposables: DisposableStore, accessor: TestInstantiationService) => Promise<R> | R): Promise<R> {
const disposables = new DisposableStore();
const instantiationService = setupInstantiationService(disposables);
const originalNotebook = createTestNotebookEditor(instantiationService, disposables, originalCells);
const modifiedNotebook = createTestNotebookEditor(instantiationService, disposables, modifiedCells);
Expand All @@ -368,7 +369,7 @@ export async function withTestNotebookDiffModel<R = any>(disposables: Disposable
}
};

const res = await callback(model, instantiationService);
const res = await callback(model, disposables, instantiationService);
if (res instanceof Promise) {
res.finally(() => {
originalNotebook.editor.dispose();
Expand Down