From 65b22997d7addf8d34400207bc8a3d7c4cf07f70 Mon Sep 17 00:00:00 2001 From: Alexandru Dima Date: Sun, 26 Oct 2025 15:11:46 +0100 Subject: [PATCH] Convert flaky API tests to unit tests (#273398) Convert flaky API test to unit test (#253863 , #254041) I maintain my conviction that there is an unrelated run-away API test which steals focus while these tests execute which then leads to these tests failing, since the undo command is sensitive to the current focused editor. --- .../src/singlefolder-tests/editor.test.ts | 49 +----- .../src/singlefolder-tests/workspace.test.ts | 34 ---- .../test/browser/mainThreadEditors.test.ts | 157 ++++++++++++++++-- 3 files changed, 147 insertions(+), 93 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/editor.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/editor.test.ts index 1bd22369152..ce8e68e0c1e 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/editor.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/editor.test.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; -import { commands, env, Position, Range, Selection, SnippetString, TextDocument, TextEditor, TextEditorCursorStyle, TextEditorLineNumbersStyle, Uri, window, workspace } from 'vscode'; +import { env, Position, Range, Selection, SnippetString, TextDocument, TextEditor, TextEditorCursorStyle, TextEditorLineNumbersStyle, Uri, window, workspace } from 'vscode'; import { assertNoRpc, closeAllEditors, createRandomFile, deleteFile } from '../utils'; suite('vscode API - editors', () => { @@ -167,53 +167,6 @@ suite('vscode API - editors', () => { }); }); - function executeReplace(editor: TextEditor, range: Range, text: string, undoStopBefore: boolean, undoStopAfter: boolean): Thenable { - return editor.edit((builder) => { - builder.replace(range, text); - }, { undoStopBefore: undoStopBefore, undoStopAfter: undoStopAfter }); - } - - test.skip('TextEditor.edit can control undo/redo stack 1', () => { - return withRandomFileEditor('Hello world!', async (editor, doc) => { - const applied1 = await executeReplace(editor, new Range(0, 0, 0, 1), 'h', false, false); - assert.ok(applied1); - assert.strictEqual(doc.getText(), 'hello world!'); - assert.ok(doc.isDirty); - - const applied2 = await executeReplace(editor, new Range(0, 1, 0, 5), 'ELLO', false, false); - assert.ok(applied2); - assert.strictEqual(doc.getText(), 'hELLO world!'); - assert.ok(doc.isDirty); - - await commands.executeCommand('undo'); - if (doc.getText() === 'hello world!') { - // see https://github.com/microsoft/vscode/issues/109131 - // it looks like an undo stop was inserted in between these two edits - // it is unclear why this happens, but it can happen for a multitude of reasons - await commands.executeCommand('undo'); - } - assert.strictEqual(doc.getText(), 'Hello world!'); - }); - }); - - test.skip('TextEditor.edit can control undo/redo stack 2', () => { - return withRandomFileEditor('Hello world!', (editor, doc) => { - return executeReplace(editor, new Range(0, 0, 0, 1), 'h', false, false).then(applied => { - assert.ok(applied); - assert.strictEqual(doc.getText(), 'hello world!'); - assert.ok(doc.isDirty); - return executeReplace(editor, new Range(0, 1, 0, 5), 'ELLO', true, false); - }).then(applied => { - assert.ok(applied); - assert.strictEqual(doc.getText(), 'hELLO world!'); - assert.ok(doc.isDirty); - return commands.executeCommand('undo'); - }).then(_ => { - assert.strictEqual(doc.getText(), 'hello world!'); - }); - }); - }); - test('issue #16573: Extension API: insertSpaces and tabSize are undefined', () => { return withRandomFileEditor('Hello world!\n\tHello world!', (editor, _doc) => { diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts index 3bcaecb2a71..19f72931512 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts @@ -1179,40 +1179,6 @@ suite('vscode API - workspace', () => { }); - test.skip('issue #110141 - TextEdit.setEndOfLine applies an edit and invalidates redo stack even when no change is made', async () => { - const file = await createRandomFile('hello\nworld'); - - const document = await vscode.workspace.openTextDocument(file); - await vscode.window.showTextDocument(document); - - // apply edit - { - const we = new vscode.WorkspaceEdit(); - we.insert(file, new vscode.Position(0, 5), '2'); - await vscode.workspace.applyEdit(we); - } - - // check the document - { - assert.strictEqual(document.getText(), 'hello2\nworld'); - assert.strictEqual(document.isDirty, true); - } - - // apply no-op edit - { - const we = new vscode.WorkspaceEdit(); - we.set(file, [vscode.TextEdit.setEndOfLine(vscode.EndOfLine.LF)]); - await vscode.workspace.applyEdit(we); - } - - // undo - { - await vscode.commands.executeCommand('undo'); - assert.strictEqual(document.getText(), 'hello\nworld'); - assert.strictEqual(document.isDirty, false); - } - }); - test('SnippetString in WorkspaceEdit', async function (): Promise { const file = await createRandomFile('hello\nworld'); diff --git a/src/vs/workbench/api/test/browser/mainThreadEditors.test.ts b/src/vs/workbench/api/test/browser/mainThreadEditors.test.ts index 511856e6607..dc04aae1ce1 100644 --- a/src/vs/workbench/api/test/browser/mainThreadEditors.test.ts +++ b/src/vs/workbench/api/test/browser/mainThreadEditors.test.ts @@ -11,12 +11,12 @@ import { mock } from '../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; import { IBulkEditService } from '../../../../editor/browser/services/bulkEditService.js'; import { ICodeEditorService } from '../../../../editor/browser/services/codeEditorService.js'; -import { EditOperation } from '../../../../editor/common/core/editOperation.js'; +import { EditOperation, ISingleEditOperation } from '../../../../editor/common/core/editOperation.js'; import { Position } from '../../../../editor/common/core/position.js'; import { Range } from '../../../../editor/common/core/range.js'; import { ILanguageService } from '../../../../editor/common/languages/language.js'; import { ILanguageConfigurationService } from '../../../../editor/common/languages/languageConfigurationRegistry.js'; -import { ITextSnapshot } from '../../../../editor/common/model.js'; +import { EndOfLineSequence, ITextSnapshot } from '../../../../editor/common/model.js'; import { IEditorWorkerService } from '../../../../editor/common/services/editorWorker.js'; import { LanguageService } from '../../../../editor/common/services/languageService.js'; import { IModelService } from '../../../../editor/common/services/model.js'; @@ -59,23 +59,36 @@ import { IWorkingCopyService } from '../../../services/workingCopy/common/workin import { TestEditorGroupsService, TestEditorService, TestEnvironmentService, TestLifecycleService, TestWorkingCopyService } from '../../../test/browser/workbenchTestServices.js'; import { TestContextService, TestFileService, TestTextResourcePropertiesService } from '../../../test/common/workbenchTestServices.js'; import { MainThreadBulkEdits } from '../../browser/mainThreadBulkEdits.js'; +import { MainThreadTextEditors, IMainThreadEditorLocator } from '../../browser/mainThreadEditors.js'; +import { MainThreadTextEditor } from '../../browser/mainThreadEditor.js'; +import { MainThreadDocuments } from '../../browser/mainThreadDocuments.js'; import { IWorkspaceTextEditDto } from '../../common/extHost.protocol.js'; import { SingleProxyRPCProtocol } from '../common/testRPCProtocol.js'; +import { ITextResourcePropertiesService } from '../../../../editor/common/services/textResourceConfiguration.js'; +import { IClipboardService } from '../../../../platform/clipboard/common/clipboardService.js'; +import { TestClipboardService } from '../../../../platform/clipboard/test/common/testClipboardService.js'; +import { createTestCodeEditor } from '../../../../editor/test/browser/testCodeEditor.js'; suite('MainThreadEditors', () => { let disposables: DisposableStore; + const existingResource = URI.parse('foo:existing'); const resource = URI.parse('foo:bar'); let modelService: IModelService; let bulkEdits: MainThreadBulkEdits; + let editors: MainThreadTextEditors; + let editorLocator: IMainThreadEditorLocator; + let testEditor: MainThreadTextEditor; const movedResources = new Map(); const copiedResources = new Map(); const createdResources = new Set(); const deletedResources = new Set(); + const editorId = 'testEditorId'; + setup(() => { disposables = new DisposableStore(); @@ -101,7 +114,8 @@ suite('MainThreadEditors', () => { services.set(IDialogService, dialogService); services.set(INotificationService, notificationService); services.set(IUndoRedoService, undoRedoService); - services.set(IModelService, modelService); + services.set(ITextResourcePropertiesService, new SyncDescriptor(TestTextResourcePropertiesService)); + services.set(IModelService, new SyncDescriptor(ModelService)); services.set(ICodeEditorService, new TestCodeEditorService(themeService)); services.set(IFileService, new TestFileService()); services.set(IUriIdentityService, new SyncDescriptor(UriIdentityService)); @@ -110,13 +124,19 @@ suite('MainThreadEditors', () => { services.set(ILifecycleService, new TestLifecycleService()); services.set(IWorkingCopyService, new TestWorkingCopyService()); services.set(IEditorGroupsService, new TestEditorGroupsService()); + services.set(IClipboardService, new TestClipboardService()); services.set(ITextFileService, new class extends mock() { override isDirty() { return false; } // eslint-disable-next-line local/code-no-any-casts override files = { onDidSave: Event.None, onDidRevert: Event.None, - onDidChangeDirty: Event.None + onDidChangeDirty: Event.None, + onDidChangeEncoding: Event.None + }; + // eslint-disable-next-line local/code-no-any-casts + override untitled = { + onDidChangeEncoding: Event.None }; override create(operations: { resource: URI }[]) { for (const o of operations) { @@ -181,14 +201,33 @@ suite('MainThreadEditors', () => { const instaService = new InstantiationService(services); - modelService = new ModelService( - configService, - new TestTextResourcePropertiesService(configService), - undoRedoService, - instaService - ); - bulkEdits = instaService.createInstance(MainThreadBulkEdits, SingleProxyRPCProtocol(null)); + const documents = instaService.createInstance(MainThreadDocuments, SingleProxyRPCProtocol(null)); + + // Create editor locator + editorLocator = { + getEditor(id: string): MainThreadTextEditor | undefined { + return id === editorId ? testEditor : undefined; + }, + findTextEditorIdFor() { return undefined; }, + getIdOfCodeEditor() { return undefined; } + }; + + editors = instaService.createInstance(MainThreadTextEditors, editorLocator, SingleProxyRPCProtocol(null)); + modelService = instaService.invokeFunction(accessor => accessor.get(IModelService)); + + // Create a test code editor using the helper + const model = modelService.createModel('Hello world!', null, existingResource); + const testCodeEditor = disposables.add(createTestCodeEditor(model)); + + testEditor = disposables.add(instaService.createInstance( + MainThreadTextEditor, + editorId, + model, + testCodeEditor, + { onGainedFocus() { }, onLostFocus() { } }, + documents + )); }); teardown(() => { @@ -250,6 +289,48 @@ suite('MainThreadEditors', () => { return Promise.all([p1, p2]); }); + test('applyWorkspaceEdit: noop eol edit keeps undo stack clean', async () => { + + const initialText = 'hello\nworld'; + const model = disposables.add(modelService.createModel(initialText, null, resource)); + const initialAlternativeVersionId = model.getAlternativeVersionId(); + + const insertEdit: IWorkspaceTextEditDto = { + resource: resource, + versionId: model.getVersionId(), + textEdit: { + range: new Range(1, 6, 1, 6), + text: '2' + } + }; + + const insertResult = await bulkEdits.$tryApplyWorkspaceEdit(new SerializableObjectWithBuffers({ edits: [insertEdit] })); + assert.strictEqual(insertResult, true); + assert.strictEqual(model.getValue(), 'hello2\nworld'); + assert.notStrictEqual(model.getAlternativeVersionId(), initialAlternativeVersionId); + + const eolEdit: IWorkspaceTextEditDto = { + resource: resource, + versionId: model.getVersionId(), + textEdit: { + range: new Range(1, 1, 1, 1), + text: '', + eol: EndOfLineSequence.LF + } + }; + + const eolResult = await bulkEdits.$tryApplyWorkspaceEdit(new SerializableObjectWithBuffers({ edits: [eolEdit] })); + assert.strictEqual(eolResult, true); + assert.strictEqual(model.getValue(), 'hello2\nworld'); + + const undoResult = model.undo(); + if (undoResult) { + await undoResult; + } + assert.strictEqual(model.getValue(), initialText); + assert.strictEqual(model.getAlternativeVersionId(), initialAlternativeVersionId); + }); + test(`applyWorkspaceEdit with only resource edit`, () => { return bulkEdits.$tryApplyWorkspaceEdit(new SerializableObjectWithBuffers({ edits: [ @@ -264,4 +345,58 @@ suite('MainThreadEditors', () => { assert.strictEqual(deletedResources.has(resource), true); }); }); + + test('applyWorkspaceEdit can control undo/redo stack 1', async () => { + const model = modelService.getModel(existingResource)!; + + const edit1: ISingleEditOperation = { + range: new Range(1, 1, 1, 2), + text: 'h', + forceMoveMarkers: false + }; + + const applied1 = await editors.$tryApplyEdits(editorId, model.getVersionId(), [edit1], { undoStopBefore: false, undoStopAfter: false }); + assert.strictEqual(applied1, true); + assert.strictEqual(model.getValue(), 'hello world!'); + + const edit2: ISingleEditOperation = { + range: new Range(1, 2, 1, 6), + text: 'ELLO', + forceMoveMarkers: false + }; + + const applied2 = await editors.$tryApplyEdits(editorId, model.getVersionId(), [edit2], { undoStopBefore: false, undoStopAfter: false }); + assert.strictEqual(applied2, true); + assert.strictEqual(model.getValue(), 'hELLO world!'); + + await model.undo(); + assert.strictEqual(model.getValue(), 'Hello world!'); + }); + + test('applyWorkspaceEdit can control undo/redo stack 2', async () => { + const model = modelService.getModel(existingResource)!; + + const edit1: ISingleEditOperation = { + range: new Range(1, 1, 1, 2), + text: 'h', + forceMoveMarkers: false + }; + + const applied1 = await editors.$tryApplyEdits(editorId, model.getVersionId(), [edit1], { undoStopBefore: false, undoStopAfter: false }); + assert.strictEqual(applied1, true); + assert.strictEqual(model.getValue(), 'hello world!'); + + const edit2: ISingleEditOperation = { + range: new Range(1, 2, 1, 6), + text: 'ELLO', + forceMoveMarkers: false + }; + + const applied2 = await editors.$tryApplyEdits(editorId, model.getVersionId(), [edit2], { undoStopBefore: true, undoStopAfter: false }); + assert.strictEqual(applied2, true); + assert.strictEqual(model.getValue(), 'hELLO world!'); + + await model.undo(); + assert.strictEqual(model.getValue(), 'hello world!'); + }); });