From f26db2a85460ef3927eb3642986198356c362ff7 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Fri, 6 Nov 2026 11:12:40 -0800 Subject: [PATCH] Dismiss VS Code E2E notifications without depending on hover (#21565) ## Description Fixes an intermittent failure in the VS Code extension tests, not a bug in Aspire itself. The tests close notifications to clear the screen. Previously, they moved the mouse over a notification and clicked its close button. If another notification appeared or expired, the notification could move away from the mouse and hide the button before the click. The test would then fail with `ElementNotInteractableError`, even though the feature being tested had worked. The tests now ask VS Code to clear all notifications through the E2E bridge, which runs VS Code's `notifications.clearAll` command, instead of clicking a notification. Cleanup keeps its bounded retry until no notifications remain. The Python debugger setup test checks its message and then uses the same cleanup. No feature assertions are weakened, and no new dependencies are required. Adds unit tests: one checks that cleanup goes through the bridge's clear-all action when a notification can't be clicked, and one checks that the bridge action runs `notifications.clearAll`. They don't recreate the browser timing race. Fixes #21742 Validation: CI for the current head is pending. The notification cleanup unit tests passed locally and fail against the old hover-based cleanup. The bridge dispatch test needs the VS Code test host, which couldn't be downloaded locally, so it relies on CI. ## Checklist - Is this feature complete? - [x] Yes. Ready to ship. - [ ] No. Follow-up changes expected. - Are you including unit tests for the changes and scenario tests if relevant? - [x] Yes - [ ] No - Did you add public API? - [ ] Yes - If yes, did you have an API Review for it? - [ ] Yes - [ ] No - Did you add `` and `` elements on your triple slash comments? - [ ] Yes - [ ] No - [x] No - Does the change make any security assumptions or guarantees? - [ ] Yes - If yes, have you done a threat model and had a security review? - [ ] Yes - [ ] No - [x] No --------- Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit af4527806f2e09424b40e72ab80895fffe82b02f) --- extension/src/test-e2e/edgeCases.e2e.test.ts | 6 +- extension/src/test-e2e/helpers/vscode.ts | 6 +- extension/src/test/e2eStateFileBridge.test.ts | 22 +++++ .../test/notificationWaitForMessage.test.ts | 86 +++++++++++++++++++ extension/src/testing/e2eStateFileBridge.ts | 4 + extension/src/types/extensionApi.ts | 1 + 6 files changed, 121 insertions(+), 4 deletions(-) diff --git a/extension/src/test-e2e/edgeCases.e2e.test.ts b/extension/src/test-e2e/edgeCases.e2e.test.ts index 2e177266de8..8a603e89f2c 100644 --- a/extension/src/test-e2e/edgeCases.e2e.test.ts +++ b/extension/src/test-e2e/edgeCases.e2e.test.ts @@ -6,7 +6,7 @@ import type { ExecutableLaunchConfiguration } from '../dcp/types'; import { getCommandInvocationCount, getDebugLaunchCount, isSamePath, waitForCommandOutcome, waitForDebugLaunch, waitForDebugSessionStartup, waitForExtensionState, waitForNoDebugSessions, waitForNoRunningAppHost, waitForRepositoryIdle, waitForRunningAppHost, waitForSelectedWorkspaceAppHost, waitForWorkspaceAppHost } from './helpers/assertions'; import { createEmptyAppHostProject, createExternalSingleFileAppHost, executeE2eControlCommand, getGeneratedAppHostPath, getGeneratedProjectRoot, isProcessAlive, removeExternalSingleFileAppHost, removeGeneratedProject, restoreWorkspaceAppHostConfig, restoreWorkspaceCliPath, runE2eTeardown, setCliUnavailableForE2E, setDebugLaunchSuppressedForE2E, stopAppHostIfRunning, stopPrimaryAppHostIfRunning, waitForKnownProcessExit, writeFileWithRetry, writeWorkspaceAppHostConfigForPath } from './helpers/fixtures'; import { getPrimaryAppHostProjectPath, getWorkspaceRoot } from './helpers/paths'; -import { chooseActiveQuickPick, executeCommandFromPalette, openAspireView, waitForEditorTitle, waitForNotificationMessage } from './helpers/vscode'; +import { chooseActiveQuickPick, dismissAllNotifications, executeCommandFromPalette, openAspireView, waitForEditorTitle, waitForNotificationMessage } from './helpers/vscode'; interface DebugSessionProcessInfo { appHostPath?: string; @@ -219,10 +219,10 @@ builder.Build().Run();`)); await executeE2eControlCommand({ name: 'runAppHost', appHostPath }, { waitFor: 'started' }); await waitForCommandOutcome('aspire-vscode.runAppHost', 'success', 180000, runBefore); - const notification = await waitForNotificationMessage( + await waitForNotificationMessage( 'Set up Python debugging support to debug resources in this app.', 60000); - await notification.dismiss(); + await dismissAllNotifications(); }); test('process-owner cleanup stops the owned CLI and AppHost process tree', async () => { diff --git a/extension/src/test-e2e/helpers/vscode.ts b/extension/src/test-e2e/helpers/vscode.ts index d41c45ba386..72261b4994a 100644 --- a/extension/src/test-e2e/helpers/vscode.ts +++ b/extension/src/test-e2e/helpers/vscode.ts @@ -1,3 +1,5 @@ +import type { AspireExtensionE2EControlCommand } from '../../types/extensionApi'; +import { applyE2eControl } from './assertions'; import { BottomBarPanel, By, EditorView, InputBox, ModalDialog, Notification, SideBarView, TreeItem, TreeSection, VSBrowser, WebView, Workbench } from './extester'; import { error as webDriverError } from 'selenium-webdriver'; @@ -666,7 +668,9 @@ export async function dismissAllNotifications(timeoutMs = 30000): Promise return true; } - await notifications[0].dismiss(); + // Clicking a toast's clear action needs the toast hovered, and toasts re-layout as others + // arrive or expire. VS Code's own command clears every notification without the DOM. + await applyE2eControl({ command: { name: 'clearAllNotifications' } satisfies AspireExtensionE2EControlCommand }); return false; } catch (error) { diff --git a/extension/src/test/e2eStateFileBridge.test.ts b/extension/src/test/e2eStateFileBridge.test.ts index 10e7ed5cb6f..200f2ee8e30 100644 --- a/extension/src/test/e2eStateFileBridge.test.ts +++ b/extension/src/test/e2eStateFileBridge.test.ts @@ -419,6 +419,28 @@ suite('E2E state file bridge', () => { provider.dispose(); }); + test('clears notifications with the VS Code clear-all command', async () => { + const repository = createRepository([]); + const terminalProvider = {} as AspireTerminalProvider; + const launchService = createLaunchService(); + const provider = {} as AspireAppHostTreeProvider; + const executeCommandStub = sandbox.stub(vscode.commands, 'executeCommand').resolves(); + const markStarted = sandbox.spy(); + + await dispatchControlCommand( + { name: 'clearAllNotifications' }, + repository, + launchService, + provider, + terminalProvider, + markStarted); + + assert.deepStrictEqual(executeCommandStub.getCalls().map(call => call.args), [ + ['notifications.clearAll'], + ]); + assert.strictEqual(markStarted.callCount, 1); + }); + test('scopes missing and exact C# extension version overrides to resource configuration creation', async () => { const repository = createRepository([]); const terminalProvider = {} as AspireTerminalProvider; diff --git a/extension/src/test/notificationWaitForMessage.test.ts b/extension/src/test/notificationWaitForMessage.test.ts index 45c3fb961d0..cdd573d4fd9 100644 --- a/extension/src/test/notificationWaitForMessage.test.ts +++ b/extension/src/test/notificationWaitForMessage.test.ts @@ -1,4 +1,6 @@ import * as assert from 'assert'; +import * as fs from 'fs'; +import * as os from 'os'; import * as path from 'path'; import { error as webDriverError } from 'selenium-webdriver'; @@ -21,6 +23,7 @@ interface NotificationExtesterStubModule { } interface VscodeHelpersModule { + dismissAllNotifications(timeoutMs?: number): Promise; waitForCodeLensText(fileName: string, expectedText: string, timeoutMs?: number): Promise; waitForEditorTitle(expectedText: string, timeoutMs?: number): Promise; waitForNotificationMessage(expectedText: string, timeoutMs?: number): Promise; @@ -32,6 +35,7 @@ const extesterStubModulePath = path.join(extensionRoot, 'out', 'test', 'fixtures const compiledE2eHelpersPath = path.join(extensionRoot, 'out', 'test-e2e', 'helpers'); const compiledExtesterModulePath = path.join(compiledE2eHelpersPath, 'extester.js'); const compiledVscodeHelpersModulePath = path.join(compiledE2eHelpersPath, 'vscode.js'); +const e2eBridgeEnvironmentVariables = ['ASPIRE_EXTENSION_E2E_CONTROL_FILE', 'ASPIRE_EXTENSION_E2E_STATE_FILE', 'ASPIRE_EXTENSION_E2E_RUN_ID']; suite('waitForNotificationMessage', () => { const originalExtesterModule = process.env.ASPIRE_EXTENSION_E2E_EXTESTER_MODULE; @@ -137,6 +141,35 @@ suite('waitForNotificationMessage', () => { ); }); + test('dismisses notifications through VS Code clear-all instead of hovering each toast', async () => { + const { stub, vscode } = loadNotificationWaitModules(); + const notification: NotificationLike = { + getMessage: async () => "Required option '--message' was not provided.", + // VS Code hides a toast's clear action once the toast moves out from under the pointer. + dismiss: async () => { + throw new webDriverError.ElementNotInteractableError('element not interactable'); + }, + }; + const bridge = startFakeE2eBridge(commandName => { + if (commandName === 'clearAllNotifications') { + stub.setNotificationPolls([]); + } + }); + + stub.setNotificationPolls([[notification], [notification], [notification]]); + + try { + await vscode.dismissAllNotifications(5000); + } + finally { + bridge.dispose(); + } + + assert.deepStrictEqual(bridge.commandNames, ['clearAllNotifications']); + // Clearing replaced the remaining polls, so a single poll after it found no notifications. + assert.strictEqual(stub.getNotificationWaitState().notificationPollCount, 1); + }); + test('waits for the tab to exist and for the provider to produce lenses', async () => { const { stub, vscode } = loadNotificationWaitModules(); @@ -184,6 +217,59 @@ function loadNotificationWaitModules(): { return { stub, vscode }; } +/** + * Answers E2E control commands the way the extension host bridge does: it reads each new control + * revision, reports it applied in the state file and records the command name. + */ +function startFakeE2eBridge(onCommand: (commandName: string) => void): { commandNames: string[]; dispose(): void } { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'aspire-e2e-bridge-')); + const controlFile = path.join(directory, 'control.json'); + const stateFile = path.join(directory, 'state.json'); + const originalEnvironment = new Map(e2eBridgeEnvironmentVariables.map(name => [name, process.env[name]])); + const commandNames: string[] = []; + let lastRevision = -1; + + process.env.ASPIRE_EXTENSION_E2E_CONTROL_FILE = controlFile; + process.env.ASPIRE_EXTENSION_E2E_STATE_FILE = stateFile; + delete process.env.ASPIRE_EXTENSION_E2E_RUN_ID; + fs.writeFileSync(stateFile, JSON.stringify({})); + + const interval = setInterval(() => { + if (!fs.existsSync(controlFile)) { + return; + } + + const payload = JSON.parse(fs.readFileSync(controlFile, 'utf8')) as { revision: number; command?: { name: string } }; + if (payload.revision <= lastRevision) { + return; + } + + lastRevision = payload.revision; + if (payload.command) { + commandNames.push(payload.command.name); + onCommand(payload.command.name); + } + + fs.writeFileSync(stateFile, JSON.stringify({ control: { revision: payload.revision, status: 'applied', startedObserved: true } })); + }, 10); + + return { + commandNames, + dispose: () => { + clearInterval(interval); + for (const [name, value] of originalEnvironment) { + if (value === undefined) { + delete process.env[name]; + } + else { + process.env[name] = value; + } + } + fs.rmSync(directory, { recursive: true, force: true }); + }, + }; +} + function resetLoadedNotificationModules(): void { for (const modulePath of [compiledVscodeHelpersModulePath, compiledExtesterModulePath, extesterStubModulePath]) { try { diff --git a/extension/src/testing/e2eStateFileBridge.ts b/extension/src/testing/e2eStateFileBridge.ts index 12bffaeac73..51bfc6b18a1 100644 --- a/extension/src/testing/e2eStateFileBridge.ts +++ b/extension/src/testing/e2eStateFileBridge.ts @@ -676,6 +676,10 @@ export async function executeE2eControlCommand( await vscode.commands.executeCommand('workbench.action.closeAllEditors'); return getActiveEditorInfo(); } + case 'clearAllNotifications': { + markStarted(); + return await vscode.commands.executeCommand('notifications.clearAll'); + } case 'getRegisteredAspireCommands': { markStarted(); const commands = await vscode.commands.getCommands(true); diff --git a/extension/src/types/extensionApi.ts b/extension/src/types/extensionApi.ts index 0ff986ae989..190c0a16825 100644 --- a/extension/src/types/extensionApi.ts +++ b/extension/src/types/extensionApi.ts @@ -235,6 +235,7 @@ export type AspireExtensionE2EControlCommand = | { name: 'startDebugging'; configurationName: string } | { name: 'stopDebugging' } | { name: 'closeAllEditors' } + | { name: 'clearAllNotifications' } | { name: 'getRegisteredAspireCommands' } | { name: 'getRegisteredLanguageModelTools' } | { name: 'prepareLanguageModelToolInvocation'; toolName: string; input: Record }