Skip to content
2 changes: 1 addition & 1 deletion extension/src/server/interactionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ export class InteractionService implements IInteractionService {
constructor(getAspireDebugSession: () => AspireDebugSession | null, rpcClient: ICliRpcClient, private readonly _globalState?: vscode.Memento) {
this._getAspireDebugSession = getAspireDebugSession;
this._rpcClient = rpcClient;
this._progressNotifier = new ProgressNotifier(this._rpcClient);
this._progressNotifier = new ProgressNotifier();
}

showStatus(statusText: string | null) {
Expand Down
50 changes: 20 additions & 30 deletions extension/src/server/progressNotifier.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import * as vscode from 'vscode';
import { ICliRpcClient } from './rpcClient';
import { formatText } from '../utils/strings';
import { collapseWhitespace, escapeCodicons, formatText } from '../utils/strings';
import { extensionLogOutputChannel } from '../utils/logging';

export class ProgressNotifier {
private _rpcClient?: ICliRpcClient;
private _currentProgress?: {
resolve: () => void;
updateMessage: (msg: string) => void;
Expand All @@ -18,18 +16,14 @@ export class ProgressNotifier {
return !!this._currentProgress || !!this._pendingClearTimeout;
}

constructor(rpcClient?: ICliRpcClient) {
this._rpcClient = rpcClient;
}

show(statusText: string | null) {
extensionLogOutputChannel.info(`Setting status/progress: ${statusText ?? 'null'}`);

if (!statusText) {
// If there is an active progress, wait a short period before
// actually clearing it. This allows callers to quickly call
// show(null) followed by show(non-null) within 250ms and have the
// existing notification updated instead of being torn down and recreated.
// existing progress updated instead of being torn down and recreated.
if (this._currentProgress) {
if (this._pendingClearTimeout) {
clearTimeout(this._pendingClearTimeout);
Expand All @@ -50,18 +44,17 @@ export class ProgressNotifier {
this._pendingClearTimeout = undefined;
}

// If a progress notification is already active, update its message
// If progress is already active, update its message
if (this._currentProgress) {
try {
this._currentProgress.updateMessage(formatText(statusText));
this._currentProgress.updateMessage(renderStatusMessage(statusText));
}
catch (err) {
extensionLogOutputChannel.error(`Failed to update progress message: ${err}`);
}
return;
}

// No active progress: create one that can be cancelled by the user
let resolveFn: () => void;
const waitPromise = new Promise<void>(resolve => { resolveFn = resolve; });

Expand All @@ -70,31 +63,20 @@ export class ProgressNotifier {
updateMessage: (_m: string) => {}
};

// `Window` rather than `Notification`: CLI status is reported for as long as the operation
// runs, and a progress notification cannot be dismissed while it is active, so it sits on
// top of the editor for the whole run (https://github.com/microsoft/aspire/issues/19036).
// Window progress renders in the status bar, which the user can ignore or hide.
vscode.window.withProgress({
location: vscode.ProgressLocation.Notification,
cancellable: true
}, async (progress, token) => {
location: vscode.ProgressLocation.Window
}, async progress => {
this._currentProgress!.updateMessage = (m: string) => progress.report({ message: m });

// Report the initial message as the progress message (no title)
progress.report({ message: formatText(statusText) });

const cancelListener = token.onCancellationRequested(() => {
extensionLogOutputChannel.info('User cancelled progress; attempting to stop CLI');
try {
this._rpcClient?.stopCli();
}
catch (err) {
extensionLogOutputChannel.error(`Failed to stop CLI: ${err}`);
}
});
progress.report({ message: renderStatusMessage(statusText) });

// Keep the progress alive until show(null) calls resolve
try {
return await waitPromise;
} finally {
return cancelListener.dispose();
}
return await waitPromise;
}).then(undefined, (err: any) => {
extensionLogOutputChannel.error(`Progress failed: ${err}`);
});
Expand All @@ -114,3 +96,11 @@ export class ProgressNotifier {
}
}
}

/**
* Makes CLI controlled status text safe to render as window progress: a single line, and without
* arbitrary codicons injected into the status bar.
*/
function renderStatusMessage(statusText: string): string {
return escapeCodicons(collapseWhitespace(formatText(statusText)));
}
10 changes: 9 additions & 1 deletion extension/src/test-e2e/appHostTree.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as assert from 'assert';
import { findRunningAppHost, getCommandInvocationCount, getResources, getTerminalCommandCount, getTreeAppHostLabel, isSamePath, waitForCommandOutcome, waitForDashboardUrl, waitForExtensionState, waitForNoDebugSessions, waitForNoRunningAppHost, waitForRepositoryIdle, waitForResource, waitForRunningAppHost, waitForTerminalCommand, waitForWorkspaceAppHost } from './helpers/assertions';
import { assertClipboardMatchesLastExpectationForE2E, captureWorkspaceAppHostPathClipboardExpectationForE2E, executeE2eControlCommand, getCliWrapperInvocationCount, getCliWrapperInvocations, restoreClipboardSnapshotForE2E, restoreE2eCliPathForE2E, restoreWorkspaceCliPath, runE2eTeardown, setCliUnavailableForE2E, setE2eCliPathForE2E, setTerminalCommandExecutionSuppressedForE2E, snapshotClipboardForE2E, stopAppHostIfRunning, stopPrimaryAppHostIfRunning, touchPrimaryAppHostProject, writeDelayedPsCliWrapper, writeGatedStreamingDiscoveryCliWrapper, writeStreamingDiscoveryCliWrapper, writeTrackedDelayedPsCliWrapper, writeTrackedStreamingDiscoveryCliWrapper } from './helpers/fixtures';
import { getPrimaryAppHostProjectPath } from './helpers/paths';
import { cancelActiveInput, clickTreeItem, executeCommandFromPalette, openAspireView, waitForChildTreeItem, waitForNotificationMessage, waitForTreeItem, waitForWorkbenchText } from './helpers/vscode';
import { cancelActiveInput, clickTreeItem, executeCommandFromPalette, getNotificationMessages, openAspireView, waitForChildTreeItem, waitForNotificationMessage, waitForTreeItem, waitForWorkbenchText } from './helpers/vscode';

suite('Aspire AppHost tree E2E', function () {
this.timeout(240000);
Expand Down Expand Up @@ -52,6 +52,14 @@ suite('Aspire AppHost tree E2E', function () {
const partialItem = await waitForTreeItem(partialSection, getTreeAppHostLabel(partialState.state));
assert.strictEqual(await partialItem.getLabel(), getTreeAppHostLabel(partialState.state));
await waitForWorkbenchText('Discovering AppHosts...');
// Discovery progress must render in the status bar. A progress notification stays on screen
// for as long as discovery runs and cannot be dismissed
// (https://github.com/microsoft/aspire/issues/19036).
const notificationMessages = await getNotificationMessages();
assert.deepStrictEqual(
notificationMessages.filter(message => message.includes('Discovering AppHosts')),
[],
`AppHost discovery progress must not use a notification. Notifications: ${JSON.stringify(notificationMessages)}`);

await waitForCommandOutcome('aspire-vscode.refreshAppHosts', 'success', 30000, invocationCountBefore);
const finalState = await waitForRepositoryIdle();
Expand Down
39 changes: 38 additions & 1 deletion extension/src/test-e2e/debugDashboard.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import * as path from 'path';
import { getCommandInvocationCount, getDebugLaunchCount, getStoppingPathEventCount, getTreeAppHostLabel, isSamePath, waitForAppHostLaunching, waitForCommandOutcome, waitForDebugConsoleOutput, waitForDebugDashboardUrl, waitForDebugLaunch, waitForDebugSessionStartup, waitForExtensionState, waitForHttpText, waitForNoDebugSessions, waitForNoRunningAppHost, waitForRepositoryIdle, waitForRunningAppHost, waitForStoppingPathEvent, waitForWorkspaceAppHost } from './helpers/assertions';
import { executeE2eControlCommand, resetDashboardDefaultChangedNotificationForE2E, restoreWorkspaceCliPath, runE2eTeardown, setCliUnavailableForE2E, setShowStatusDelayForE2E, stopPrimaryAppHostIfRunning, writeFileWithRetry, writeWorkspaceSetting } from './helpers/fixtures';
import { getPrimaryAppHostProjectPath } from './helpers/paths';
import { openAspireView, waitForEditorTitle, waitForNotificationMessage, waitForTreeItem, waitForWorkbenchTextAfterIntegratedBrowserNavigation } from './helpers/vscode';
import { getNotificationMessages, openAspireView, waitForAnyWorkbenchText, waitForEditorTitle, waitForNotificationMessage, waitForTreeItem, waitForWorkbenchTextAfterIntegratedBrowserNavigation } from './helpers/vscode';

// Long-running statuses the Aspire CLI reports through the showStatus RPC during `aspire run`.
const cliRunStatusTexts = ['Building AppHost...', 'Connecting to AppHost...', 'Starting dashboard...'];

suite('Aspire debug dashboard E2E', function () {
this.timeout(240000);
Expand Down Expand Up @@ -189,6 +192,40 @@ suite('Aspire debug dashboard E2E', function () {
await waitForNoRunningAppHost(120000, appHostPath);
});

test('keeps long-running CLI run status out of notifications', async () => {
await openAspireView();
await waitForRepositoryIdle();
const discovered = await waitForWorkspaceAppHost();
const appHostPath = discovered.state.workspaceAppHostPath ?? getPrimaryAppHostProjectPath();

// Each showStatus RPC is held briefly so a single status stays on screen long enough for the
// poll below to observe it. Without the delay a fast build can move through every status
// between two polls.
await setShowStatusDelayForE2E(2500);
try {
const before = getCommandInvocationCount('aspire-vscode.debugAppHost');
await executeE2eControlCommand({ name: 'debugAppHost', appHostPath }, { waitFor: 'started' });
await waitForAnyWorkbenchText(cliRunStatusTexts, 120000);
Comment thread
adamint marked this conversation as resolved.
Outdated

// A progress notification cannot be dismissed while the operation runs, so CLI status
// stayed on top of the editor for the whole run
// (https://github.com/microsoft/aspire/issues/19036).
const notificationMessages = await getNotificationMessages();
assert.deepStrictEqual(
notificationMessages.filter(message => cliRunStatusTexts.some(status => message.includes(status))),
[],
`Expected CLI run status to never appear as a notification: ${JSON.stringify(notificationMessages)}`);
await waitForCommandOutcome('aspire-vscode.debugAppHost', 'success', 120000, before);
}
finally {
await setShowStatusDelayForE2E(undefined);
}

await stopPrimaryAppHostIfRunning();
await waitForNoDebugSessions(180000);
await waitForNoRunningAppHost(180000, appHostPath);
});

test('surfaces AppHost build failure logs in the debug console when the CLI exits after a build failure', async function () {
if (process.env.ASPIRE_EXTENSION_E2E_SKIP_CURRENT_CLI_REGRESSIONS === 'true') {
return;
Expand Down
20 changes: 20 additions & 0 deletions extension/src/test-e2e/helpers/vscode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,11 @@ export async function getNotificationCount(): Promise<number> {
return (await new Workbench().getNotifications()).length;
}

export async function getNotificationMessages(): Promise<string[]> {
const notifications = await new Workbench().getNotifications();
return await Promise.all(notifications.map(notification => notification.getMessage()));
}

export async function waitForNotificationCountGreaterThan(count: number, timeoutMs = 30000): Promise<void> {
await VSBrowser.instance.driver.wait(async () => {
const currentCount = await getNotificationCount();
Expand Down Expand Up @@ -307,6 +312,21 @@ export async function waitForWorkbenchText(expectedText: string, timeoutMs = 300
}
}

export async function waitForAnyWorkbenchText(expectedTexts: readonly string[], timeoutMs = 30000): Promise<string> {
let lastText = '';
const expectedTextDescription = expectedTexts.map(text => `'${text}'`).join(' or ');

try {
return await VSBrowser.instance.driver.wait(async () => {
lastText = await getWorkbenchAndWebviewText();
return expectedTexts.some(text => lastText.includes(text)) ? lastText : false;
}, timeoutMs, `Timed out waiting for workbench text containing ${expectedTextDescription}.`);
}
catch (error) {
throw withWaitDiagnostics(error, [`Last workbench/webview text (${lastText.length} chars):\n${truncateDiagnosticText(lastText)}`]);
}
}

export async function waitForWorkbenchTextAfterIntegratedBrowserNavigation(expectedText: string | readonly string[], timeoutMs = 120000): Promise<string> {
const expectedTexts = Array.isArray(expectedText) ? expectedText : [expectedText];
const expectedTextDescription = expectedTexts.map(text => `'${text}'`).join(' or ');
Expand Down
4 changes: 2 additions & 2 deletions extension/src/test/appHostDataRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3570,7 +3570,7 @@ suite('AppHostDataRepository', () => {
}
});

test('workspace discovery shows non-cancellable progress only while the workspace view is visible', async () => {
test('workspace discovery shows non-cancellable window progress only while the workspace view is visible', async () => {
const workspaceFolder = {
uri: vscode.Uri.file('/workspace'),
name: 'workspace',
Expand Down Expand Up @@ -3598,7 +3598,7 @@ suite('AppHostDataRepository', () => {
await waitForCondition(() => withProgressStub.calledOnce, 'workspace discovery progress did not appear');

assert.deepStrictEqual(withProgressStub.firstCall.args[0], {
location: vscode.ProgressLocation.Notification,
location: vscode.ProgressLocation.Window,
title: 'Discovering AppHosts...',
cancellable: false,
});
Expand Down
Loading
Loading