diff --git a/.github/skills/validate-ui-scenario/SKILL.md b/.github/skills/validate-ui-scenario/SKILL.md index 16b002502f7d3e..afc97d15746d51 100644 --- a/.github/skills/validate-ui-scenario/SKILL.md +++ b/.github/skills/validate-ui-scenario/SKILL.md @@ -18,21 +18,36 @@ 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 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 | +|----------|---------| +| 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, 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 | +|--------|-------|---------------|---------| +| 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 +56,12 @@ 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. 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 @@ -117,20 +135,41 @@ 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 ```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. @@ -170,6 +209,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_*`), @@ -181,16 +226,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/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/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..193a3a308ac21c 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': @@ -95,10 +130,69 @@ 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', + // 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 => { + 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,10 +205,18 @@ 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(); + // 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) { logger.log(`Running desktop remote smoke tests against ${electronPath}`); 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/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..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) @@ -299,12 +359,21 @@ 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); + console.warn(`Unable to render evidence captions: ${message}. The raw recording is unaffected.`); } } + +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..50970fcc9d8c05 100644 --- a/test/scenario/src/runScenario.ts +++ b/test/scenario/src/runScenario.ts @@ -7,14 +7,44 @@ 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 { EvidenceService, StepBlocker } from './evidence'; +import { resolveVideoTool, 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'] as const).filter(tool => !resolveVideoTool(tool)); + 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 ')} 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` + + ` 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. * * ``` - * 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 @@ -31,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 { @@ -57,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 @@ -94,7 +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( @@ -122,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. @@ -140,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; } } @@ -149,16 +218,29 @@ 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}`); - renderChapters(runPath); - return { runPath, outcome }; + tryRenderChapters(runPath); + 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) { 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);