Skip to content

Commit 6465d65

Browse files
committed
Add -control pipe for test automation
Rename test control flag to -dbg-control
1 parent 70fa4ae commit 6465d65

25 files changed

Lines changed: 968 additions & 300 deletions

agents.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,13 @@ Structure of each test (so they compose in tests/all.ts):
7171

7272
Guidelines for test scripts:
7373
- build the app the same way cmd/build.ts does (via `buildApp`/`runStandalone` in tests/util.ts) and test the resulting out/dbg64/SumatraPDF-dll.exe
74-
- if a needed external tool (e.g. MiKTeX) isn't installed, don't fail the test: print a clear message (with instructions to install it) and skip that part, returning normally so `tests/all.ts` continues
75-
- a good test fails when the fix is reverted (verify this) — not just passes with the fix present
76-
- bun has FFI; if you need to call Windows APIs, put reusable wrappers in tests/winapi.ts
77-
- prefer driving the app via cmd-line flags that write a machine-readable result (see `-test-synctex`) over GUI automation
78-
- never write runtime scratch / result files directly into `tests/` — that leaves the repo dirty. Write them under `tests/tmp/` (gitignored), using `tmpPath("name")` from `tests/util.ts` (it creates the dir on demand); the OS temp dir (`os.tmpdir()`) is also fine if you clean up after
79-
- if a binary test fixture (e.g. a .pdf) is generated from source (LaTeX, a script, etc.), commit the source alongside it (e.g. `tests/issue-<number>.tex` next to `tests/issue-<number>.pdf`) with a comment on how to regenerate it, so the fixture can be modified later
74+
- if a needed external tool (e.g. MiKTeX) isn't installed, don't fail the test: print a clear message (with instructions to install it) and skip that part, returning normally so `tests/all.ts` continues
75+
- a good test fails when the fix is reverted (verify this) — not just passes with the fix present
76+
- bun has FFI; if you need to call Windows APIs, put reusable wrappers in tests/winapi.ts
77+
- prefer driving the app through `-dbg-control <named-pipe>` and `cmd/control.ts` over GUI automation or adding new test-only command-line flags. Tests should pick a unique pipe name, launch `SumatraPDF-dll.exe -for-testing -dbg-control <name>`, send binary request/response commands, and quit the app through the control client.
78+
- `-dbg-control` protocol: requests are `[u32 payloadSize][u16 command][u16 requestId][args...]`; responses are `[u32 payloadSize][u16 requestId][results...]`. Arguments/results are encoded as `[u16 type]` where `0=end`, `1=i32` plus 4 bytes, `2=bytes` plus u32 length and data, `3=utf8 string` plus u32 length, bytes, and a zero terminator, and `4=list` plus u16 element count followed by encoded elements.
79+
- never write runtime scratch / result files directly into `tests/` — that leaves the repo dirty. Write them under `tests/tmp/` (gitignored), using `tmpPath("name")` from `tests/util.ts` (it creates the dir on demand); the OS temp dir (`os.tmpdir()`) is also fine if you clean up after
80+
- if a binary test fixture (e.g. a .pdf) is generated from source (LaTeX, a script, etc.), commit the source alongside it (e.g. `tests/issue-<number>.tex` next to `tests/issue-<number>.pdf`) with a comment on how to regenerate it, so the fixture can be modified later
8081

8182
## Windows Shell Safety
8283

cmd/build-with-mingw.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1642,6 +1642,7 @@ const sumatraFiles: FileGroup[] = [
16421642
"Selection.*",
16431643
"SettingsStructs.*",
16441644
"SimpleBrowserWindow.*",
1645+
"SumatraControl.*",
16451646
"SumatraPDF.cpp",
16461647
"SumatraStartup.cpp",
16471648
"SumatraConfig.cpp",

cmd/control.ts

Lines changed: 338 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,338 @@
1+
import { Socket, createConnection } from "node:net";
2+
3+
export enum ControlCommand {
4+
Ping = 1,
5+
Quit = 2,
6+
TestSynctex = 10,
7+
TestSearch = 11,
8+
TestDest = 12,
9+
TestNamedDest = 13,
10+
TestChm = 14,
11+
}
12+
13+
export type ControlArg = number | string | Uint8Array | ControlArg[];
14+
15+
const enum ArgType {
16+
End = 0,
17+
Int32 = 1,
18+
Bytes = 2,
19+
String = 3,
20+
List = 4,
21+
}
22+
23+
function appendU16(out: number[], v: number): void {
24+
out.push(v & 0xff, (v >>> 8) & 0xff);
25+
}
26+
27+
function appendU32(out: number[], v: number): void {
28+
out.push(v & 0xff, (v >>> 8) & 0xff, (v >>> 16) & 0xff, (v >>> 24) & 0xff);
29+
}
30+
31+
function appendBytes(out: number[], bytes: Uint8Array): void {
32+
for (const b of bytes) {
33+
out.push(b);
34+
}
35+
}
36+
37+
function encodeArg(out: number[], arg: ControlArg): void {
38+
if (typeof arg === "number") {
39+
appendU16(out, ArgType.Int32);
40+
appendU32(out, arg | 0);
41+
return;
42+
}
43+
if (typeof arg === "string") {
44+
const bytes = new TextEncoder().encode(arg);
45+
appendU16(out, ArgType.String);
46+
appendU32(out, bytes.length);
47+
appendBytes(out, bytes);
48+
out.push(0);
49+
return;
50+
}
51+
if (Array.isArray(arg)) {
52+
appendU16(out, ArgType.List);
53+
appendU16(out, arg.length);
54+
for (const el of arg) {
55+
encodeArg(out, el);
56+
}
57+
return;
58+
}
59+
appendU16(out, ArgType.Bytes);
60+
appendU32(out, arg.byteLength);
61+
appendBytes(out, arg);
62+
}
63+
64+
function encodeRequest(cmd: number, id: number, args: ControlArg[]): Buffer {
65+
const payload: number[] = [];
66+
appendU16(payload, cmd);
67+
appendU16(payload, id);
68+
for (const arg of args) {
69+
encodeArg(payload, arg);
70+
}
71+
appendU16(payload, ArgType.End);
72+
73+
const packet: number[] = [];
74+
appendU32(packet, payload.length);
75+
packet.push(...payload);
76+
return Buffer.from(packet);
77+
}
78+
79+
class PacketReader {
80+
pos = 0;
81+
82+
constructor(readonly data: Buffer) {}
83+
84+
u16(): number {
85+
const v = this.data.readUInt16LE(this.pos);
86+
this.pos += 2;
87+
return v;
88+
}
89+
90+
u32(): number {
91+
const v = this.data.readUInt32LE(this.pos);
92+
this.pos += 4;
93+
return v;
94+
}
95+
96+
i32(): number {
97+
const v = this.data.readInt32LE(this.pos);
98+
this.pos += 4;
99+
return v;
100+
}
101+
102+
bytes(len: number): Buffer {
103+
const v = this.data.subarray(this.pos, this.pos + len);
104+
this.pos += len;
105+
return v;
106+
}
107+
}
108+
109+
function decodeArg(r: PacketReader): ControlArg | undefined {
110+
const type = r.u16();
111+
if (type === ArgType.End) {
112+
return undefined;
113+
}
114+
if (type === ArgType.Int32) {
115+
return r.i32();
116+
}
117+
if (type === ArgType.Bytes) {
118+
return r.bytes(r.u32());
119+
}
120+
if (type === ArgType.String) {
121+
const len = r.u32();
122+
const bytes = r.bytes(len);
123+
const zero = r.bytes(1)[0];
124+
if (zero !== 0) {
125+
throw new Error("invalid control string terminator");
126+
}
127+
return new TextDecoder().decode(bytes);
128+
}
129+
if (type === ArgType.List) {
130+
const n = r.u16();
131+
const list: ControlArg[] = [];
132+
for (let i = 0; i < n; i++) {
133+
const arg = decodeArg(r);
134+
if (arg === undefined) {
135+
throw new Error("unexpected end marker in control list");
136+
}
137+
list.push(arg);
138+
}
139+
return list;
140+
}
141+
throw new Error(`unknown control argument type ${type}`);
142+
}
143+
144+
function pipePath(pipeName: string): string {
145+
return pipeName.startsWith("\\\\.\\pipe\\") ? pipeName : `\\\\.\\pipe\\${pipeName}`;
146+
}
147+
148+
async function readExactly(socket: Socket, len: number): Promise<Buffer> {
149+
const chunks: Buffer[] = [];
150+
let total = 0;
151+
while (total < len) {
152+
const chunk = socket.read(len - total) as Buffer | null;
153+
if (chunk) {
154+
chunks.push(chunk);
155+
total += chunk.length;
156+
continue;
157+
}
158+
await waitForReadable(socket);
159+
}
160+
return Buffer.concat(chunks, total);
161+
}
162+
163+
function waitForReadable(socket: Socket): Promise<void> {
164+
return new Promise((resolve, reject) => {
165+
const cleanup = () => {
166+
socket.off("readable", onReadable);
167+
socket.off("end", onEnd);
168+
socket.off("close", onClose);
169+
socket.off("error", onError);
170+
};
171+
const onReadable = () => {
172+
cleanup();
173+
resolve();
174+
};
175+
const onEnd = () => {
176+
cleanup();
177+
reject(new Error("control pipe closed while reading"));
178+
};
179+
const onClose = () => {
180+
cleanup();
181+
reject(new Error("control pipe closed while reading"));
182+
};
183+
const onError = (err: Error) => {
184+
cleanup();
185+
reject(err);
186+
};
187+
socket.once("readable", onReadable);
188+
socket.once("end", onEnd);
189+
socket.once("close", onClose);
190+
socket.once("error", onError);
191+
});
192+
}
193+
194+
function connectSocket(path: string): Promise<Socket> {
195+
return new Promise((resolve, reject) => {
196+
const socket = createConnection(path);
197+
const cleanup = () => {
198+
socket.off("connect", onConnect);
199+
socket.off("error", onError);
200+
};
201+
const onConnect = () => {
202+
cleanup();
203+
resolve(socket);
204+
};
205+
const onError = (err: Error) => {
206+
cleanup();
207+
socket.destroy();
208+
reject(err);
209+
};
210+
socket.once("connect", onConnect);
211+
socket.once("error", onError);
212+
});
213+
}
214+
215+
function cleanEnv(env: Record<string, string | undefined> | undefined): Record<string, string> | undefined {
216+
if (!env) {
217+
return undefined;
218+
}
219+
const res: Record<string, string> = {};
220+
for (const [key, value] of Object.entries(env)) {
221+
if (value !== undefined) {
222+
res[key] = value;
223+
}
224+
}
225+
return res;
226+
}
227+
228+
export class ControlClient {
229+
private nextId = 1;
230+
231+
constructor(readonly socket: Socket) {}
232+
233+
static async connect(pipeName: string, timeoutMs = 10000): Promise<ControlClient> {
234+
const path = pipePath(pipeName);
235+
const deadline = Date.now() + timeoutMs;
236+
let lastErr: unknown;
237+
while (Date.now() < deadline) {
238+
try {
239+
const socket = await connectSocket(path);
240+
return new ControlClient(socket);
241+
} catch (e) {
242+
lastErr = e;
243+
await new Promise((resolve) => setTimeout(resolve, 50));
244+
}
245+
}
246+
throw new Error(`failed to connect to Sumatra control pipe ${path}: ${lastErr}`);
247+
}
248+
249+
async request(cmd: ControlCommand, args: ControlArg[] = []): Promise<ControlArg[]> {
250+
const id = this.nextId++ & 0xffff;
251+
this.socket.write(encodeRequest(cmd, id, args));
252+
253+
const sizeBuf = await readExactly(this.socket, 4);
254+
const size = sizeBuf.readUInt32LE(0);
255+
const payload = await readExactly(this.socket, size);
256+
const r = new PacketReader(payload);
257+
const responseId = r.u16();
258+
if (responseId !== id) {
259+
throw new Error(`control response id mismatch: got ${responseId}, expected ${id}`);
260+
}
261+
const result: ControlArg[] = [];
262+
for (;;) {
263+
const arg = decodeArg(r);
264+
if (arg === undefined) {
265+
break;
266+
}
267+
result.push(arg);
268+
}
269+
return result;
270+
}
271+
272+
async quit(): Promise<void> {
273+
await this.request(ControlCommand.Quit);
274+
}
275+
276+
close(): void {
277+
this.socket.end();
278+
}
279+
}
280+
281+
export function uniquePipeName(prefix = "sumatra-control"): string {
282+
return `${prefix}-${process.pid}-${Date.now()}-${Math.floor(Math.random() * 0x100000000).toString(16)}`;
283+
}
284+
285+
export async function withControlledSumatra<T>(
286+
exe: string,
287+
fn: (client: ControlClient) => Promise<T>,
288+
extraArgs: string[] = [],
289+
options: { cwd?: string; env?: Record<string, string | undefined>; connectTimeoutMs?: number } = {},
290+
): Promise<T> {
291+
const pipeName = uniquePipeName();
292+
const proc = Bun.spawn([exe, "-for-testing", "-dbg-control", pipeName, ...extraArgs], {
293+
stdout: "ignore",
294+
stderr: "ignore",
295+
cwd: options.cwd,
296+
env: cleanEnv(options.env),
297+
});
298+
let client: ControlClient | undefined;
299+
try {
300+
client = await ControlClient.connect(pipeName, options.connectTimeoutMs ?? 10000);
301+
return await fn(client);
302+
} finally {
303+
if (client) {
304+
try {
305+
await client.quit();
306+
} catch {
307+
proc.kill();
308+
}
309+
client.close();
310+
} else {
311+
proc.kill();
312+
}
313+
await proc.exited;
314+
}
315+
}
316+
317+
export async function runControlCommand(
318+
exe: string,
319+
cmd: ControlCommand,
320+
args: ControlArg[] = [],
321+
extraArgs: string[] = [],
322+
): Promise<ControlArg[]> {
323+
return await withControlledSumatra(exe, (client) => client.request(cmd, args), extraArgs);
324+
}
325+
326+
if (import.meta.main) {
327+
const [exe, cmdName, ...args] = process.argv.slice(2);
328+
if (!exe || !cmdName) {
329+
console.log("Usage: bun cmd/control.ts <SumatraPDF-dll.exe> <ping|test-search> [args...]");
330+
process.exit(1);
331+
}
332+
const cmd = cmdName === "ping" ? ControlCommand.Ping : cmdName === "test-search" ? ControlCommand.TestSearch : 0;
333+
if (!cmd) {
334+
throw new Error(`unknown command: ${cmdName}`);
335+
}
336+
const res = await runControlCommand(exe, cmd, args);
337+
console.log(JSON.stringify(res));
338+
}

cmd/gen-flags.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,7 @@ const args = [
8383
"SetColorRange", "set-color-range",
8484
"UpgradeFrom", "upgrade-from",
8585
"ForTesting", "for-testing",
86-
"TestSynctex", "test-synctex",
87-
"TestSearch", "test-search",
88-
"TestDest", "test-dest",
89-
"TestNamedDest", "test-named-dest",
90-
"TestChm", "test-chm",
86+
"Control", "dbg-control",
9187
];
9288

9389
function generateCode(): string {

docs/md/Command-line-arguments.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ Anything that is not recognized as a known option is interpreted as a file path
1616
- `-appdata <directory>` : set custom directory where we'll store `SumatraPDF-settings.txt` file and thumbnail cache
1717
- `-restrict` : runs in restricted mode where you can disable features that require access to file system, registry and the internet. Useful for kiosk-like usage. Read more detailed documentation.
1818
- `-for-testing` : for ad-hoc testing by humans or agents. Always starts a new instance, doesn't restore a session (only loads files given on the command line) and doesn't save settings (**ver 3.7+**)
19+
- `-dbg-control <named-pipe>` : starts a test control server on a named pipe. Used by automated tests through `cmd/control.ts`; combine with `-for-testing`. (**ver 3.7+**)
1920
- `-pwd <password>` : use the given password to open password-protected documents. If the password is wrong, SumatraPDF falls back to default passwords and then asks interactively.
2021

2122
## Navigation options

docs/md/Version-history.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ Available in [pre-release](https://www.sumatrapdfreader.org/prerelease) builds.
2121
- [external viewer](Customize-external-viewers.md) command lines now support `%%` as a literal `%`, so e.g. `%%d` is passed to the program as `%d` (fixes #5583)
2222
- with `ReuseInstance`, opening a file now reuses a window on the current virtual desktop (or opens a new window there) instead of switching to a window on another desktop (fixes #5630)
2323
- add `-for-testing` [cmd-line argument](Command-line-arguments.md) for ad-hoc testing: always starts a new instance, doesn't restore a session, doesn't save settings
24+
- add `-dbg-control` [cmd-line argument](Command-line-arguments.md) and `cmd/control.ts` to drive automated tests over a named-pipe request/response protocol
2425
- add `-pwd` [cmd-line argument](Command-line-arguments.md) for opening password-protected documents from the command line (fixes #906)
2526
- add Back / Forward navigation buttons to the toolbar; navigation history now also records views you scrolled to and stayed on, not just table of contents / link jumps
2627
- add `CmdRemoveDeletedFilesFromHistory` (`Remove Deleted Files From History` in `Ctrl + k` [command palette](Command-Palette.md)) to remove history entries for files that no longer exist on disk

0 commit comments

Comments
 (0)