From 78211f21d24fcf017effd4d8fc0e9d5d66921781 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Thu, 20 Aug 2026 15:27:45 -0700 Subject: [PATCH 1/4] Default to an installed build, and report missing video tooling Three problems surfaced running the skill from a fresh checkout. Nothing ran without a target. With no flags the runner used the build from the checkout, which only exists after compiling the product, so the documented starting point failed to launch. Reproducing a reported issue means running the shipped product anyway, so with no target flag it now finds an installed VS Code Insiders (falling back to Stable) and logs which one it chose. `--dev` selects the checkout build, and `--build` still pins an exact install. A missing ffmpeg was only discovered after the run, as a raw ENOENT, and it threw out of `runScenario` after the report had been written. The runner now checks for ffmpeg and ffprobe before launching anything and prints the install command for the platform, and caption rendering can no longer fail a run that has already produced its evidence. The window did not fill the recording. The canvas is 1920x1080 while VS Code sizes its own window (1440x900 with a workspace, 1200x800 empty), so the capture showed the window in the top-left corner surrounded by dead space. The window is now sized to the canvas once recording is on; a window larger than the display still renders at that size, so this holds on smaller screens. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb443eb-11e5-40a1-8608-7f593fa79485 --- .github/skills/validate-ui-scenario/SKILL.md | 57 ++++++++++++-------- test/automation/src/playwrightElectron.ts | 20 ++++++- test/scenario/src/application.ts | 53 +++++++++++++++++- test/scenario/src/options.ts | 4 +- test/scenario/src/renderEvidenceChapters.ts | 22 ++++++-- test/scenario/src/runScenario.ts | 38 ++++++++++++- 6 files changed, 163 insertions(+), 31 deletions(-) diff --git a/.github/skills/validate-ui-scenario/SKILL.md b/.github/skills/validate-ui-scenario/SKILL.md index 16b002502f7d3e..867f0ed40e4c0a 100644 --- a/.github/skills/validate-ui-scenario/SKILL.md +++ b/.github/skills/validate-ui-scenario/SKILL.md @@ -18,21 +18,34 @@ step boundary, writes the report, and captions the recording with each step and ## Prepare ```bash -npm install # once +npm install # once npm --prefix test/scenario run compile # after any change under test/scenario ``` -Add `ffmpeg` and `ffprobe` to `PATH` to get the caption band on the video. Without them the run still -succeeds and the raw recording is kept. - -| Target | Extra flags | Also required | Use for | -|--------|-------------|---------------|---------| -| Installed Insiders | `--build ` | nothing | Reproducing a report against shipped behavior | -| Dev build from this checkout | *(none)* | `npm run electron`, `npm run transpile-client` | Verifying an unmerged change | +**Check `ffmpeg` and `ffprobe` are on `PATH` before running.** Without them the scenario still runs +and keeps the raw recording, but the video is not captioned with step titles. The runner warns at +startup; if they are missing, tell the user how to install them rather than silently returning an +unannotated video: + +| Platform | Install | +|----------|---------| +| Windows | `winget install Gyan.FFmpeg` | +| macOS | `brew install ffmpeg` | +| Linux | `sudo apt install ffmpeg` | + +A new terminal may be needed for `PATH` to pick them up. An existing run can be annotated afterwards +with `node test/scenario/out/renderEvidenceChapters.js `. + +| Target | Flags | Also required | Use for | +|--------|-------|---------------|---------| +| Installed Insiders, else Stable | *(none — the default)* | nothing | Reproducing a report against shipped behavior | +| Dev build from this checkout | `--dev` | `npm run electron`, `npm run transpile-client` | Verifying an unmerged change | +| A specific install | `--build ` | nothing | Pinning an exact build | | Web | `--web --headless` | `npm run transpile-client` | Browser-only behavior | -`--build` takes the application root — the install directory on Windows and Linux, or the `.app` -bundle on macOS: +With no target flag the runner finds an installed VS Code Insiders (falling back to Stable) and logs +which one it chose. `--build` takes the application root — the install directory on Windows and +Linux, or the `.app` bundle on macOS: ```bash # Windows @@ -41,9 +54,10 @@ bundle on macOS: --build "/Applications/Visual Studio Code - Insiders.app" ``` -An installed build runs with its own profile and extensions directory, so your extensions and -settings never leak into the recording. Insiders only reproduces **shipped** behavior — to validate -an unmerged change, run the dev build from a checkout that contains it. +Every target runs with its own profile and extensions directory, so your extensions and settings +never leak into the recording, and the window is sized to the recording canvas so the capture has no +empty margins. An installed build only reproduces **shipped** behavior — to validate an unmerged +change, use `--dev` in a checkout that contains it. ## Write the scenario @@ -130,7 +144,7 @@ Each step receives a `context` with `app`, `workbench`, `code`, `page`, and `ski ## Run it ```bash -node test/scenario/out/runScenario.js --build "" +node test/scenario/out/runScenario.js ``` Exit code `0` means every step passed, `1` means the run failed or was aborted, `2` a usage error. @@ -181,16 +195,17 @@ the issue or pull request by dragging it into the comment box. skill when a scenario is not yet covered there, or to iterate locally before proposing one. -User: "/validate-ui-scenario reproduce https://github.com/microsoft/vscode/issues/250159 against my -installed VS Code Insiders, and give me the report and the annotated video." +User: "/validate-ui-scenario reproduce https://github.com/microsoft/vscode/issues/250159" -1. Read the issue and identify the observable claim: searching `chat confirm` in the Settings editor +1. Confirm `ffmpeg`/`ffprobe` are available; if not, say so and give the install command before + running, so the user is not surprised by a video without step titles. +2. Read the issue and identify the observable claim: searching `chat confirm` in the Settings editor should match **Max Requests**, whose description mentions confirmation. -2. Add a baseline step (`max requests` finds the setting) so a failure cannot be explained by the +3. Add a baseline step (`max requests` finds the setting) so a failure cannot be explained by the setting being missing from the build. -3. Write `.build/vscode-playwright-mcp/issue-250159.cjs`, run it with `--build`, and read the - printed report path. -4. Report the outcome per step, link `report.html`, and attach `videos/annotated.mp4`. +4. Write `.build/vscode-playwright-mcp/issue-250159.cjs` and run it with no target flag, which uses + the installed Insiders; read the printed report path. +5. Report the outcome per step, link `report.html`, and attach `videos/annotated.mp4`. The run fails at the search step, and that is the answer: the issue reproduces. Report it as a successful reproduction, not as a broken scenario. diff --git a/test/automation/src/playwrightElectron.ts b/test/automation/src/playwrightElectron.ts index c161c5d29b93df..1e4dc298437003 100644 --- a/test/automation/src/playwrightElectron.ts +++ b/test/automation/src/playwrightElectron.ts @@ -37,6 +37,12 @@ export async function launch(options: LaunchOptions): Promise<{ electronProcess: async function launchElectron(configuration: IElectronConfiguration, options: LaunchOptions) { const { logger, tracing, snapshots } = options; + // The recording canvas is fixed, but VS Code sizes its own window (1440x900 + // with a workspace, 1200x800 empty), so the capture would otherwise show the + // window in the top-left corner of a larger frame. The window is resized to + // match below, which also renders reliably when it is larger than the screen. + const videoSize = { width: 1920, height: 1080 }; + const playwrightImpl = options.playwright ?? playwright; let electron; try { @@ -46,7 +52,7 @@ async function launchElectron(configuration: IElectronConfiguration, options: La recordVideo: options.videosPath ? { dir: options.videosPath, - size: { width: 1920, height: 1080 } + size: videoSize } : undefined, env: configuration.env as { [key: string]: string }, timeout: LAUNCH_TIMEOUT @@ -63,6 +69,18 @@ async function launchElectron(configuration: IElectronConfiguration, options: La throw enrichLaunchError(error, options); } } + if (options.videosPath) { + try { + await electron.evaluate(({ BrowserWindow }, size) => { + const target = BrowserWindow.getAllWindows()[0]; + target?.setBounds({ x: 0, y: 0, width: size.width, height: size.height }); + }, videoSize); + } catch (error) { + // A mismatched window only wastes pixels in the recording, so never fail + // a run because the window could not be resized. + logger.log(`Playwright (Electron): Failed to size the window to the recording (${error})`); + } + } // Recording is per page, so sample the origin once the first window exists // rather than when the application finished launching. const videoStartedAt = options.videosPath ? Date.now() : undefined; diff --git a/test/scenario/src/application.ts b/test/scenario/src/application.ts index dfbe0e00d72de5..0ca8765d42f8c2 100644 --- a/test/scenario/src/application.ts +++ b/test/scenario/src/application.ts @@ -95,10 +95,61 @@ function parseQuality(): Quality { // // #### Electron #### // +/** + * Locate an installed VS Code Insiders, then Stable. + * + * Reproducing a reported issue is the common case, and that means running the + * shipped product rather than a build from this checkout, so an installed build + * is used when the caller did not choose a target. + */ +function findInstalledBuild(): string | undefined { + const candidates: string[] = []; + switch (process.platform) { + case 'win32': { + const roots = [process.env.LOCALAPPDATA, process.env.ProgramFiles, process.env['ProgramFiles(x86)']].filter((root): root is string => !!root); + for (const root of roots) { + candidates.push(path.join(root, 'Programs', 'Microsoft VS Code Insiders'), path.join(root, 'Microsoft VS Code Insiders')); + } + for (const root of roots) { + candidates.push(path.join(root, 'Programs', 'Microsoft VS Code'), path.join(root, 'Microsoft VS Code')); + } + break; + } + case 'darwin': + candidates.push( + '/Applications/Visual Studio Code - Insiders.app', + path.join(os.homedir(), 'Applications', 'Visual Studio Code - Insiders.app'), + '/Applications/Visual Studio Code.app', + path.join(os.homedir(), 'Applications', 'Visual Studio Code.app') + ); + break; + default: + candidates.push('/usr/share/code-insiders', '/opt/visual-studio-code-insiders', '/usr/share/code', '/opt/visual-studio-code'); + break; + } + return candidates.find(candidate => { + try { + return fs.existsSync(candidate) && fs.existsSync(getBuildElectronPath(candidate)); + } catch { + return false; // an incomplete install is not a usable target + } + }); +} + if (!opts.web) { let testCodePath = opts.build; let electronPath: string | undefined; + if (!testCodePath && !opts.dev) { + testCodePath = findInstalledBuild(); + if (testCodePath) { + // `getApplication` launches whatever `opts.build` names, so record the + // choice there rather than only in this block. + opts.build = testCodePath; + logger.log(`No target given, using the installed build at ${testCodePath}. Pass --dev to run this checkout instead.`); + } + } + if (testCodePath) { electronPath = getBuildElectronPath(testCodePath); version = getBuildVersion(testCodePath); @@ -111,7 +162,7 @@ if (!opts.web) { } if (!fs.existsSync(electronPath || '')) { - fail(`Cannot find VSCode at ${electronPath}. Please run VSCode once first (scripts/code.sh, scripts\\code.bat) and try again.`); + fail(`Cannot find VS Code at ${electronPath}. Install VS Code Insiders, pass --build , or build this checkout and pass --dev.`); } quality = parseQuality(); diff --git a/test/scenario/src/options.ts b/test/scenario/src/options.ts index 424f6c271a858b..24ce8520eb3e90 100644 --- a/test/scenario/src/options.ts +++ b/test/scenario/src/options.ts @@ -20,7 +20,8 @@ export const opts = minimist(args, { 'web', 'headless', 'video', - 'autostart' + 'autostart', + 'dev' ], default: { verbose: false @@ -31,6 +32,7 @@ export const opts = minimist(args, { headless?: boolean; web?: boolean; build?: string; + dev?: boolean; browser?: 'chromium' | 'webkit' | 'firefox' | 'chromium-msedge' | 'chromium-chrome' | undefined; electronArgs?: string; video?: boolean; diff --git a/test/scenario/src/renderEvidenceChapters.ts b/test/scenario/src/renderEvidenceChapters.ts index 2e0d51e84ef630..d0e2892c572324 100644 --- a/test/scenario/src/renderEvidenceChapters.ts +++ b/test/scenario/src/renderEvidenceChapters.ts @@ -299,12 +299,24 @@ function wrap(value: string, limit: number, maxLines: number): string[] { return lines; } -if (require.main === module) { +/** + * Render captions without letting a presentation step fail a validation run. + * + * The raw recording is authoritative, so a missing or failing ffmpeg is reported + * and otherwise ignored. + */ +export function tryRenderChapters(runRoot: string): void { try { - renderChapters(path.resolve(process.argv[2] ?? process.env.RUN_ROOT ?? '.')); + renderChapters(runRoot); } catch (error) { - // Captions are a presentation aid, so never fail a validation run because - // the recording could not be annotated. The raw recording is authoritative. - console.warn(`Unable to render evidence captions: ${error instanceof Error ? error.message : error}`); + const message = error instanceof Error ? error.message : String(error); + const missingTool = /spawnSync (ffmpeg|ffprobe) ENOENT/u.exec(message); + console.warn(missingTool + ? `Unable to render evidence captions: ${missingTool[1]} is not on PATH. Install ffmpeg and re-run this command; the raw recording is unaffected.` + : `Unable to render evidence captions: ${message}`); } } + +if (require.main === module) { + tryRenderChapters(path.resolve(process.argv[2] ?? process.env.RUN_ROOT ?? '.')); +} diff --git a/test/scenario/src/runScenario.ts b/test/scenario/src/runScenario.ts index aa45a1b72173bc..143cdf0065a150 100644 --- a/test/scenario/src/runScenario.ts +++ b/test/scenario/src/runScenario.ts @@ -3,12 +3,45 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { execFileSync } from 'child_process'; import type { Page } from '@playwright/test'; import * as path from 'path'; import type { Application, Code, Workbench } from '../../automation'; import { ApplicationService, JSONValue } from './application'; import { EvidenceService } from './evidence'; -import { renderChapters } from './renderEvidenceChapters'; +import { tryRenderChapters } from './renderEvidenceChapters'; + +/** + * Report missing video tooling before anything is launched. + * + * Captions are rendered after the run, so a missing ffmpeg is only discovered + * once the scenario has already finished. Say so up front, with the command to + * fix it, rather than letting the run complete and produce no annotated video. + */ +function checkVideoTooling(): void { + const missing = ['ffmpeg', 'ffprobe'].filter(tool => { + try { + execFileSync(process.env[`${tool.toUpperCase()}_PATH`] ?? tool, ['-version'], { stdio: 'ignore' }); + return false; + } catch { + return true; + } + }); + if (!missing.length) { + return; + } + const install = process.platform === 'win32' + ? 'winget install Gyan.FFmpeg' + : process.platform === 'darwin' + ? 'brew install ffmpeg' + : 'sudo apt install ffmpeg'; + console.warn( + `Warning: ${missing.join(' and ')} not found on PATH, so the recording will not be captioned with step titles.\n` + + ` The run still produces the raw video, screenshots, trace and report.\n` + + ` To caption it, install ffmpeg (${install}), make sure it is on PATH, and re-run,\n` + + ` or annotate the finished run with: node test/scenario/out/renderEvidenceChapters.js ` + ); +} /** * Runs a UI validation scenario end to end and writes an evidence bundle. @@ -95,6 +128,7 @@ function loadScenario(scenarioPath: string): Scenario { } export async function runScenario(scenario: Scenario): Promise<{ runPath: string; outcome: 'passed' | 'failed' | 'aborted' }> { + checkVideoTooling(); const appService = new ApplicationService(); const evidence = new EvidenceService(appService); const runPath = await evidence.start( @@ -151,7 +185,7 @@ export async function runScenario(scenario: Scenario): Promise<{ runPath: string const reportPath = await evidence.finish(outcome, notes); console.log(`Report: ${reportPath}`); - renderChapters(runPath); + tryRenderChapters(runPath); return { runPath, outcome }; } From 430b12e27446027bf80fd58898ab51584f633307 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Thu, 20 Aug 2026 16:00:55 -0700 Subject: [PATCH 2/4] Find an installed ffmpeg, pace the steps, and classify blocked steps Follow-up to the same skill run. Captions were missing even though ffmpeg was installed. A PATH edit only reaches processes started afterwards, so an editor that was already running never sees it, and the runner concluded ffmpeg was absent. It now looks in the usual install locations as well as PATH, which is the difference between an annotated recording and a raw one on a machine that already has ffmpeg. Steps flowed past too quickly to read. A caption is only legible for as long as its step is on screen, and steps that assert rather than type can complete in a few hundred milliseconds. Each finished step is now held briefly, controlled by `stepPauseMs` and disabled with `0` for timing-sensitive scenarios. Steps that cannot be automated were indistinguishable from ones that were merely unavailable. `skip` now takes `needs: human` or `needs: infrastructure`: the first means a person has to check it, the second means the harness could do it but cannot yet, which is an enhancement request rather than a permanent limit. The distinction is recorded in the manifest, highlighted in the report, shown on the video caption, and printed at the end of the run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb443eb-11e5-40a1-8608-7f593fa79485 --- .github/skills/validate-ui-scenario/SKILL.md | 47 +++++++++-- test/scenario/src/evidence.ts | 29 +++++-- test/scenario/src/renderEvidenceChapters.ts | 71 ++++++++++++++-- test/scenario/src/runScenario.ts | 89 +++++++++++++++----- 4 files changed, 193 insertions(+), 43 deletions(-) diff --git a/.github/skills/validate-ui-scenario/SKILL.md b/.github/skills/validate-ui-scenario/SKILL.md index 867f0ed40e4c0a..941424d6727cb4 100644 --- a/.github/skills/validate-ui-scenario/SKILL.md +++ b/.github/skills/validate-ui-scenario/SKILL.md @@ -22,10 +22,11 @@ npm install # once npm --prefix test/scenario run compile # after any change under test/scenario ``` -**Check `ffmpeg` and `ffprobe` are on `PATH` before running.** Without them the scenario still runs -and keeps the raw recording, but the video is not captioned with step titles. The runner warns at -startup; if they are missing, tell the user how to install them rather than silently returning an -unannotated video: +**Check `ffmpeg` and `ffprobe` are available before running.** The runner looks on `PATH` and in the +usual install locations, so an ffmpeg installed after the editor started is still found. Without them +the scenario still runs and keeps the raw recording, but the video is not captioned with step titles. +The runner warns at startup; if they are missing, tell the user how to install them rather than +silently returning an unannotated video: | Platform | Install | |----------|---------| @@ -33,8 +34,9 @@ unannotated video: | macOS | `brew install ffmpeg` | | Linux | `sudo apt install ffmpeg` | -A new terminal may be needed for `PATH` to pick them up. An existing run can be annotated afterwards -with `node test/scenario/out/renderEvidenceChapters.js `. +A new terminal may be needed for `PATH` to pick them up, or set `FFMPEG_PATH` and `FFPROBE_PATH`. An +existing run can be annotated afterwards with +`node test/scenario/out/renderEvidenceChapters.js `. | Target | Flags | Also required | Use for | |--------|-------|---------------|---------| @@ -131,15 +133,36 @@ module.exports = { | `workspacePath` | Disposable folder to open | | `userSettings` | Settings seeded into the profile before launch | | `extraArgs` | Extra VS Code command-line arguments | +| `stepPauseMs` | How long to hold each finished step so its caption is readable. Defaults to `1000`; set `0` when the scenario is timing-sensitive | -Each step receives a `context` with `app`, `workbench`, `code`, `page`, and `skip(reason)`. +Each step receives a `context` with `app`, `workbench`, `code`, `page`, and `skip(reason, options)`. `workbench` exposes the feature helpers (`settingsEditor`, `quickaccess`, `editors`, `terminal`, `chat`, …); `page` is the Playwright page for anything they do not cover. - **Return a string** describing how the step was validated. It appears in the report. - **Throw** to fail the step. The message is recorded, and the run stops. -- **Call `skip(reason)`** when hardware, an account, or a service is unavailable. The run stops and - is reported as `aborted`, never as passed. +- **Call `skip(reason, { needs })`** when the step cannot be validated automatically. The run stops + and is reported as `aborted`, never as passed. + +## Steps that cannot be automated + +Decide this while planning, before writing the scenario, and classify each one — the two kinds have +different consequences: + +| `needs` | Meaning | What to do | +|---------|---------|------------| +| `human` | A person is required: physical hardware, a subjective judgement, a sign-in that cannot be scripted | Report the step so someone can check it by hand | +| `infrastructure` | Automatable in principle, but the harness cannot do it yet | Report it as an **enhancement to this skill**, naming the missing capability | + +```js +ctx.skip('Comparing physical print output requires a person with a printer.', { needs: 'human' }); +ctx.skip('The harness cannot drive native OS file dialogs.', { needs: 'infrastructure' }); +``` + +Blocked steps are recorded in `manifest.json`, highlighted in a **Needs attention** section of +`report.html`, marked on the video caption (`SKIPPED - NEEDS HUMAN`), and printed at the end of the +run. Surface them in your summary — never quietly drop a step you could not perform, and never +weaken an assertion so that it passes. ## Run it @@ -184,6 +207,12 @@ Summarize the outcome, list failed or skipped steps, link `report.html`, and sta VS Code version and quality (both are in `manifest.json`), and the source issue. Attach the video to the issue or pull request by dragging it into the comment box. +Always call out, separately from the pass/fail result: + +- **steps that need a person**, so someone knows what is still unverified; +- **steps blocked on a missing harness capability**, named as a concrete enhancement to this skill; +- **anything that degraded the evidence**, such as a missing ffmpeg leaving the video uncaptioned. + ## Related - **Interactive exploration.** `test/mcp` also serves these tools over MCP (`vscode_automation_*`), diff --git a/test/scenario/src/evidence.ts b/test/scenario/src/evidence.ts index d2a3f3c029d683..856cb23bd03f51 100644 --- a/test/scenario/src/evidence.ts +++ b/test/scenario/src/evidence.ts @@ -15,6 +15,15 @@ const logsRootPath = path.join(artifactRootPath, 'logs'); const qualityNames = ['Dev', 'Insiders', 'Stable', 'Exploration', 'OSS']; export type StepStatus = 'started' | 'passed' | 'failed' | 'skipped'; +/** + * Why a step could not be validated automatically. + * + * `human` means the step needs a person (hardware, a judgement call, a sign-in + * that cannot be scripted). `infrastructure` means it could be automated, but + * the harness is missing a capability — those are enhancement requests, not + * permanent limits, so they are reported separately. + */ +export type StepBlocker = 'human' | 'infrastructure'; export type RunOutcome = 'passed' | 'failed' | 'aborted'; interface EvidenceCapture { @@ -23,6 +32,7 @@ interface EvidenceCapture { screenshot: string; windowUrl: string; details?: string; + blockedOn?: StepBlocker; } interface EvidenceStep { @@ -180,7 +190,7 @@ export class EvidenceService { } } - async step(id: string, title: string, status: StepStatus, details?: string): Promise<{ screenshot: Buffer; screenshotPath: string }> { + async step(id: string, title: string, status: StepStatus, details?: string, blockedOn?: StepBlocker): Promise<{ screenshot: Buffer; screenshotPath: string }> { const run = this.requireRun(); if (run.state !== 'active') { throw new Error(`Evidence run '${run.id}' is busy (${run.state}).`); @@ -243,7 +253,8 @@ export class EvidenceService { timestamp: new Date().toISOString(), screenshot: screenshotName, windowUrl: app.code.driver.currentPage.url(), - details + details, + blockedOn }); this.writeManifest(); @@ -453,19 +464,27 @@ export class EvidenceService { const rows = run.steps.map(step => { const result = step.captures.at(-1); const screenshots = step.captures.map(capture => `${escapeHtml(capture.status)}`).join(', '); - return `${escapeHtml(step.id)}${escapeHtml(step.title)}${escapeHtml(result?.status ?? 'unknown')}${screenshots}${escapeHtml(result?.details ?? '')}`; + const blocker = result?.blockedOn ? ` (needs ${escapeHtml(result.blockedOn)})` : ''; + return `${escapeHtml(step.id)}${escapeHtml(step.title)}${escapeHtml(result?.status ?? 'unknown')}${blocker}${screenshots}${escapeHtml(result?.details ?? '')}`; }).join(''); + const blocked = run.steps + .map(step => ({ step, capture: step.captures.at(-1) })) + .filter((entry): entry is { step: EvidenceStep; capture: EvidenceCapture } => !!entry.capture?.blockedOn); + const blockedSection = blocked.length + ? `

Needs attention

    ${blocked.map(({ step, capture }) => + `
  • ${escapeHtml(step.id)} needs ${escapeHtml(capture.blockedOn ?? '')}: ${escapeHtml(capture.details ?? '')}
    • `).join('')}
` + : ''; const videoElements = run.artifacts.videos.length ? run.artifacts.videos.map(video => ``).join('') : '

No video file was produced.

'; const logs = run.artifacts.logs.map(log => `
  • ${escapeHtml(log)}
    • `).join(''); const html = ` ${escapeHtml(run.title)} - +

    ${escapeHtml(run.title)}

    Scenario: ${escapeHtml(run.scenarioId)}
    Outcome: ${escapeHtml(run.outcome ?? 'unknown')}
    Started: ${escapeHtml(run.startedAt)}
    Completed: ${escapeHtml(run.completedAt ?? '')}

    Source: ${run.source ? `${escapeHtml(run.source)}` : 'Not recorded'}
    Workspace: ${escapeHtml(run.workspacePath ?? 'Not specified')}
    Environment: ${escapeHtml(`${run.environment.platform} ${run.environment.architecture}; VS Code ${run.environment.vscodeVersion} (${run.environment.quality}); Node ${run.environment.nodeVersion}; commit ${run.environment.commit ?? 'unknown'}`)}

    ${escapeHtml(run.notes ?? '')}

    Steps

    ${rows}
    IDTitleResultScreenshotsDetails
    -

    Video

    ${videoElements}

    Trace and logs

    ${logs ? `
      ${logs}
    ` : '

    No new trace or log content was produced.

    '}`; +${blockedSection}

    Video

    ${videoElements}

    Trace and logs

    ${logs ? `
      ${logs}
    ` : '

    No new trace or log content was produced.

    '}`; const reportPath = path.join(run.runPath, 'report.html'); fs.writeFileSync(reportPath, html); return reportPath; diff --git a/test/scenario/src/renderEvidenceChapters.ts b/test/scenario/src/renderEvidenceChapters.ts index d0e2892c572324..3a0c528396315e 100644 --- a/test/scenario/src/renderEvidenceChapters.ts +++ b/test/scenario/src/renderEvidenceChapters.ts @@ -24,6 +24,7 @@ interface Capture { status?: string; timestamp?: string; details?: string; + blockedOn?: string; } interface Step { @@ -50,8 +51,62 @@ interface Caption { accent: string; } -const ffmpeg = process.env.FFMPEG_PATH ?? 'ffmpeg'; -const ffprobe = process.env.FFPROBE_PATH ?? 'ffprobe'; +/** + * Locate ffmpeg or ffprobe. + * + * A PATH edit only reaches processes started afterwards, so ffmpeg is commonly + * installed and still invisible to an editor that was already running. Well + * known install locations are probed before giving up, which is the difference + * between an annotated recording and a raw one. + */ +export function resolveVideoTool(tool: 'ffmpeg' | 'ffprobe'): string | undefined { + const override = process.env[`${tool.toUpperCase()}_PATH`]; + const executable = process.platform === 'win32' ? `${tool}.exe` : tool; + const candidates = override ? [override] : [tool, ...installedToolCandidates(executable)]; + for (const candidate of candidates) { + try { + execFileSync(candidate, ['-version'], { stdio: 'ignore' }); + return candidate; + } catch { + // try the next location + } + } + return undefined; +} + +function installedToolCandidates(executable: string): string[] { + const candidates: string[] = []; + if (process.platform === 'win32') { + const localAppData = process.env.LOCALAPPDATA; + if (localAppData) { + candidates.push(path.join(localAppData, 'Microsoft', 'WinGet', 'Links', executable)); + // winget unpacks into Packages///bin, so the build directory + // carries a version that cannot be hard-coded. + const packages = path.join(localAppData, 'Microsoft', 'WinGet', 'Packages'); + for (const pkg of readDirectories(packages).filter(name => /ffmpeg/iu.test(name))) { + for (const build of readDirectories(path.join(packages, pkg))) { + candidates.push(path.join(packages, pkg, build, 'bin', executable)); + } + } + } + candidates.push( + path.join(process.env.ProgramData ?? '', 'chocolatey', 'bin', executable), + path.join(process.env.ProgramFiles ?? '', 'ffmpeg', 'bin', executable) + ); + } else { + candidates.push(`/opt/homebrew/bin/${executable}`, `/usr/local/bin/${executable}`, `/usr/bin/${executable}`); + } + return candidates.filter(candidate => fs.existsSync(candidate)); +} + +function readDirectories(root: string): string[] { + try { + return fs.readdirSync(root, { withFileTypes: true }).filter(entry => entry.isDirectory()).map(entry => entry.name); + } catch { + return []; + } +} + const fontCandidates = process.env.CHAPTER_FONT ? [process.env.CHAPTER_FONT] : [ '/usr/share/fonts/truetype/dejavu/DejaVuSans-Bold.ttf', '/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf', @@ -99,6 +154,11 @@ export function renderChapters(runRoot: string): void { console.log('No usable font was found, so no captions were rendered.'); return; } + const ffmpeg = resolveVideoTool('ffmpeg'); + const ffprobe = resolveVideoTool('ffprobe'); + if (!ffmpeg || !ffprobe) { + throw new Error(`${[!ffmpeg && 'ffmpeg', !ffprobe && 'ffprobe'].filter(Boolean).join(' and ')} could not be found`); + } const videoPath = path.join(runRoot, relativeVideo); const outputRelative = 'videos/annotated.mp4'; @@ -172,7 +232,7 @@ export function renderChapters(runRoot: string): void { captions.push({ from: boundary.at, to: index + 1 < boundaries.length ? boundaries[index + 1].at : duration, - eyebrow: `STEP ${index + 1} OF ${boundaries.length} ${String(step.id ?? '').toUpperCase()} ${status.toUpperCase()}`, + eyebrow: `STEP ${index + 1} OF ${boundaries.length} ${String(step.id ?? '').toUpperCase()} ${status.toUpperCase()}${closing?.blockedOn ? ` - NEEDS ${closing.blockedOn.toUpperCase()}` : ''}`, title: wrap(step.title ?? '', columnsFor(titleSize), MAX_TITLE_LINES), details: wrap(closing?.details ?? '', columnsFor(detailSize), MAX_DETAIL_LINES), accent: accentFor(status) @@ -310,10 +370,7 @@ export function tryRenderChapters(runRoot: string): void { renderChapters(runRoot); } catch (error) { const message = error instanceof Error ? error.message : String(error); - const missingTool = /spawnSync (ffmpeg|ffprobe) ENOENT/u.exec(message); - console.warn(missingTool - ? `Unable to render evidence captions: ${missingTool[1]} is not on PATH. Install ffmpeg and re-run this command; the raw recording is unaffected.` - : `Unable to render evidence captions: ${message}`); + console.warn(`Unable to render evidence captions: ${message}. The raw recording is unaffected.`); } } diff --git a/test/scenario/src/runScenario.ts b/test/scenario/src/runScenario.ts index 143cdf0065a150..8b857d2ffeadfa 100644 --- a/test/scenario/src/runScenario.ts +++ b/test/scenario/src/runScenario.ts @@ -3,13 +3,12 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { execFileSync } from 'child_process'; import type { Page } from '@playwright/test'; import * as path from 'path'; import type { Application, Code, Workbench } from '../../automation'; import { ApplicationService, JSONValue } from './application'; -import { EvidenceService } from './evidence'; -import { tryRenderChapters } from './renderEvidenceChapters'; +import { EvidenceService, StepBlocker } from './evidence'; +import { resolveVideoTool, tryRenderChapters } from './renderEvidenceChapters'; /** * Report missing video tooling before anything is launched. @@ -19,14 +18,7 @@ import { tryRenderChapters } from './renderEvidenceChapters'; * fix it, rather than letting the run complete and produce no annotated video. */ function checkVideoTooling(): void { - const missing = ['ffmpeg', 'ffprobe'].filter(tool => { - try { - execFileSync(process.env[`${tool.toUpperCase()}_PATH`] ?? tool, ['-version'], { stdio: 'ignore' }); - return false; - } catch { - return true; - } - }); + const missing = (['ffmpeg', 'ffprobe'] as const).filter(tool => !resolveVideoTool(tool)); if (!missing.length) { return; } @@ -36,13 +28,18 @@ function checkVideoTooling(): void { ? 'brew install ffmpeg' : 'sudo apt install ffmpeg'; console.warn( - `Warning: ${missing.join(' and ')} not found on PATH, so the recording will not be captioned with step titles.\n` + + `Warning: ${missing.join(' and ')} could not be found, so the recording will not be captioned with step titles.\n` + ` The run still produces the raw video, screenshots, trace and report.\n` + - ` To caption it, install ffmpeg (${install}), make sure it is on PATH, and re-run,\n` + - ` or annotate the finished run with: node test/scenario/out/renderEvidenceChapters.js ` + ` Install ffmpeg (${install}) and re-run. If it is already installed, a PATH change does not\n` + + ` reach an editor that was already running, so restart it or set FFMPEG_PATH and FFPROBE_PATH,\n` + + ` then annotate the finished run with: node test/scenario/out/renderEvidenceChapters.js ` ); } +function wait(milliseconds: number): Promise { + return new Promise(resolve => setTimeout(resolve, milliseconds)); +} + /** * Runs a UI validation scenario end to end and writes an evidence bundle. * @@ -64,8 +61,15 @@ export interface ScenarioContext { readonly code: Code; /** The window the driver is currently attached to. */ readonly page: Page; - /** Marks the current step `skipped` and stops the run. */ - skip(reason: string): never; + /** + * Marks the current step `skipped` and stops the run. + * + * Say what is missing rather than what failed, and classify it: `human` when + * a person is required, `infrastructure` when the harness could do it but + * cannot yet. Both are reported prominently; the second is an enhancement + * request against this skill. + */ + skip(reason: string, options?: { needs?: StepBlocker }): never; } export interface ScenarioStep { @@ -90,10 +94,25 @@ export interface Scenario { readonly workspacePath?: string; readonly userSettings?: Record; readonly extraArgs?: string[]; + /** + * How long to hold on each completed step, in milliseconds. + * + * The recording is watched by a person, and a caption is only readable for as + * long as its step is on screen, so each step is held briefly once it + * finishes. Set `0` when the scenario depends on timing and must run at full + * speed. + */ + readonly stepPauseMs?: number; readonly steps: readonly ScenarioStep[]; } -class SkipStep extends Error { } +const DEFAULT_STEP_PAUSE_MS = 1000; + +class SkipStep extends Error { + constructor(reason: string, readonly needs?: StepBlocker) { + super(reason); + } +} function loadScenario(scenarioPath: string): Scenario { // A CommonJS scenario needs a `.cjs` extension because this package is an ES @@ -127,8 +146,17 @@ function loadScenario(scenarioPath: string): Scenario { return scenario; } -export async function runScenario(scenario: Scenario): Promise<{ runPath: string; outcome: 'passed' | 'failed' | 'aborted' }> { +export interface ScenarioBlocker { + readonly id: string; + readonly title: string; + readonly needs: StepBlocker; + readonly reason: string; +} + +export async function runScenario(scenario: Scenario): Promise<{ runPath: string; outcome: 'passed' | 'failed' | 'aborted'; blockers: ScenarioBlocker[] }> { checkVideoTooling(); + const pauseMs = Math.max(0, scenario.stepPauseMs ?? DEFAULT_STEP_PAUSE_MS); + const blockers: ScenarioBlocker[] = []; const appService = new ApplicationService(); const evidence = new EvidenceService(appService); const runPath = await evidence.start( @@ -156,17 +184,23 @@ export async function runScenario(scenario: Scenario): Promise<{ runPath: string workbench: app.workbench, code: app.code, page: app.code.driver.currentPage, - skip: (reason: string) => { throw new SkipStep(reason); } + skip: (reason: string, options?: { needs?: StepBlocker }) => { throw new SkipStep(reason, options?.needs); } }; try { const details = await step.run(context); await evidence.step(step.id, step.title, 'passed', details || undefined); console.log(` PASS ${step.id} ${step.title}`); + // Hold the finished step so its caption is readable in the recording. + await wait(pauseMs); } catch (error) { const message = error instanceof Error ? error.message : String(error); const skipped = error instanceof SkipStep; - await evidence.step(step.id, step.title, skipped ? 'skipped' : 'failed', message); - console.log(` ${skipped ? 'SKIP' : 'FAIL'} ${step.id} ${step.title}: ${message}`); + const needs = error instanceof SkipStep ? error.needs : undefined; + await evidence.step(step.id, step.title, skipped ? 'skipped' : 'failed', message, needs); + console.log(` ${skipped ? 'SKIP' : 'FAIL'} ${step.id} ${step.title}${needs ? ` [needs ${needs}]` : ''}: ${message}`); + if (needs) { + blockers.push({ id: step.id, title: step.title, needs, reason: message }); + } // A later step cannot be trusted once the product is in an // unexpected state, and a skipped step means its precondition is // unavailable, so stop either way rather than reporting noise. @@ -174,6 +208,7 @@ export async function runScenario(scenario: Scenario): Promise<{ runPath: string // reported as aborted rather than passed. outcome = skipped ? 'aborted' : 'failed'; notes = `${skipped ? 'Skipped' : 'Failed'} at step '${step.id}': ${message}`; + await wait(pauseMs); break; } } @@ -183,10 +218,20 @@ export async function runScenario(scenario: Scenario): Promise<{ runPath: string console.error(`Scenario aborted: ${notes}`); } + if (blockers.length) { + notes = [notes, ...blockers.map(blocker => `Step '${blocker.id}' needs ${blocker.needs}: ${blocker.reason}`)].filter(Boolean).join('\n'); + } + const reportPath = await evidence.finish(outcome, notes); console.log(`Report: ${reportPath}`); tryRenderChapters(runPath); - return { runPath, outcome }; + for (const blocker of blockers) { + const reason = blocker.reason.replace(/\s*\.\s*$/u, ''); + console.log(blocker.needs === 'human' + ? `Needs a person: ${blocker.id} ${blocker.title} - ${reason}.` + : `Needs harness support: ${blocker.id} ${blocker.title} - ${reason}. This is automatable, so report it as an enhancement to the skill.`); + } + return { runPath, outcome, blockers }; } if (require.main === module) { From a692449faed64ab915c22b7c0f848f5e52c0dec7 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Thu, 20 Aug 2026 16:25:51 -0700 Subject: [PATCH 3/4] Report the quality of the build that ran, and finish the target options Review follow-ups. The evidence labelled every installed run `Dev`. Quality was read from the environment, which only describes a build made from this checkout, so a run against installed Insiders was reported as Dev in both the manifest and the report - the evidence named the wrong product. An installed build stamps its own quality in `product.json`, so that is now the source when a build path is given. This also corrects `--build`, which had the same problem before this change. Linux missed Snap installs. Snap keeps the app under a read-only revision root, so a machine with VS Code installed only through Snap found nothing and fell back to the unbuilt checkout - the exact failure the new default exists to avoid. The web launcher recorded 1920x1080 while sizing the page to 1440x900, so the no-empty-margins claim did not hold there. It now matches the canvas while recording and keeps its established size otherwise, so smoke runs are unchanged. `--dev` was accepted but undocumented in the runner's own help, which now lists all three targets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb443eb-11e5-40a1-8608-7f593fa79485 --- .github/skills/validate-ui-scenario/SKILL.md | 6 ++- test/automation/src/playwrightBrowser.ts | 5 +- test/scenario/src/application.ts | 53 ++++++++++++++++++-- test/scenario/src/runScenario.ts | 7 ++- 4 files changed, 61 insertions(+), 10 deletions(-) diff --git a/.github/skills/validate-ui-scenario/SKILL.md b/.github/skills/validate-ui-scenario/SKILL.md index 941424d6727cb4..afc97d15746d51 100644 --- a/.github/skills/validate-ui-scenario/SKILL.md +++ b/.github/skills/validate-ui-scenario/SKILL.md @@ -58,8 +58,10 @@ Linux, or the `.app` bundle on macOS: Every target runs with its own profile and extensions directory, so your extensions and settings never leak into the recording, and the window is sized to the recording canvas so the capture has no -empty margins. An installed build only reproduces **shipped** behavior — to validate an unmerged -change, use `--dev` in a checkout that contains it. +empty margins. The evidence records the quality of the build that actually ran (`Insiders`, +`Stable`, `Dev`), so a report always names the product it validated. An installed build only +reproduces **shipped** behavior — to validate an unmerged change, use `--dev` in a checkout that +contains it. ## Write the scenario diff --git a/test/automation/src/playwrightBrowser.ts b/test/automation/src/playwrightBrowser.ts index a0459eed009db3..deeb7b07d214bb 100644 --- a/test/automation/src/playwrightBrowser.ts +++ b/test/automation/src/playwrightBrowser.ts @@ -133,7 +133,10 @@ async function launchBrowser(options: LaunchOptions, endpoint: string) { // long enough to visibly skew offsets measured against it. const videoStartedAt = options.videosPath ? Date.now() : undefined; const page = await measureAndLog(() => context.newPage(), 'context.newPage()', logger); - await measureAndLog(() => page.setViewportSize({ width: 1440, height: 900 }), 'page.setViewportSize', logger); + // Match the recording canvas while recording, so the capture has no empty + // margins; keep the established size otherwise so smoke runs are unchanged. + const viewport = options.videosPath ? { width: 1920, height: 1080 } : { width: 1440, height: 900 }; + await measureAndLog(() => page.setViewportSize(viewport), 'page.setViewportSize', logger); // Always log failed requests and console errors/warnings (even without // `--verbose`) so that hard-to-reproduce startup stalls can be root caused diff --git a/test/scenario/src/application.ts b/test/scenario/src/application.ts index 0ca8765d42f8c2..2219b5d9187ef4 100644 --- a/test/scenario/src/application.ts +++ b/test/scenario/src/application.ts @@ -71,12 +71,47 @@ function fail(errorMessage): void { let quality: Quality; let version: string | undefined; -function parseQuality(): Quality { - if (process.env.VSCODE_DEV === '1') { +/** + * Read the `quality` a build was stamped with. + * + * `parseQuality` reads the environment, which only describes a build made from + * this checkout. An installed build carries its own quality in `product.json`, + * and without it every installed run is labelled `Dev` in the evidence, which + * misreports which product was actually validated. + */ +function readBuildQuality(root: string): string | undefined { + // Windows installs nest the app under a commit-stamped directory, so the + // manifest is not always directly under the application root. + const candidates = [path.join(root, 'resources', 'app', 'product.json')]; + try { + for (const entry of fs.readdirSync(root, { withFileTypes: true })) { + if (entry.isDirectory()) { + candidates.push(path.join(root, entry.name, 'resources', 'app', 'product.json')); + } + } + } catch { + // an unreadable root is reported by the electron path check below + } + candidates.push(path.join(root, 'Contents', 'Resources', 'app', 'product.json')); // macOS bundle + for (const candidate of candidates) { + try { + const product = JSON.parse(fs.readFileSync(candidate, 'utf8')) as { quality?: string }; + if (product.quality) { + return product.quality; + } + } catch { + // try the next location + } + } + return undefined; +} + +function parseQuality(stamped?: string): Quality { + if (!stamped && process.env.VSCODE_DEV === '1') { return Quality.Dev; } - const quality = process.env.VSCODE_QUALITY ?? ''; + const quality = stamped ?? process.env.VSCODE_QUALITY ?? ''; switch (quality) { case 'stable': @@ -124,7 +159,15 @@ function findInstalledBuild(): string | undefined { ); break; default: - candidates.push('/usr/share/code-insiders', '/opt/visual-studio-code-insiders', '/usr/share/code', '/opt/visual-studio-code'); + candidates.push( + '/usr/share/code-insiders', + '/opt/visual-studio-code-insiders', + // Snap keeps the app under a read-only revision root. + '/snap/code-insiders/current/usr/share/code-insiders', + '/usr/share/code', + '/opt/visual-studio-code', + '/snap/code/current/usr/share/code' + ); break; } return candidates.find(candidate => { @@ -165,7 +208,7 @@ if (!opts.web) { fail(`Cannot find VS Code at ${electronPath}. Install VS Code Insiders, pass --build , or build this checkout and pass --dev.`); } - quality = parseQuality(); + quality = parseQuality(testCodePath ? readBuildQuality(testCodePath) : undefined); if (opts.remote) { logger.log(`Running desktop remote smoke tests against ${electronPath}`); diff --git a/test/scenario/src/runScenario.ts b/test/scenario/src/runScenario.ts index 8b857d2ffeadfa..50970fcc9d8c05 100644 --- a/test/scenario/src/runScenario.ts +++ b/test/scenario/src/runScenario.ts @@ -44,7 +44,7 @@ function wait(milliseconds: number): Promise { * Runs a UI validation scenario end to end and writes an evidence bundle. * * ``` - * node test/scenario/out/runScenario.js [--build ] + * node test/scenario/out/runScenario.js [--build | --dev] * ``` * * The scenario file is not part of this repository, so it can be written next to @@ -237,7 +237,10 @@ export async function runScenario(scenario: Scenario): Promise<{ runPath: string if (require.main === module) { const scenarioArgument = process.argv.slice(2).find(argument => !argument.startsWith('--')); if (!scenarioArgument) { - console.error('Usage: node test/scenario/out/runScenario.js [--build ]'); + console.error('Usage: node test/scenario/out/runScenario.js [--build | --dev]'); + console.error(' (no target) run the installed VS Code Insiders, else Stable'); + console.error(' --build run a specific installed build'); + console.error(' --dev run the build from this checkout'); process.exit(2); } const scenarioPath = path.resolve(scenarioArgument); From 8d6bef2c1ab31ff69f3518dfa6466c45a2e4f4cd Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Fri, 21 Aug 2026 09:19:04 -0700 Subject: [PATCH 4/4] Detect a staged update instead of timing out VS Code on Windows applies a downloaded update by swapping the executable during startup, so a launch attempt exits before showing a window. Playwright then waits the full launch timeout and reports that the process "likely crashed or hung", which sends the reader looking for crash dumps that do not exist. Insiders downloads an update most days, so anyone reproducing an issue will meet this. A `new_` beside the target is the marker, and checking for it turns a 60s misleading timeout into an immediate statement of the cause and the fix. Confirmed the launch failure is environmental rather than harness behaviour: a bare Playwright launch of the same installed build, with none of this code in the path, also never receives a window while the update is staged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb443eb-11e5-40a1-8608-7f593fa79485 --- test/scenario/src/application.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/scenario/src/application.ts b/test/scenario/src/application.ts index 2219b5d9187ef4..193a3a308ac21c 100644 --- a/test/scenario/src/application.ts +++ b/test/scenario/src/application.ts @@ -208,6 +208,14 @@ if (!opts.web) { fail(`Cannot find VS Code at ${electronPath}. Install VS Code Insiders, pass --build , or build this checkout and pass --dev.`); } + // Windows applies a downloaded update by swapping the executable during + // startup, so the launched process exits before it ever shows a window and + // the failure reads as a crash. Insiders updates daily, so say what is + // actually wrong instead of leaving a 60s timeout to be misread. + if (electronPath && fs.existsSync(path.join(path.dirname(electronPath), `new_${path.basename(electronPath)}`))) { + fail(`${electronPath} has a downloaded update waiting to be applied, and it exits during startup to install it instead of opening a window. Start and quit VS Code once to apply the update, then run this again.`); + } + quality = parseQuality(testCodePath ? readBuildQuality(testCodePath) : undefined); if (opts.remote) {