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

Fixes microsoft#214011

Author: connor4312

src/vs/base/browser/ui/list/listView.ts

@@ -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()
 		}
 	}
 

src/vs/editor/contrib/zoneWidget/browser/zoneWidget.ts

@@ -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) {

src/vs/workbench/contrib/debug/browser/callStackWidget.ts

@@ -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,

src/vs/workbench/contrib/testing/browser/testResultsView/testMessageStack.ts

@@ -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,19 +27,19 @@ 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';
 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) {

src/vs/workbench/contrib/testing/browser/testResultsView/testResultsViewContent.ts

@@ -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();