testing: fix errors peeks being oversized relative to their contents (#236763)

Fixes #214011
pull/236767/head
Connor Peet 2024-12-20 13:59:08 -08:00 committed by GitHub
parent 3751af286f
commit 0c51035590
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 85 additions and 95 deletions

View File

@ -567,6 +567,8 @@ export class ListView<T> implements IListView<T> {
if (this.supportDynamicHeights) {
this._rerender(this.lastRenderTop, this.lastRenderHeight);
} else {
this._onDidChangeContentHeight.fire(this.contentHeight); // otherwise fired in _rerender()
}
}

View File

@ -29,7 +29,6 @@ export interface IOptions {
frameColor?: Color | string;
arrowColor?: Color;
keepEditorSelection?: boolean;
allowUnlimitedHeight?: boolean;
ordinal?: number;
showInHiddenAreas?: boolean;
}
@ -376,6 +375,11 @@ export abstract class ZoneWidget implements IHorizontalSashLayoutProvider {
return result;
}
/** Gets the maximum widget height in lines. */
protected _getMaximumHeightInLines(): number | undefined {
return Math.max(12, (this.editor.getLayoutInfo().height / this.editor.getOption(EditorOption.lineHeight)) * 0.8);
}
private _showImpl(where: Range, heightInLines: number): void {
const position = where.getStartPosition();
const layoutInfo = this.editor.getLayoutInfo();
@ -389,8 +393,8 @@ export abstract class ZoneWidget implements IHorizontalSashLayoutProvider {
const lineHeight = this.editor.getOption(EditorOption.lineHeight);
// adjust heightInLines to viewport
if (!this.options.allowUnlimitedHeight) {
const maxHeightInLines = Math.max(12, (this.editor.getLayoutInfo().height / lineHeight) * 0.8);
const maxHeightInLines = this._getMaximumHeightInLines();
if (maxHeightInLines !== undefined) {
heightInLines = Math.min(heightInLines, maxHeightInLines);
}
@ -493,7 +497,9 @@ export abstract class ZoneWidget implements IHorizontalSashLayoutProvider {
// implement in subclass
}
protected _relayout(newHeightInLines: number): void {
protected _relayout(_newHeightInLines: number): void {
const maxHeightInLines = this._getMaximumHeightInLines();
const newHeightInLines = maxHeightInLines === undefined ? _newHeightInLines : Math.min(maxHeightInLines, _newHeightInLines);
if (this._viewZone && this._viewZone.heightInLines !== newHeightInLines) {
this.editor.changeViewZones(accessor => {
if (this._viewZone) {

View File

@ -119,10 +119,18 @@ export class CallStackWidget extends Disposable {
private readonly currentFramesDs = this._register(new DisposableStore());
private cts?: CancellationTokenSource;
public get onDidChangeContentHeight() {
return this.list.onDidChangeContentHeight;
}
public get onDidScroll() {
return this.list.onDidScroll;
}
public get contentHeight() {
return this.list.contentHeight;
}
constructor(
container: HTMLElement,
containingEditor: ICodeEditor | undefined,

View File

@ -1,49 +0,0 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { Disposable } from '../../../../../base/common/lifecycle.js';
import { ICodeEditor } from '../../../../../editor/browser/editorBrowser.js';
import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js';
import { AnyStackFrame, CallStackFrame, CallStackWidget } from '../../../debug/browser/callStackWidget.js';
import { ITestMessageStackFrame } from '../../common/testTypes.js';
export class TestResultStackWidget extends Disposable {
private readonly widget: CallStackWidget;
public get onDidScroll() {
return this.widget.onDidScroll;
}
constructor(
private readonly container: HTMLElement,
containingEditor: ICodeEditor | undefined,
@IInstantiationService instantiationService: IInstantiationService,
) {
super();
this.widget = this._register(instantiationService.createInstance(
CallStackWidget,
container,
containingEditor,
));
}
public collapseAll() {
this.widget.collapseAll();
}
public update(messageFrame: AnyStackFrame, stack: ITestMessageStackFrame[]) {
this.widget.setFrames([messageFrame, ...stack.map(frame => new CallStackFrame(
frame.label,
frame.uri,
frame.position?.lineNumber,
frame.position?.column,
))]);
}
public layout(height?: number, width?: number) {
this.widget.layout(height ?? this.container.clientHeight, width);
}
}

View File

@ -14,7 +14,6 @@ import { Emitter, Event, Relay } from '../../../../../base/common/event.js';
import { KeyCode } from '../../../../../base/common/keyCodes.js';
import { Disposable, DisposableStore, IDisposable, toDisposable } from '../../../../../base/common/lifecycle.js';
import { observableValue } from '../../../../../base/common/observable.js';
import './testResultsViewContent.css';
import { ICodeEditor } from '../../../../../editor/browser/editorBrowser.js';
import { ITextModelService } from '../../../../../editor/common/services/resolverService.js';
import { localize } from '../../../../../nls.js';
@ -28,12 +27,7 @@ import { IInstantiationService, ServicesAccessor } from '../../../../../platform
import { ServiceCollection } from '../../../../../platform/instantiation/common/serviceCollection.js';
import { IQuickInputService } from '../../../../../platform/quickinput/common/quickInput.js';
import { IUriIdentityService } from '../../../../../platform/uriIdentity/common/uriIdentity.js';
import { CustomStackFrame } from '../../../debug/browser/callStackWidget.js';
import * as icons from '../icons.js';
import { TestResultStackWidget } from './testMessageStack.js';
import { DiffContentProvider, IPeekOutputRenderer, MarkdownTestMessagePeek, PlainTextMessagePeek, TerminalMessagePeek } from './testResultsOutput.js';
import { equalsSubject, getSubjectTestItem, InspectSubject, MessageSubject, TaskSubject, TestOutputSubject } from './testResultsSubject.js';
import { OutputPeekTree } from './testResultsTree.js';
import { AnyStackFrame, CallStackFrame, CallStackWidget, CustomStackFrame } from '../../../debug/browser/callStackWidget.js';
import { TestCommandId } from '../../common/constants.js';
import { IObservableValue } from '../../common/observableValue.js';
import { capabilityContextKeys, ITestProfileService } from '../../common/testProfileService.js';
@ -41,6 +35,11 @@ import { LiveTestResult } from '../../common/testResult.js';
import { ITestFollowup, ITestService } from '../../common/testService.js';
import { ITestMessageStackFrame, TestRunProfileBitset } from '../../common/testTypes.js';
import { TestingContextKeys } from '../../common/testingContextKeys.js';
import * as icons from '../icons.js';
import { DiffContentProvider, IPeekOutputRenderer, MarkdownTestMessagePeek, PlainTextMessagePeek, TerminalMessagePeek } from './testResultsOutput.js';
import { equalsSubject, getSubjectTestItem, InspectSubject, MessageSubject, TaskSubject, TestOutputSubject } from './testResultsSubject.js';
import { OutputPeekTree } from './testResultsTree.js';
import './testResultsViewContent.css';
const enum SubView {
Diff = 0,
@ -183,7 +182,7 @@ export class TestResultsViewContent extends Disposable {
private contextKeyTestMessage!: IContextKey<string>;
private contextKeyResultOutdated!: IContextKey<boolean>;
private stackContainer!: HTMLElement;
private callStackWidget!: TestResultStackWidget;
private callStackWidget!: CallStackWidget;
private currentTopFrame?: MessageStackFrame;
private isDoingLayoutUpdate?: boolean;
@ -209,6 +208,14 @@ export class TestResultsViewContent extends Disposable {
};
}
public get onDidChangeContentHeight() {
return this.callStackWidget.onDidChangeContentHeight;
}
public get contentHeight() {
return this.callStackWidget?.contentHeight || 0;
}
constructor(
private readonly editor: ICodeEditor | undefined,
private readonly options: {
@ -233,7 +240,7 @@ export class TestResultsViewContent extends Disposable {
const messageContainer = this.messageContainer = dom.$('.test-output-peek-message-container');
this.stackContainer = dom.append(containerElement, dom.$('.test-output-call-stack-container'));
this.callStackWidget = this._register(this.instantiationService.createInstance(TestResultStackWidget, this.stackContainer, this.editor));
this.callStackWidget = this._register(this.instantiationService.createInstance(CallStackWidget, this.stackContainer, this.editor));
this.followupWidget = this._register(this.instantiationService.createInstance(FollowupActionWidget, this.editor));
this.onCloseEmitter.input = this.followupWidget.onClose;
@ -315,13 +322,22 @@ export class TestResultsViewContent extends Disposable {
this.currentSubjectStore.clear();
const callFrames = this.getCallFrames(opts.subject) || [];
const topFrame = await this.prepareTopFrame(opts.subject, callFrames);
this.callStackWidget.update(topFrame, callFrames);
this.setCallStackFrames(topFrame, callFrames);
this.followupWidget.show(opts.subject);
this.populateFloatingClick(opts.subject);
});
}
private setCallStackFrames(messageFrame: AnyStackFrame, stack: ITestMessageStackFrame[]) {
this.callStackWidget.setFrames([messageFrame, ...stack.map(frame => new CallStackFrame(
frame.label,
frame.uri,
frame.position?.lineNumber,
frame.position?.column,
))]);
}
/**
* Collapses all displayed stack frames.
*/
@ -375,7 +391,6 @@ export class TestResultsViewContent extends Disposable {
}));
}
if (provider.onDidContentSizeChange) {
this.currentSubjectStore.add(provider.onDidContentSizeChange(() => {
if (this.dimension && !this.isDoingLayoutUpdate) {

View File

@ -6,16 +6,16 @@
import * as dom from '../../../../base/browser/dom.js';
import { alert } from '../../../../base/browser/ui/aria/aria.js';
import { IAction } from '../../../../base/common/actions.js';
import { RunOnceScheduler } from '../../../../base/common/async.js';
import { Codicon } from '../../../../base/common/codicons.js';
import { Color } from '../../../../base/common/color.js';
import { Emitter, Event } from '../../../../base/common/event.js';
import { Event } from '../../../../base/common/event.js';
import { stripIcons } from '../../../../base/common/iconLabels.js';
import { Iterable } from '../../../../base/common/iterator.js';
import { KeyCode, KeyMod } from '../../../../base/common/keyCodes.js';
import { Lazy } from '../../../../base/common/lazy.js';
import { Disposable } from '../../../../base/common/lifecycle.js';
import { derived, disposableObservableValue, observableValue } from '../../../../base/common/observable.js';
import { count } from '../../../../base/common/strings.js';
import { URI } from '../../../../base/common/uri.js';
import { ICodeEditor, isCodeEditor } from '../../../../editor/browser/editorBrowser.js';
import { EditorAction2 } from '../../../../editor/browser/editorExtensions.js';
@ -678,10 +678,8 @@ export class TestingOutputPeekController extends Disposable implements IEditorCo
class TestResultsPeek extends PeekViewWidget {
private static lastHeightInLines?: number;
private readonly visibilityChange = this._disposables.add(new Emitter<boolean>());
public readonly current = observableValue<InspectSubject | undefined>('testPeekCurrent', undefined);
private resizeOnNextContentHeightUpdate = false;
private content!: TestResultsViewContent;
private scopedContextKeyService!: IContextKeyService;
private dimension?: dom.Dimension;
@ -701,10 +699,22 @@ class TestResultsPeek extends PeekViewWidget {
super(editor, { showFrame: true, frameWidth: 1, showArrow: true, isResizeable: true, isAccessible: true, className: 'test-output-peek' }, instantiationService);
this._disposables.add(themeService.onDidColorThemeChange(this.applyTheme, this));
this._disposables.add(this.onDidClose(() => this.visibilityChange.fire(false)));
peekViewService.addExclusiveWidget(editor, this);
}
protected override _getMaximumHeightInLines(): number | undefined {
const defaultMaxHeight = super._getMaximumHeightInLines();
const contentHeight = this.content?.contentHeight;
if (!contentHeight) { // undefined or 0
return defaultMaxHeight;
}
const lineHeight = this.editor.getOption(EditorOption.lineHeight);
// 41 is experimentally determined to be the overhead of the peek view itself
// to avoid showing scrollbars by default in its content.
return Math.min(defaultMaxHeight || Infinity, (contentHeight + 41) / lineHeight);
}
private applyTheme() {
const theme = this.themeService.getColorTheme();
const current = this.current.get();
@ -764,6 +774,27 @@ class TestResultsPeek extends PeekViewWidget {
protected override _fillBody(containerElement: HTMLElement): void {
this.content.fillBody(containerElement);
// Resize on height updates for a short time to allow any heights made
// by editor contributions to come into effect before.
const contentHeightSettleTimer = this._disposables.add(new RunOnceScheduler(() => {
this.resizeOnNextContentHeightUpdate = false;
}, 500));
this._disposables.add(this.content.onDidChangeContentHeight(height => {
if (!this.resizeOnNextContentHeightUpdate || !height) {
return;
}
const displayed = this._getMaximumHeightInLines();
if (displayed) {
this._relayout(Math.min(displayed, this.getVisibleEditorLines() / 2));
if (!contentHeightSettleTimer.isScheduled()) {
contentHeightSettleTimer.schedule();
}
}
}));
this._disposables.add(this.content.onDidRequestReveal(sub => {
TestingOutputPeekController.get(this.editor)?.show(sub instanceof MessageSubject
? sub.messageUri
@ -780,7 +811,6 @@ class TestResultsPeek extends PeekViewWidget {
return this.showInPlace(subject);
}
const message = subject.message;
const previous = this.current;
const revealLocation = subject.revealLocation?.range.getStartPosition();
if (!revealLocation && !previous) {
@ -792,13 +822,8 @@ class TestResultsPeek extends PeekViewWidget {
return this.showInPlace(subject);
}
// If there is a stack we want to display, ensure the default size is large-ish
const peekLines = TestResultsPeek.lastHeightInLines || Math.max(
inspectSubjectHasStack(subject) ? Math.ceil(this.getVisibleEditorLines() / 2) : 0,
hintMessagePeekHeight(message)
);
this.show(revealLocation, peekLines);
this.resizeOnNextContentHeightUpdate = true;
this.show(revealLocation, 10); // 10 is just a random number, we resize once content is available
this.editor.revealRangeNearTopIfOutsideViewport(Range.fromPositions(revealLocation), ScrollType.Smooth);
return this.showInPlace(subject);
@ -832,11 +857,6 @@ class TestResultsPeek extends PeekViewWidget {
await this.content.reveal({ subject, preserveFocus: false });
}
protected override _relayout(newHeightInLines: number): void {
super._relayout(newHeightInLines);
TestResultsPeek.lastHeightInLines = newHeightInLines;
}
/** @override */
protected override _doLayoutBody(height: number, width: number) {
super._doLayoutBody(height, width);
@ -919,23 +939,11 @@ export class TestResultsView extends ViewPane {
}
}
const hintMessagePeekHeight = (msg: ITestMessage) => {
const msgHeight = ITestMessage.isDiffable(msg)
? Math.max(hintPeekStrHeight(msg.actual), hintPeekStrHeight(msg.expected))
: hintPeekStrHeight(typeof msg.message === 'string' ? msg.message : msg.message.value);
// add 8ish lines for the size of the title and decorations in the peek.
return msgHeight + 8;
};
const firstLine = (str: string) => {
const index = str.indexOf('\n');
return index === -1 ? str : str.slice(0, index);
};
const hintPeekStrHeight = (str: string) => Math.min(count(str, '\n'), 24);
function getOuterEditorFromDiffEditor(codeEditorService: ICodeEditorService): ICodeEditor | null {
const diffEditors = codeEditorService.listDiffEditors();