Skip to content

Commit 78211f2

Browse files
bryanchen-dCopilot
andcommitted
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
1 parent 7edc21d commit 78211f2

6 files changed

Lines changed: 163 additions & 31 deletions

File tree

.github/skills/validate-ui-scenario/SKILL.md

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,34 @@ step boundary, writes the report, and captions the recording with each step and
1818
## Prepare
1919

2020
```bash
21-
npm install # once
21+
npm install # once
2222
npm --prefix test/scenario run compile # after any change under test/scenario
2323
```
2424

25-
Add `ffmpeg` and `ffprobe` to `PATH` to get the caption band on the video. Without them the run still
26-
succeeds and the raw recording is kept.
27-
28-
| Target | Extra flags | Also required | Use for |
29-
|--------|-------------|---------------|---------|
30-
| Installed Insiders | `--build <app-root>` | nothing | Reproducing a report against shipped behavior |
31-
| Dev build from this checkout | *(none)* | `npm run electron`, `npm run transpile-client` | Verifying an unmerged change |
25+
**Check `ffmpeg` and `ffprobe` are on `PATH` before running.** Without them the scenario still runs
26+
and keeps the raw recording, but the video is not captioned with step titles. The runner warns at
27+
startup; if they are missing, tell the user how to install them rather than silently returning an
28+
unannotated video:
29+
30+
| Platform | Install |
31+
|----------|---------|
32+
| Windows | `winget install Gyan.FFmpeg` |
33+
| macOS | `brew install ffmpeg` |
34+
| Linux | `sudo apt install ffmpeg` |
35+
36+
A new terminal may be needed for `PATH` to pick them up. An existing run can be annotated afterwards
37+
with `node test/scenario/out/renderEvidenceChapters.js <run-dir>`.
38+
39+
| Target | Flags | Also required | Use for |
40+
|--------|-------|---------------|---------|
41+
| Installed Insiders, else Stable | *(none — the default)* | nothing | Reproducing a report against shipped behavior |
42+
| Dev build from this checkout | `--dev` | `npm run electron`, `npm run transpile-client` | Verifying an unmerged change |
43+
| A specific install | `--build <app-root>` | nothing | Pinning an exact build |
3244
| Web | `--web --headless` | `npm run transpile-client` | Browser-only behavior |
3345

34-
`--build` takes the application root — the install directory on Windows and Linux, or the `.app`
35-
bundle on macOS:
46+
With no target flag the runner finds an installed VS Code Insiders (falling back to Stable) and logs
47+
which one it chose. `--build` takes the application root — the install directory on Windows and
48+
Linux, or the `.app` bundle on macOS:
3649

3750
```bash
3851
# Windows
@@ -41,9 +54,10 @@ bundle on macOS:
4154
--build "/Applications/Visual Studio Code - Insiders.app"
4255
```
4356

44-
An installed build runs with its own profile and extensions directory, so your extensions and
45-
settings never leak into the recording. Insiders only reproduces **shipped** behavior — to validate
46-
an unmerged change, run the dev build from a checkout that contains it.
57+
Every target runs with its own profile and extensions directory, so your extensions and settings
58+
never leak into the recording, and the window is sized to the recording canvas so the capture has no
59+
empty margins. An installed build only reproduces **shipped** behavior — to validate an unmerged
60+
change, use `--dev` in a checkout that contains it.
4761

4862
## Write the scenario
4963

@@ -130,7 +144,7 @@ Each step receives a `context` with `app`, `workbench`, `code`, `page`, and `ski
130144
## Run it
131145

132146
```bash
133-
node test/scenario/out/runScenario.js <scenario.cjs> --build "<app-root>"
147+
node test/scenario/out/runScenario.js <scenario.cjs>
134148
```
135149

136150
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.
181195
skill when a scenario is not yet covered there, or to iterate locally before proposing one.
182196

183197
<example>
184-
User: "/validate-ui-scenario reproduce https://github.com/microsoft/vscode/issues/250159 against my
185-
installed VS Code Insiders, and give me the report and the annotated video."
198+
User: "/validate-ui-scenario reproduce https://github.com/microsoft/vscode/issues/250159"
186199

187-
1. Read the issue and identify the observable claim: searching `chat confirm` in the Settings editor
200+
1. Confirm `ffmpeg`/`ffprobe` are available; if not, say so and give the install command before
201+
running, so the user is not surprised by a video without step titles.
202+
2. Read the issue and identify the observable claim: searching `chat confirm` in the Settings editor
188203
should match **Max Requests**, whose description mentions confirmation.
189-
2. Add a baseline step (`max requests` finds the setting) so a failure cannot be explained by the
204+
3. Add a baseline step (`max requests` finds the setting) so a failure cannot be explained by the
190205
setting being missing from the build.
191-
3. Write `.build/vscode-playwright-mcp/issue-250159.cjs`, run it with `--build`, and read the
192-
printed report path.
193-
4. Report the outcome per step, link `report.html`, and attach `videos/annotated.mp4`.
206+
4. Write `.build/vscode-playwright-mcp/issue-250159.cjs` and run it with no target flag, which uses
207+
the installed Insiders; read the printed report path.
208+
5. Report the outcome per step, link `report.html`, and attach `videos/annotated.mp4`.
194209

195210
The run fails at the search step, and that is the answer: the issue reproduces. Report it as a
196211
successful reproduction, not as a broken scenario.

test/automation/src/playwrightElectron.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ export async function launch(options: LaunchOptions): Promise<{ electronProcess:
3737
async function launchElectron(configuration: IElectronConfiguration, options: LaunchOptions) {
3838
const { logger, tracing, snapshots } = options;
3939

40+
// The recording canvas is fixed, but VS Code sizes its own window (1440x900
41+
// with a workspace, 1200x800 empty), so the capture would otherwise show the
42+
// window in the top-left corner of a larger frame. The window is resized to
43+
// match below, which also renders reliably when it is larger than the screen.
44+
const videoSize = { width: 1920, height: 1080 };
45+
4046
const playwrightImpl = options.playwright ?? playwright;
4147
let electron;
4248
try {
@@ -46,7 +52,7 @@ async function launchElectron(configuration: IElectronConfiguration, options: La
4652
recordVideo: options.videosPath
4753
? {
4854
dir: options.videosPath,
49-
size: { width: 1920, height: 1080 }
55+
size: videoSize
5056
} : undefined,
5157
env: configuration.env as { [key: string]: string },
5258
timeout: LAUNCH_TIMEOUT
@@ -63,6 +69,18 @@ async function launchElectron(configuration: IElectronConfiguration, options: La
6369
throw enrichLaunchError(error, options);
6470
}
6571
}
72+
if (options.videosPath) {
73+
try {
74+
await electron.evaluate(({ BrowserWindow }, size) => {
75+
const target = BrowserWindow.getAllWindows()[0];
76+
target?.setBounds({ x: 0, y: 0, width: size.width, height: size.height });
77+
}, videoSize);
78+
} catch (error) {
79+
// A mismatched window only wastes pixels in the recording, so never fail
80+
// a run because the window could not be resized.
81+
logger.log(`Playwright (Electron): Failed to size the window to the recording (${error})`);
82+
}
83+
}
6684
// Recording is per page, so sample the origin once the first window exists
6785
// rather than when the application finished launching.
6886
const videoStartedAt = options.videosPath ? Date.now() : undefined;

test/scenario/src/application.ts

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,61 @@ function parseQuality(): Quality {
9595
//
9696
// #### Electron ####
9797
//
98+
/**
99+
* Locate an installed VS Code Insiders, then Stable.
100+
*
101+
* Reproducing a reported issue is the common case, and that means running the
102+
* shipped product rather than a build from this checkout, so an installed build
103+
* is used when the caller did not choose a target.
104+
*/
105+
function findInstalledBuild(): string | undefined {
106+
const candidates: string[] = [];
107+
switch (process.platform) {
108+
case 'win32': {
109+
const roots = [process.env.LOCALAPPDATA, process.env.ProgramFiles, process.env['ProgramFiles(x86)']].filter((root): root is string => !!root);
110+
for (const root of roots) {
111+
candidates.push(path.join(root, 'Programs', 'Microsoft VS Code Insiders'), path.join(root, 'Microsoft VS Code Insiders'));
112+
}
113+
for (const root of roots) {
114+
candidates.push(path.join(root, 'Programs', 'Microsoft VS Code'), path.join(root, 'Microsoft VS Code'));
115+
}
116+
break;
117+
}
118+
case 'darwin':
119+
candidates.push(
120+
'/Applications/Visual Studio Code - Insiders.app',
121+
path.join(os.homedir(), 'Applications', 'Visual Studio Code - Insiders.app'),
122+
'/Applications/Visual Studio Code.app',
123+
path.join(os.homedir(), 'Applications', 'Visual Studio Code.app')
124+
);
125+
break;
126+
default:
127+
candidates.push('/usr/share/code-insiders', '/opt/visual-studio-code-insiders', '/usr/share/code', '/opt/visual-studio-code');
128+
break;
129+
}
130+
return candidates.find(candidate => {
131+
try {
132+
return fs.existsSync(candidate) && fs.existsSync(getBuildElectronPath(candidate));
133+
} catch {
134+
return false; // an incomplete install is not a usable target
135+
}
136+
});
137+
}
138+
98139
if (!opts.web) {
99140
let testCodePath = opts.build;
100141
let electronPath: string | undefined;
101142

143+
if (!testCodePath && !opts.dev) {
144+
testCodePath = findInstalledBuild();
145+
if (testCodePath) {
146+
// `getApplication` launches whatever `opts.build` names, so record the
147+
// choice there rather than only in this block.
148+
opts.build = testCodePath;
149+
logger.log(`No target given, using the installed build at ${testCodePath}. Pass --dev to run this checkout instead.`);
150+
}
151+
}
152+
102153
if (testCodePath) {
103154
electronPath = getBuildElectronPath(testCodePath);
104155
version = getBuildVersion(testCodePath);
@@ -111,7 +162,7 @@ if (!opts.web) {
111162
}
112163

113164
if (!fs.existsSync(electronPath || '')) {
114-
fail(`Cannot find VSCode at ${electronPath}. Please run VSCode once first (scripts/code.sh, scripts\\code.bat) and try again.`);
165+
fail(`Cannot find VS Code at ${electronPath}. Install VS Code Insiders, pass --build <app-root>, or build this checkout and pass --dev.`);
115166
}
116167

117168
quality = parseQuality();

test/scenario/src/options.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ export const opts = minimist(args, {
2020
'web',
2121
'headless',
2222
'video',
23-
'autostart'
23+
'autostart',
24+
'dev'
2425
],
2526
default: {
2627
verbose: false
@@ -31,6 +32,7 @@ export const opts = minimist(args, {
3132
headless?: boolean;
3233
web?: boolean;
3334
build?: string;
35+
dev?: boolean;
3436
browser?: 'chromium' | 'webkit' | 'firefox' | 'chromium-msedge' | 'chromium-chrome' | undefined;
3537
electronArgs?: string;
3638
video?: boolean;

test/scenario/src/renderEvidenceChapters.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -299,12 +299,24 @@ function wrap(value: string, limit: number, maxLines: number): string[] {
299299
return lines;
300300
}
301301

302-
if (require.main === module) {
302+
/**
303+
* Render captions without letting a presentation step fail a validation run.
304+
*
305+
* The raw recording is authoritative, so a missing or failing ffmpeg is reported
306+
* and otherwise ignored.
307+
*/
308+
export function tryRenderChapters(runRoot: string): void {
303309
try {
304-
renderChapters(path.resolve(process.argv[2] ?? process.env.RUN_ROOT ?? '.'));
310+
renderChapters(runRoot);
305311
} catch (error) {
306-
// Captions are a presentation aid, so never fail a validation run because
307-
// the recording could not be annotated. The raw recording is authoritative.
308-
console.warn(`Unable to render evidence captions: ${error instanceof Error ? error.message : error}`);
312+
const message = error instanceof Error ? error.message : String(error);
313+
const missingTool = /spawnSync (ffmpeg|ffprobe) ENOENT/u.exec(message);
314+
console.warn(missingTool
315+
? `Unable to render evidence captions: ${missingTool[1]} is not on PATH. Install ffmpeg and re-run this command; the raw recording is unaffected.`
316+
: `Unable to render evidence captions: ${message}`);
309317
}
310318
}
319+
320+
if (require.main === module) {
321+
tryRenderChapters(path.resolve(process.argv[2] ?? process.env.RUN_ROOT ?? '.'));
322+
}

test/scenario/src/runScenario.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,45 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6+
import { execFileSync } from 'child_process';
67
import type { Page } from '@playwright/test';
78
import * as path from 'path';
89
import type { Application, Code, Workbench } from '../../automation';
910
import { ApplicationService, JSONValue } from './application';
1011
import { EvidenceService } from './evidence';
11-
import { renderChapters } from './renderEvidenceChapters';
12+
import { tryRenderChapters } from './renderEvidenceChapters';
13+
14+
/**
15+
* Report missing video tooling before anything is launched.
16+
*
17+
* Captions are rendered after the run, so a missing ffmpeg is only discovered
18+
* once the scenario has already finished. Say so up front, with the command to
19+
* fix it, rather than letting the run complete and produce no annotated video.
20+
*/
21+
function checkVideoTooling(): void {
22+
const missing = ['ffmpeg', 'ffprobe'].filter(tool => {
23+
try {
24+
execFileSync(process.env[`${tool.toUpperCase()}_PATH`] ?? tool, ['-version'], { stdio: 'ignore' });
25+
return false;
26+
} catch {
27+
return true;
28+
}
29+
});
30+
if (!missing.length) {
31+
return;
32+
}
33+
const install = process.platform === 'win32'
34+
? 'winget install Gyan.FFmpeg'
35+
: process.platform === 'darwin'
36+
? 'brew install ffmpeg'
37+
: 'sudo apt install ffmpeg';
38+
console.warn(
39+
`Warning: ${missing.join(' and ')} not found on PATH, so the recording will not be captioned with step titles.\n` +
40+
` The run still produces the raw video, screenshots, trace and report.\n` +
41+
` To caption it, install ffmpeg (${install}), make sure it is on PATH, and re-run,\n` +
42+
` or annotate the finished run with: node test/scenario/out/renderEvidenceChapters.js <run-dir>`
43+
);
44+
}
1245

1346
/**
1447
* Runs a UI validation scenario end to end and writes an evidence bundle.
@@ -95,6 +128,7 @@ function loadScenario(scenarioPath: string): Scenario {
95128
}
96129

97130
export async function runScenario(scenario: Scenario): Promise<{ runPath: string; outcome: 'passed' | 'failed' | 'aborted' }> {
131+
checkVideoTooling();
98132
const appService = new ApplicationService();
99133
const evidence = new EvidenceService(appService);
100134
const runPath = await evidence.start(
@@ -151,7 +185,7 @@ export async function runScenario(scenario: Scenario): Promise<{ runPath: string
151185

152186
const reportPath = await evidence.finish(outcome, notes);
153187
console.log(`Report: ${reportPath}`);
154-
renderChapters(runPath);
188+
tryRenderChapters(runPath);
155189
return { runPath, outcome };
156190
}
157191

0 commit comments

Comments
 (0)