From 04efea43fa3ab33e8a50bbf3caf0270f6b0b3030 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Tue, 12 Jan 2021 12:57:22 -0800 Subject: [PATCH] testing: peek diff test outputs --- .../editor/browser/widget/diffEditorWidget.ts | 20 ++- .../api/browser/mainThreadTesting.ts | 1 + .../workbench/api/common/extHost.protocol.ts | 4 +- src/vs/workbench/api/common/extHostTesting.ts | 10 ++ .../hierarchalByLocation.ts | 2 +- .../explorerProjections/stateByLocation.ts | 2 +- .../explorerProjections/stateByName.ts | 2 +- .../contrib/testing/browser/media/testing.css | 8 ++ .../testing/browser/testExplorerActions.ts | 2 +- .../testing/browser/testExplorerTree.ts | 4 +- .../testing/browser/testing.contribution.ts | 52 +++++-- .../testing/browser/testingExplorerView.ts | 74 ++++++++-- .../testing/browser/testingOutputPeek.ts | 134 ++++++++++++++---- .../contrib/testing/browser/theme.ts | 8 +- .../contrib/testing/common/testService.ts | 22 +-- .../contrib/testing/common/testServiceImpl.ts | 9 +- .../testingCollectionService.ts | 27 +++- .../testing/common/testingContentProvider.ts | 64 +++++++++ .../testing/common/testingContextKeys.ts | 1 + .../contrib/testing/common/testingUri.ts | 77 ++++++++++ 20 files changed, 452 insertions(+), 71 deletions(-) rename src/vs/workbench/contrib/testing/{browser => common}/testingCollectionService.ts (91%) create mode 100644 src/vs/workbench/contrib/testing/common/testingContentProvider.ts create mode 100644 src/vs/workbench/contrib/testing/common/testingUri.ts diff --git a/src/vs/editor/browser/widget/diffEditorWidget.ts b/src/vs/editor/browser/widget/diffEditorWidget.ts index becef8869c9..014a1d68506 100644 --- a/src/vs/editor/browser/widget/diffEditorWidget.ts +++ b/src/vs/editor/browser/widget/diffEditorWidget.ts @@ -319,7 +319,9 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE this._register(dom.addStandardDisposableListener(this._overviewDomElement, 'mousedown', (e) => { this._modifiedEditor.delegateVerticalScrollbarMouseDown(e); })); - this._containerDomElement.appendChild(this._overviewDomElement); + if (this._renderOverviewRuler) { + this._containerDomElement.appendChild(this._overviewDomElement); + } // Create left side this._originalDomNode = document.createElement('div'); @@ -646,7 +648,9 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE this._modifiedOverviewRuler.dispose(); } this._overviewDomElement.removeChild(this._overviewViewportDomElement.domNode); - this._containerDomElement.removeChild(this._overviewDomElement); + if (this._renderOverviewRuler) { + this._containerDomElement.removeChild(this._overviewDomElement); + } this._containerDomElement.removeChild(this._originalDomNode); this._originalEditor.dispose(); @@ -759,6 +763,16 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE // Update class name this._containerDomElement.className = DiffEditorWidget._getClassName(this._themeService.getColorTheme(), this._renderSideBySide); } + + // renderOverviewRuler + if (typeof newOptions.renderOverviewRuler !== 'undefined' && this._renderOverviewRuler !== newOptions.renderOverviewRuler) { + this._renderOverviewRuler = newOptions.renderOverviewRuler; + if (this._renderOverviewRuler) { + this._containerDomElement.appendChild(this._overviewDomElement); + } else { + this._containerDomElement.removeChild(this._overviewDomElement); + } + } } public getModel(): editorCommon.IDiffEditorModel { @@ -768,7 +782,7 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE }; } - public setModel(model: editorCommon.IDiffEditorModel): void { + public setModel(model: editorCommon.IDiffEditorModel | null): void { // Guard us against partial null model if (model && (!model.original || !model.modified)) { throw new Error(!model.original ? 'DiffEditorWidget.setModel: Original model is null' : 'DiffEditorWidget.setModel: Modified model is null'); diff --git a/src/vs/workbench/api/browser/mainThreadTesting.ts b/src/vs/workbench/api/browser/mainThreadTesting.ts index b4f0f219e30..4e255fc43a2 100644 --- a/src/vs/workbench/api/browser/mainThreadTesting.ts +++ b/src/vs/workbench/api/browser/mainThreadTesting.ts @@ -53,6 +53,7 @@ export class MainThreadTesting extends Disposable implements MainThreadTestingSh public $registerTestProvider(id: string) { this.testService.registerTestController(id, { runTests: (req, token) => this.proxy.$runTestsForProvider(req, token), + lookupTest: test => this.proxy.$lookupTest(test), }); } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index fd686128317..d43726ec006 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -58,7 +58,7 @@ import { ISerializableEnvironmentVariableCollection } from 'vs/workbench/contrib import { DebugConfigurationProviderTriggerKind } from 'vs/workbench/api/common/extHostTypes'; import { IAccessibilityInformation } from 'vs/platform/accessibility/common/accessibility'; import { IExtensionIdWithVersion } from 'vs/platform/userDataSync/common/extensionsStorageSync'; -import { RunTestForProviderRequest, RunTestsRequest, RunTestsResult, TestsDiff } from 'vs/workbench/contrib/testing/common/testCollection'; +import { InternalTestItem, RunTestForProviderRequest, RunTestsRequest, RunTestsResult, TestIdWithProvider, TestsDiff } from 'vs/workbench/contrib/testing/common/testCollection'; export interface IEnvironment { isExtensionDevelopmentDebug: boolean; @@ -1807,7 +1807,7 @@ export interface ExtHostTestingShape { $runTestsForProvider(req: RunTestForProviderRequest, token: CancellationToken): Promise; $subscribeToTests(resource: ExtHostTestingResource, uri: UriComponents): void; $unsubscribeFromTests(resource: ExtHostTestingResource, uri: UriComponents): void; - + $lookupTest(test: TestIdWithProvider): Promise; $acceptDiff(resource: ExtHostTestingResource, uri: UriComponents, diff: TestsDiff): void; } diff --git a/src/vs/workbench/api/common/extHostTesting.ts b/src/vs/workbench/api/common/extHostTesting.ts index 28a6363711a..4ee971c82f6 100644 --- a/src/vs/workbench/api/common/extHostTesting.ts +++ b/src/vs/workbench/api/common/extHostTesting.ts @@ -212,6 +212,16 @@ export class ExtHostTesting implements ExtHostTestingShape { throw e; } } + + public $lookupTest(req: TestIdWithProvider): Promise { + const owned = this.ownedTests.getTestById(req.testId); + if (!owned) { + return Promise.resolve(undefined); + } + + const { actual, previousChildren, previousEquals, ...item } = owned; + return Promise.resolve(item); + } } const keyMap: { [K in keyof Omit]: null } = { diff --git a/src/vs/workbench/contrib/testing/browser/explorerProjections/hierarchalByLocation.ts b/src/vs/workbench/contrib/testing/browser/explorerProjections/hierarchalByLocation.ts index 3731594fbe9..b28bebf82aa 100644 --- a/src/vs/workbench/contrib/testing/browser/explorerProjections/hierarchalByLocation.ts +++ b/src/vs/workbench/contrib/testing/browser/explorerProjections/hierarchalByLocation.ts @@ -15,7 +15,7 @@ import { IWorkspaceFolder, IWorkspaceFoldersChangeEvent } from 'vs/platform/work import { ITestTreeElement, ITestTreeProjection } from 'vs/workbench/contrib/testing/browser/explorerProjections'; import { HierarchicalElement, HierarchicalFolder } from 'vs/workbench/contrib/testing/browser/explorerProjections/hierarchalNodes'; import { locationsEqual, TestLocationStore } from 'vs/workbench/contrib/testing/browser/explorerProjections/locationStore'; -import { TestSubscriptionListener } from 'vs/workbench/contrib/testing/browser/testingCollectionService'; +import { TestSubscriptionListener } from 'vs/workbench/contrib/testing/common/testingCollectionService'; import { InternalTestItem, TestDiffOpType, TestsDiff } from 'vs/workbench/contrib/testing/common/testCollection'; /** diff --git a/src/vs/workbench/contrib/testing/browser/explorerProjections/stateByLocation.ts b/src/vs/workbench/contrib/testing/browser/explorerProjections/stateByLocation.ts index 9f22cbea700..c0923c97625 100644 --- a/src/vs/workbench/contrib/testing/browser/explorerProjections/stateByLocation.ts +++ b/src/vs/workbench/contrib/testing/browser/explorerProjections/stateByLocation.ts @@ -18,7 +18,7 @@ import { locationsEqual, TestLocationStore } from 'vs/workbench/contrib/testing/ import { isRunningState, NodeChangeList } from 'vs/workbench/contrib/testing/browser/explorerProjections/nodeHelper'; import { StateElement } from 'vs/workbench/contrib/testing/browser/explorerProjections/stateNodes'; import { statesInOrder } from 'vs/workbench/contrib/testing/browser/testExplorerTree'; -import { TestSubscriptionListener } from 'vs/workbench/contrib/testing/browser/testingCollectionService'; +import { TestSubscriptionListener } from 'vs/workbench/contrib/testing/common/testingCollectionService'; import { AbstractIncrementalTestCollection, IncrementalChangeCollector, IncrementalTestCollectionItem, InternalTestItem, TestDiffOpType, TestIdWithProvider, TestsDiff } from 'vs/workbench/contrib/testing/common/testCollection'; interface IStatusTestItem extends IncrementalTestCollectionItem { diff --git a/src/vs/workbench/contrib/testing/browser/explorerProjections/stateByName.ts b/src/vs/workbench/contrib/testing/browser/explorerProjections/stateByName.ts index e9aa93cd0de..070c9a11b82 100644 --- a/src/vs/workbench/contrib/testing/browser/explorerProjections/stateByName.ts +++ b/src/vs/workbench/contrib/testing/browser/explorerProjections/stateByName.ts @@ -18,7 +18,7 @@ import { ListElementType } from 'vs/workbench/contrib/testing/browser/explorerPr import { locationsEqual, TestLocationStore } from 'vs/workbench/contrib/testing/browser/explorerProjections/locationStore'; import { NodeChangeList } from 'vs/workbench/contrib/testing/browser/explorerProjections/nodeHelper'; import { StateElement } from 'vs/workbench/contrib/testing/browser/explorerProjections/stateNodes'; -import { TestSubscriptionListener } from 'vs/workbench/contrib/testing/browser/testingCollectionService'; +import { TestSubscriptionListener } from 'vs/workbench/contrib/testing/common/testingCollectionService'; import { AbstractIncrementalTestCollection, IncrementalChangeCollector, IncrementalTestCollectionItem, InternalTestItem, TestDiffOpType, TestIdWithProvider, TestsDiff } from 'vs/workbench/contrib/testing/common/testCollection'; class ListTestStateElement implements ITestTreeElement { diff --git a/src/vs/workbench/contrib/testing/browser/media/testing.css b/src/vs/workbench/contrib/testing/browser/media/testing.css index 5ff9ccc79cf..62c273375b6 100644 --- a/src/vs/workbench/contrib/testing/browser/media/testing.css +++ b/src/vs/workbench/contrib/testing/browser/media/testing.css @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +/** -- explorer */ .test-explorer { display: flex; flex-direction: column; @@ -60,3 +61,10 @@ /* Use steps to throttle FPS to reduce CPU usage */ animation: codicon-spin 1.25s steps(30) infinite; } + +/** -- peek */ +.monaco-editor .zone-widget .zone-widget-container.peekview-widget { + border-top-width: 2px; + border-bottom-width: 2px; +} + diff --git a/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts b/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts index 1f70c910444..51ed459afe9 100644 --- a/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts +++ b/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts @@ -13,7 +13,7 @@ import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation import { ThemeIcon } from 'vs/platform/theme/common/themeService'; import { ViewAction } from 'vs/workbench/browser/parts/views/viewPane'; import * as icons from 'vs/workbench/contrib/testing/browser/icons'; -import { ITestingCollectionService } from 'vs/workbench/contrib/testing/browser/testingCollectionService'; +import { ITestingCollectionService } from 'vs/workbench/contrib/testing/common/testingCollectionService'; import { TestingExplorerView, TestingExplorerViewModel } from 'vs/workbench/contrib/testing/browser/testingExplorerView'; import { TestExplorerViewGrouping, TestExplorerViewMode, Testing } from 'vs/workbench/contrib/testing/common/constants'; import { EMPTY_TEST_RESULT, InternalTestItem, RunTestsResult, TestIdWithProvider } from 'vs/workbench/contrib/testing/common/testCollection'; diff --git a/src/vs/workbench/contrib/testing/browser/testExplorerTree.ts b/src/vs/workbench/contrib/testing/browser/testExplorerTree.ts index 791f685e1db..e6510f57f07 100644 --- a/src/vs/workbench/contrib/testing/browser/testExplorerTree.ts +++ b/src/vs/workbench/contrib/testing/browser/testExplorerTree.ts @@ -7,7 +7,6 @@ import { TestRunState } from 'vs/workbench/api/common/extHostTypes'; export type TreeStateNode = { statusNode: true; state: TestRunState; priority: number }; - /** * List of display priorities for different run states. When tests update, * the highest-priority state from any of their children will be the state @@ -23,6 +22,9 @@ export const statePriority: { [K in TestRunState]: number } = { [TestRunState.Unset]: 0, }; + +export const isFailedState = (s: TestRunState) => s === TestRunState.Errored || s === TestRunState.Failed; + export const stateNodes = Object.entries(statePriority).reduce( (acc, [stateStr, priority]) => { const state = Number(stateStr) as TestRunState; diff --git a/src/vs/workbench/contrib/testing/browser/testing.contribution.ts b/src/vs/workbench/contrib/testing/browser/testing.contribution.ts index c74935f2ddd..ad79e2a599f 100644 --- a/src/vs/workbench/contrib/testing/browser/testing.contribution.ts +++ b/src/vs/workbench/contrib/testing/browser/testing.contribution.ts @@ -3,9 +3,11 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { Codicon } from 'vs/base/common/codicons'; +import { KeyCode } from 'vs/base/common/keyCodes'; import { URI } from 'vs/base/common/uri'; -import { isCodeEditor } from 'vs/editor/browser/editorBrowser'; -import { registerEditorContribution } from 'vs/editor/browser/editorExtensions'; +import { ICodeEditor, isCodeEditor } from 'vs/editor/browser/editorBrowser'; +import { EditorAction2, registerEditorContribution } from 'vs/editor/browser/editorExtensions'; import { localize } from 'vs/nls'; import { registerAction2 } from 'vs/platform/actions/common/actions'; import { CommandsRegistry } from 'vs/platform/commands/common/commands'; @@ -13,19 +15,23 @@ import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey'; import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors'; import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; +import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry'; import { Registry } from 'vs/platform/registry/common/platform'; +import { Extensions as WorkbenchExtensions, IWorkbenchContributionsRegistry } from 'vs/workbench/common/contributions'; import { Extensions as ViewContainerExtensions, IViewContainersRegistry, IViewsRegistry, ViewContainerLocation } from 'vs/workbench/common/views'; import { testingViewIcon } from 'vs/workbench/contrib/testing/browser/icons'; -import { ITestingCollectionService, TestingCollectionService } from 'vs/workbench/contrib/testing/browser/testingCollectionService'; import { TestingExplorerView } from 'vs/workbench/contrib/testing/browser/testingExplorerView'; import { TestingOutputPeekController } from 'vs/workbench/contrib/testing/browser/testingOutputPeek'; import { TestingViewPaneContainer } from 'vs/workbench/contrib/testing/browser/testingViewPaneContainer'; import { Testing } from 'vs/workbench/contrib/testing/common/constants'; -import { ITestMessage, TestIdWithProvider } from 'vs/workbench/contrib/testing/common/testCollection'; +import { TestIdWithProvider } from 'vs/workbench/contrib/testing/common/testCollection'; +import { ITestingCollectionService, TestingCollectionService } from 'vs/workbench/contrib/testing/common/testingCollectionService'; +import { TestingContentProvider } from 'vs/workbench/contrib/testing/common/testingContentProvider'; import { TestingContextKeys } from 'vs/workbench/contrib/testing/common/testingContextKeys'; import { ITestService } from 'vs/workbench/contrib/testing/common/testService'; import { TestService } from 'vs/workbench/contrib/testing/common/testServiceImpl'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; +import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle'; import * as Action from './testExplorerActions'; registerSingleton(ITestService, TestService); @@ -82,6 +88,8 @@ registerAction2(Action.TestingGroupByLocationAction); registerAction2(Action.TestingGroupByStatusAction); registerAction2(Action.RefreshTestsAction); +Registry.as(WorkbenchExtensions.Workbench).registerWorkbenchContribution(TestingContentProvider, LifecyclePhase.Eventually); + registerEditorContribution(Testing.OutputPeekContributionId, TestingOutputPeekController); CommandsRegistry.registerCommand({ @@ -102,13 +110,17 @@ CommandsRegistry.registerCommand({ CommandsRegistry.registerCommand({ id: 'vscode.revealTestMessage', - handler: async (accessor: ServicesAccessor, message: ITestMessage) => { - if (!message.location) { - console.warn('Cannot reveal a test message without an associated location'); + handler: async (accessor: ServicesAccessor, testRef: TestIdWithProvider, messageIndex: number) => { + const editorService = accessor.get(IEditorService); + const testService = accessor.get(ITestService); + + const test = await testService.lookupTest(testRef); + const message = test?.item.state.messages[messageIndex]; + if (!test || !message?.location) { return; } - const pane = await accessor.get(IEditorService).openEditor({ + const pane = await editorService.openEditor({ resource: URI.revive(message.location.uri), options: { selection: message.location.range } }); @@ -118,6 +130,28 @@ CommandsRegistry.registerCommand({ return; } - TestingOutputPeekController.get(control).show(message); + TestingOutputPeekController.get(control).show(test, messageIndex); + } +}); + +registerAction2(class CloseTestPeek extends EditorAction2 { + constructor() { + super({ + id: 'editor.closeTestPeek', + title: localize('close', 'Close'), + icon: Codicon.close, + precondition: ContextKeyExpr.and( + TestingContextKeys.peekVisible, + ContextKeyExpr.not('config.editor.stablePeek') + ), + keybinding: { + weight: KeybindingWeight.WorkbenchContrib + 10, + primary: KeyCode.Escape + } + }); + } + + runEditorCommand(_accessor: ServicesAccessor, editor: ICodeEditor): void { + TestingOutputPeekController.get(editor).removePeek(); } }); diff --git a/src/vs/workbench/contrib/testing/browser/testingExplorerView.ts b/src/vs/workbench/contrib/testing/browser/testingExplorerView.ts index 414492fcd2f..df7830264a0 100644 --- a/src/vs/workbench/contrib/testing/browser/testingExplorerView.ts +++ b/src/vs/workbench/contrib/testing/browser/testingExplorerView.ts @@ -17,7 +17,7 @@ import { splitGlobAware } from 'vs/base/common/glob'; import { Iterable } from 'vs/base/common/iterator'; import { Disposable, DisposableStore, toDisposable } from 'vs/base/common/lifecycle'; import 'vs/css!./media/testing'; -import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; +import { ICodeEditor, isCodeEditor } from 'vs/editor/browser/editorBrowser'; import { ICodeEditorService } from 'vs/editor/browser/services/codeEditorService'; import { localize } from 'vs/nls'; import { MenuEntryActionViewItem } from 'vs/platform/actions/browser/menuEntryActionViewItem'; @@ -48,14 +48,15 @@ import { StateByLocationProjection } from 'vs/workbench/contrib/testing/browser/ import { StateByNameProjection } from 'vs/workbench/contrib/testing/browser/explorerProjections/stateByName'; import { StateElement } from 'vs/workbench/contrib/testing/browser/explorerProjections/stateNodes'; import { testingStatesToIcons } from 'vs/workbench/contrib/testing/browser/icons'; -import { cmpPriority } from 'vs/workbench/contrib/testing/browser/testExplorerTree'; -import { ITestingCollectionService, TestSubscriptionListener } from 'vs/workbench/contrib/testing/browser/testingCollectionService'; +import { cmpPriority, isFailedState } from 'vs/workbench/contrib/testing/browser/testExplorerTree'; +import { ITestingCollectionService, TestSubscriptionListener } from 'vs/workbench/contrib/testing/common/testingCollectionService'; import { TestingExplorerFilter, TestingFilterState } from 'vs/workbench/contrib/testing/browser/testingExplorerFilter'; import { TestExplorerViewGrouping, TestExplorerViewMode } from 'vs/workbench/contrib/testing/common/constants'; import { TestingContextKeys } from 'vs/workbench/contrib/testing/common/testingContextKeys'; import { ITestService } from 'vs/workbench/contrib/testing/common/testService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { DebugAction, RunAction } from './testExplorerActions'; +import { TestingOutputPeekController } from 'vs/workbench/contrib/testing/browser/testingOutputPeek'; export class TestingExplorerView extends ViewPane { public viewModel!: TestingExplorerViewModel; @@ -207,7 +208,7 @@ export class TestingExplorerViewModel extends Disposable { private listener: TestSubscriptionListener | undefined, filterState: TestingFilterState, @IInstantiationService instantiationService: IInstantiationService, - @IEditorService editorService: IEditorService, + @IEditorService private readonly editorService: IEditorService, @ICodeEditorService codeEditorService: ICodeEditorService, @IStorageService private readonly storageService: IStorageService, @IContextKeyService private readonly contextKeyService: IContextKeyService, @@ -247,15 +248,10 @@ export class TestingExplorerViewModel extends Disposable { this.onDidChangeSelection = this.tree.onDidChangeSelection; this._register(this.tree.onDidChangeSelection(evt => { - const location = evt.elements[0]?.location; - if (!location || !evt.browserEvent) { - return; + const selected = evt.elements[0]; + if (selected && evt.browserEvent) { + this.openEditorForItem(selected); } - - editorService.openEditor({ - resource: location.uri, - options: { selection: location.range, preserveFocus: true } - }); })); const tracker = this._register(new CodeEditorTracker(codeEditorService, this)); @@ -313,6 +309,60 @@ export class TestingExplorerViewModel extends Disposable { this.tree.setSelection([item]); } + /** + * Opens an editor for the item. If there is a failure associated with the + * test item, it will be shown. + */ + private async openEditorForItem(item: ITestTreeElement) { + if (await this.tryPeekError(item)) { + return; + } + + const location = item?.location; + if (!location) { + return; + } + + const pane = await this.editorService.openEditor({ + resource: location.uri, + options: { selection: location.range, preserveFocus: true } + }); + + // if the user selected a failed test and now they didn't, hide the peek + const control = pane?.getControl(); + if (isCodeEditor(control)) { + TestingOutputPeekController.get(control).removePeek(); + } + } + + /** + * Tries to peek the first test error, if the item is in a failed state. + */ + private async tryPeekError(item: ITestTreeElement) { + if (!item.test || !isFailedState(item.test.item.state.runState)) { + return false; + } + + const index = item.test.item.state.messages.findIndex(m => !!m.location); + if (index === -1) { + return; + } + + const message = item.test.item.state.messages[index]; + const pane = await this.editorService.openEditor({ + resource: message.location!.uri, + options: { selection: message.location!.range, preserveFocus: true } + }); + + const control = pane?.getControl(); + if (!isCodeEditor(control)) { + return false; + } + + TestingOutputPeekController.get(control).show(item.test, index); + return true; + } + private updatePreferredProjection() { this.projection?.dispose(); if (!this.listener) { diff --git a/src/vs/workbench/contrib/testing/browser/testingOutputPeek.ts b/src/vs/workbench/contrib/testing/browser/testingOutputPeek.ts index 21de43168dd..9a8996d30fb 100644 --- a/src/vs/workbench/contrib/testing/browser/testingOutputPeek.ts +++ b/src/vs/workbench/contrib/testing/browser/testingOutputPeek.ts @@ -5,19 +5,24 @@ import * as dom from 'vs/base/browser/dom'; import { Color } from 'vs/base/common/color'; -import { DisposableStore } from 'vs/base/common/lifecycle'; +import { DisposableStore, IDisposable, IReference } from 'vs/base/common/lifecycle'; +import { clamp } from 'vs/base/common/numbers'; +import { count } from 'vs/base/common/strings'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { EmbeddedDiffEditorWidget } from 'vs/editor/browser/widget/embeddedCodeEditorWidget'; import { IDiffEditorOptions } from 'vs/editor/common/config/editorOptions'; import { IEditorContribution } from 'vs/editor/common/editorCommon'; -import { TextModel } from 'vs/editor/common/model/textModel'; -import { IPeekViewService, peekViewBorder, peekViewTitleBackground, peekViewTitleForeground, peekViewTitleInfoForeground, PeekViewWidget } from 'vs/editor/contrib/peekView/peekView'; +import { IResolvedTextEditorModel, ITextModelService } from 'vs/editor/common/services/resolverService'; +import { IPeekViewService, peekViewTitleBackground, peekViewTitleForeground, peekViewTitleInfoForeground, PeekViewWidget } from 'vs/editor/contrib/peekView/peekView'; +import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IColorTheme, IThemeService } from 'vs/platform/theme/common/themeService'; -import { IUndoRedoService } from 'vs/platform/undoRedo/common/undoRedo'; import { EditorModel } from 'vs/workbench/common/editor'; +import { testingPeekBorder } from 'vs/workbench/contrib/testing/browser/theme'; import { Testing } from 'vs/workbench/contrib/testing/common/constants'; -import { ITestMessage } from 'vs/workbench/contrib/testing/common/testCollection'; +import { InternalTestItem, ITestMessage } from 'vs/workbench/contrib/testing/common/testCollection'; +import { TestingContextKeys } from 'vs/workbench/contrib/testing/common/testingContextKeys'; +import { buildTestUri, TestUriType } from 'vs/workbench/contrib/testing/common/testingUri'; export class TestingOutputPeekController implements IEditorContribution { /** @@ -32,26 +37,51 @@ export class TestingOutputPeekController implements IEditorContribution { */ private peek?: TestingOutputPeek; - constructor(private readonly editor: ICodeEditor, @IInstantiationService private readonly instantiationService: IInstantiationService) { } + /** + * Context key updated when the peek is visible/hidden. + */ + private readonly visible: IContextKey; + constructor( + private readonly editor: ICodeEditor, + @IInstantiationService private readonly instantiationService: IInstantiationService, + @IContextKeyService contextKeyService: IContextKeyService, + ) { + this.visible = TestingContextKeys.peekVisible.bindTo(contextKeyService); + } + + /** + * @inheritdoc + */ public dispose(): void { - // no-op + this.removePeek(); } /** * Shows a peek for the message in th editor. */ - public show(output: ITestMessage) { - this.removePeek(); - if (!output.location) { + public async show(test: InternalTestItem, messageIndex: number) { + const message = test?.item.state.messages[messageIndex]; + if (!test || !message?.location) { return; } - this.peek = this.instantiationService.createInstance(TestingOutputPeek, this.editor, output); - this.peek.show(output.location.range, 18); + if (!this.peek) { + this.peek = this.instantiationService.createInstance(TestingOutputPeek, this.editor); + } + + this.visible.set(true); + this.peek.setModel(test, messageIndex); + this.peek.onDidClose(() => { + this.visible.set(false); + this.peek = undefined; + }); } - private removePeek() { + /** + * Disposes the peek view, if any. + */ + public removePeek() { if (this.peek) { this.peek.dispose(); this.peek = undefined; @@ -62,29 +92,26 @@ export class TestingOutputPeekController implements IEditorContribution { export class TestingOutputPeek extends PeekViewWidget { private readonly disposable = new DisposableStore(); private diff?: EmbeddedDiffEditorWidget; + private model?: IDisposable; private dimension?: dom.Dimension; constructor( editor: ICodeEditor, - private readonly message: ITestMessage, @IThemeService themeService: IThemeService, @IPeekViewService peekViewService: IPeekViewService, @IInstantiationService protected readonly instantiationService: IInstantiationService, - @IUndoRedoService private readonly undoRedo: IUndoRedoService, + @ITextModelService private readonly modelService: ITextModelService, ) { super(editor, { showFrame: false, showArrow: true, isResizeable: true, isAccessible: true }, instantiationService); this._disposables.add(themeService.onDidColorThemeChange(this.applyTheme, this)); this.applyTheme(themeService.getColorTheme()); - peekViewService.addExclusiveWidget(editor, this); - this.create(); - this.setTitle(message.message.toString()); } private applyTheme(theme: IColorTheme) { - const borderColor = theme.getColor(peekViewBorder) || Color.transparent; + const borderColor = theme.getColor(testingPeekBorder) || Color.transparent; this.style({ arrowColor: borderColor, frameColor: borderColor, @@ -99,6 +126,7 @@ export class TestingOutputPeek extends PeekViewWidget { */ public dispose() { super.dispose(); + this.model?.dispose(); this.disposable.dispose(); } @@ -117,7 +145,6 @@ export class TestingOutputPeek extends PeekViewWidget { horizontalHasArrows: false, alwaysConsumeMouseWheel: false }, - overviewRulerLanes: 2, fixedOverflowWidgets: true, readOnly: true, minimap: { @@ -133,18 +160,31 @@ export class TestingOutputPeek extends PeekViewWidget { const preview = this.diff = this.instantiationService.createInstance(EmbeddedDiffEditorWidget, diffContainer, options, this.editor); this.disposable.add(preview); - // todo: we probably want to have uri schemes for these guys since this - // does not work very well right now - preview.setModel(new SimpleDiffEditorModel( - new TextModel(this.message.expectedOutput!, TextModel.DEFAULT_CREATION_OPTIONS, null, null, this.undoRedo), - new TextModel(this.message.actualOutput!, TextModel.DEFAULT_CREATION_OPTIONS, null, null, this.undoRedo) - )); - if (this.dimension) { preview.layout(this.dimension); } } + public async setModel(test: InternalTestItem, messageIndex: number) { + const message = test.item.state.messages[messageIndex]; + if (!message?.location) { + return; + } + + this.show(message.location.range, hintPeekHeight(message)); + + if (this.model) { + this.model.dispose(); + } + + this.setTitle(message.message.toString(), test.item.label); + if (message.actualOutput !== undefined && message.expectedOutput !== undefined) { + await this.showDiffInEditor(test, messageIndex); + } else { + await this.showMessageInEditor(test, messageIndex); + } + } + /** * @override */ @@ -153,12 +193,42 @@ export class TestingOutputPeek extends PeekViewWidget { this.dimension = new dom.Dimension(width, height); this.diff?.layout(this.dimension); } + + private async showMessageInEditor(test: InternalTestItem, messageIndex: number) { + // todo? not sure if this is a useful experience + this.model?.dispose(); + this.diff?.setModel(null); + } + + private async showDiffInEditor(test: InternalTestItem, messageIndex: number) { + const uriParts = { messageIndex, testId: test.id, providerId: test.providerId }; + const [original, modified] = await Promise.all([ + this.modelService.createModelReference(buildTestUri({ ...uriParts, type: TestUriType.ExpectedOutput })), + this.modelService.createModelReference(buildTestUri({ ...uriParts, type: TestUriType.ActualOutput })), + ]); + + this.model?.dispose(); + const model = this.model = new SimpleDiffEditorModel(original, modified); + if (!this.diff) { + model.dispose(); + } else { + this.diff.setModel(model); + } + } } +const hintPeekHeight = (message: ITestMessage) => { + const lines = Math.max(count(message.actualOutput || '', '\n'), count(message.expectedOutput || '', '\n')); + return clamp(lines, 5, 20); +}; + class SimpleDiffEditorModel extends EditorModel { + public readonly original = this._original.object.textEditorModel; + public readonly modified = this._modified.object.textEditorModel; + constructor( - readonly original: TextModel, - readonly modified: TextModel, + private readonly _original: IReference, + private readonly _modified: IReference, ) { super(); } @@ -166,4 +236,10 @@ class SimpleDiffEditorModel extends EditorModel { async load(): Promise { return this; } + + public dispose() { + super.dispose(); + this._original.dispose(); + this._modified.dispose(); + } } diff --git a/src/vs/workbench/contrib/testing/browser/theme.ts b/src/vs/workbench/contrib/testing/browser/theme.ts index 39b3af41800..be142f697d9 100644 --- a/src/vs/workbench/contrib/testing/browser/theme.ts +++ b/src/vs/workbench/contrib/testing/browser/theme.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { localize } from 'vs/nls'; -import { registerColor } from 'vs/platform/theme/common/colorRegistry'; +import { editorErrorForeground, registerColor } from 'vs/platform/theme/common/colorRegistry'; import { TestRunState } from 'vs/workbench/api/common/extHostTypes'; export const testingColorIconFailed = registerColor('testing.iconFailed', { @@ -44,6 +44,12 @@ export const testingColorIconSkipped = registerColor('testing.iconSkipped', { hc: '#848484' }, localize('testing.iconSkipped', "Color for the 'Skipped' icon in the test explorer.")); +export const testingPeekBorder = registerColor('testing.peekBorder', { + dark: editorErrorForeground, + light: editorErrorForeground, + hc: editorErrorForeground, +}, localize('testing.peekBorder', 'Color of the peek view borders and arrow.')); + export const testStatesToIconColors: { [K in TestRunState]?: string } = { [TestRunState.Errored]: testingColorIconErrored, [TestRunState.Failed]: testingColorIconFailed, diff --git a/src/vs/workbench/contrib/testing/common/testService.ts b/src/vs/workbench/contrib/testing/common/testService.ts index b29765b0fbe..5bf87dd417c 100644 --- a/src/vs/workbench/contrib/testing/common/testService.ts +++ b/src/vs/workbench/contrib/testing/common/testService.ts @@ -9,11 +9,12 @@ import { IDisposable } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { ExtHostTestingResource } from 'vs/workbench/api/common/extHost.protocol'; -import { RunTestForProviderRequest, RunTestsRequest, RunTestsResult, TestsDiff } from 'vs/workbench/contrib/testing/common/testCollection'; +import { InternalTestItem, RunTestForProviderRequest, RunTestsRequest, RunTestsResult, TestIdWithProvider, TestsDiff } from 'vs/workbench/contrib/testing/common/testCollection'; export const ITestService = createDecorator('testService'); export interface MainTestController { + lookupTest(test: TestIdWithProvider): Promise; runTests(request: RunTestForProviderRequest, token: CancellationToken): Promise; } @@ -21,25 +22,25 @@ export type TestDiffListener = (diff: TestsDiff) => void; export interface ITestService { readonly _serviceBrand: undefined; - readonly onShouldSubscribe: Event<{ resource: ExtHostTestingResource, uri: URI }>; - readonly onShouldUnsubscribe: Event<{ resource: ExtHostTestingResource, uri: URI }>; - readonly onDidChangeProviders: Event<{ delta: number }>; + readonly onShouldSubscribe: Event<{ resource: ExtHostTestingResource, uri: URI; }>; + readonly onShouldUnsubscribe: Event<{ resource: ExtHostTestingResource, uri: URI; }>; + readonly onDidChangeProviders: Event<{ delta: number; }>; readonly providers: number; - readonly subscriptions: ReadonlyArray<{ resource: ExtHostTestingResource, uri: URI }>; + readonly subscriptions: ReadonlyArray<{ resource: ExtHostTestingResource, uri: URI; }>; readonly testRuns: Iterable; readonly onTestRunStarted: Event; - readonly onTestRunCompleted: Event<{ req: RunTestsRequest, result: RunTestsResult }>; + readonly onTestRunCompleted: Event<{ req: RunTestsRequest, result: RunTestsResult; }>; /** * List of resources where tests are actively being discovered. */ - readonly busyTestLocations: Iterable<{ resource: ExtHostTestingResource, uri: URI }>; + readonly busyTestLocations: Iterable<{ resource: ExtHostTestingResource, uri: URI; }>; /** * Fires when the busy state of a resource changes. */ - readonly onBusyStateChange: Event<{ resource: ExtHostTestingResource, uri: URI, busy: boolean }>; + readonly onBusyStateChange: Event<{ resource: ExtHostTestingResource, uri: URI, busy: boolean; }>; registerTestController(id: string, controller: MainTestController): void; unregisterTestController(id: string): void; @@ -53,6 +54,11 @@ export interface ITestService { */ updateDiscoveringCount(resource: ExtHostTestingResource, uri: URI, delta: number): void; + /** + * Looks up a test, by a request to extension hosts. + */ + lookupTest(test: TestIdWithProvider): Promise; + /** * Requests to resubscribe to all active subscriptions, discarding old tests. */ diff --git a/src/vs/workbench/contrib/testing/common/testServiceImpl.ts b/src/vs/workbench/contrib/testing/common/testServiceImpl.ts index 4bfa4197e07..3f664481dff 100644 --- a/src/vs/workbench/contrib/testing/common/testServiceImpl.ts +++ b/src/vs/workbench/contrib/testing/common/testServiceImpl.ts @@ -14,7 +14,7 @@ import { localize } from 'vs/nls'; import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { INotificationService } from 'vs/platform/notification/common/notification'; import { ExtHostTestingResource } from 'vs/workbench/api/common/extHost.protocol'; -import { AbstractIncrementalTestCollection, collectTestResults, EMPTY_TEST_RESULT, getTestSubscriptionKey, IncrementalTestCollectionItem, InternalTestItem, RunTestsRequest, RunTestsResult, TestDiffOpType, TestsDiff } from 'vs/workbench/contrib/testing/common/testCollection'; +import { AbstractIncrementalTestCollection, collectTestResults, EMPTY_TEST_RESULT, getTestSubscriptionKey, IncrementalTestCollectionItem, InternalTestItem, RunTestsRequest, RunTestsResult, TestDiffOpType, TestIdWithProvider, TestsDiff } from 'vs/workbench/contrib/testing/common/testCollection'; import { TestingContextKeys } from 'vs/workbench/contrib/testing/common/testingContextKeys'; import { ITestService, MainTestController, TestDiffListener } from 'vs/workbench/contrib/testing/common/testService'; @@ -105,6 +105,13 @@ export class TestService extends Disposable implements ITestService { return Iterable.map(Iterable.filter(this.testSubscriptions.values(), s => s.stillDiscovering > 0), s => s.ident); } + /** + * @inheritdoc + */ + public async lookupTest(test: TestIdWithProvider) { + return this.testControllers.get(test.providerId)?.lookupTest(test); + } + /** * @inheritdoc diff --git a/src/vs/workbench/contrib/testing/browser/testingCollectionService.ts b/src/vs/workbench/contrib/testing/common/testingCollectionService.ts similarity index 91% rename from src/vs/workbench/contrib/testing/browser/testingCollectionService.ts rename to src/vs/workbench/contrib/testing/common/testingCollectionService.ts index 31fcd27c0ec..434bb22dd6c 100644 --- a/src/vs/workbench/contrib/testing/browser/testingCollectionService.ts +++ b/src/vs/workbench/contrib/testing/common/testingCollectionService.ts @@ -40,7 +40,7 @@ export class TestSubscriptionListener extends Disposable { return this.subscription.workspaceFolderCollections; } - constructor(private readonly subscription: TestSubscription, public readonly onDispose: () => void) { + constructor(public readonly subscription: TestSubscription, public readonly onDispose: () => void) { super(); this._register(toDisposable(onDispose)); } @@ -66,6 +66,12 @@ export interface ITestingCollectionService { */ readonly count: number; + /** + * Gets a test by ID if it exists in the collection. This is *not* guarenteed + * and will only include workspace tests. + */ + getTestById(id: string): ITestSubscriptionItem | undefined; + /** * Gets all workspace folders we're listening to. */ @@ -97,6 +103,25 @@ export class TestingCollectionService implements ITestingCollectionService { return this.subscription?.testCount || 0; } + /** + * @inheritdoc + */ + public getTestById(id: string) { + const collections = this.subscription?.workspaceFolderCollections; + if (!collections) { + return undefined; + } + + for (const [, collection] of collections) { + const test = collection.getNodeById(id); + if (test) { + return test; + } + } + + return undefined; + } + /** * @inheritdoc */ diff --git a/src/vs/workbench/contrib/testing/common/testingContentProvider.ts b/src/vs/workbench/contrib/testing/common/testingContentProvider.ts new file mode 100644 index 00000000000..a07c8ead2b8 --- /dev/null +++ b/src/vs/workbench/contrib/testing/common/testingContentProvider.ts @@ -0,0 +1,64 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { URI } from 'vs/base/common/uri'; +import { ITextModel } from 'vs/editor/common/model'; +import { IModelService } from 'vs/editor/common/services/modelService'; +import { ITextModelContentProvider, ITextModelService } from 'vs/editor/common/services/resolverService'; +import { IWorkbenchContribution } from 'vs/workbench/common/contributions'; +import { ITestingCollectionService } from 'vs/workbench/contrib/testing/common/testingCollectionService'; +import { parseTestUri, TestUriType, TEST_DATA_SCHEME } from 'vs/workbench/contrib/testing/common/testingUri'; +import { ITestService } from 'vs/workbench/contrib/testing/common/testService'; + +export class TestingContentProvider implements IWorkbenchContribution, ITextModelContentProvider { + constructor( + @ITextModelService textModelResolverService: ITextModelService, + @IModelService private readonly modelService: IModelService, + @ITestingCollectionService private readonly collection: ITestingCollectionService, + @ITestService private readonly testService: ITestService, + ) { + textModelResolverService.registerTextModelContentProvider(TEST_DATA_SCHEME, this); + } + + /** + * @inheritdoc + */ + public async provideTextContent(resource: URI): Promise { + const existing = this.modelService.getModel(resource); + if (existing && !existing.isDisposed()) { + return existing; + } + + const parsed = parseTestUri(resource); + if (!parsed) { + return null; + } + + const test = this.collection.getTestById(resource.authority) || + await this.testService.lookupTest({ providerId: parsed.providerId, testId: parsed.testId }); + if (!test) { + return null; + } + + let text: string | undefined; + switch (parsed.type) { + case TestUriType.ActualOutput: + text = test.item.state.messages[parsed.messageIndex]?.actualOutput; + break; + case TestUriType.ExpectedOutput: + text = test.item.state.messages[parsed.messageIndex]?.expectedOutput; + break; + case TestUriType.Message: + text = test.item.state.messages[parsed.messageIndex]?.message.toString(); + break; + } + + if (text === undefined) { + return null; + } + + return this.modelService.createModel(text, null, resource, true); + } +} diff --git a/src/vs/workbench/contrib/testing/common/testingContextKeys.ts b/src/vs/workbench/contrib/testing/common/testingContextKeys.ts index dd7ac117d6e..f46626e41a0 100644 --- a/src/vs/workbench/contrib/testing/common/testingContextKeys.ts +++ b/src/vs/workbench/contrib/testing/common/testingContextKeys.ts @@ -11,4 +11,5 @@ export namespace TestingContextKeys { export const viewMode = new RawContextKey('testExplorerViewMode', TestExplorerViewMode.List); export const viewGrouping = new RawContextKey('testExplorerViewGrouping', TestExplorerViewGrouping.ByLocation); export const isRunning = new RawContextKey('testIsrunning', false); + export const peekVisible = new RawContextKey('testPeekVisible', false); } diff --git a/src/vs/workbench/contrib/testing/common/testingUri.ts b/src/vs/workbench/contrib/testing/common/testingUri.ts new file mode 100644 index 00000000000..dac64916d03 --- /dev/null +++ b/src/vs/workbench/contrib/testing/common/testingUri.ts @@ -0,0 +1,77 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { URI } from 'vs/base/common/uri'; + +export const TEST_DATA_SCHEME = 'vscode-test-data'; + +export const enum TestUriType { + Message, + ActualOutput, + ExpectedOutput, +} + +interface IGenericTestUri { + providerId: string; + testId: string; +} + +interface ITestMessageReference extends IGenericTestUri { + type: TestUriType.Message; + messageIndex: number; +} + +interface ITestOutputReference extends IGenericTestUri { + type: TestUriType.ActualOutput | TestUriType.ExpectedOutput; + messageIndex: number; +} + +export type ParsedTestUri = ITestMessageReference | ITestOutputReference; + +const enum TestUriParts { + Messages = 'message', + Text = 'text', + ActualOutput = 'actualOutput', + ExpectedOutput = 'expectedOutput', +} + +export const parseTestUri = (uri: URI): ParsedTestUri | undefined => { + const providerId = uri.authority; + const [testId, ...request] = uri.path.slice(1).split('/'); + + if (request[0] === TestUriParts.Messages) { + const index = Number(request[1]); + const part = request[2]; + switch (part) { + case TestUriParts.Text: + return { providerId, testId, messageIndex: index, type: TestUriType.Message }; + case TestUriParts.ActualOutput: + return { providerId, testId, messageIndex: index, type: TestUriType.ActualOutput }; + case TestUriParts.ExpectedOutput: + return { providerId, testId, messageIndex: index, type: TestUriType.ExpectedOutput }; + default: + return undefined; + } + } + + return undefined; +}; + +export const buildTestUri = (parsed: ParsedTestUri): URI => { + const uriParts = { scheme: TEST_DATA_SCHEME, authority: parsed.testId }; + const msgRef = (index: number, ...remaining: string[]) => + URI.from({ ...uriParts, path: ['', parsed.testId, TestUriParts.Messages, index, ...remaining].join('/') }); + + switch (parsed.type) { + case TestUriType.ActualOutput: + return msgRef(parsed.messageIndex, TestUriParts.ActualOutput); + case TestUriType.ExpectedOutput: + return msgRef(parsed.messageIndex, TestUriParts.ExpectedOutput); + case TestUriType.Message: + return msgRef(parsed.messageIndex, TestUriParts.Text); + default: + throw new Error('Invalid test uri'); + } +};