Skip to content

Commit 082f877

Browse files
voidstackloopclaude
andcommitted
fix(e2e): fake-llamacpp test hooks no longer reach the main process via require()
The whole e2e suite's llama.cpp-backed specs (8 of them) were failing in CI with "ReferenceError: require is not defined" -- confirmed real by reproducing locally with a headless Electron launch (no Xvfb needed to repro, though CI uses xvfb-run for the real suite). Playwright's ElectronApplication.evaluate() runs its callback in a bare V8 context with no `require` anywhere in scope, including via indirect eval -- so fake-llamacpp.ts's approach of re-require()-ing llamacpp-manager.ts's compiled output from outside the process, to reach its test-only setNextChatTurn/getChatRequestCount hooks, no longer works at all. Fixed by having llamacpp-manager.ts expose those hooks directly on globalThis, gated behind the exact same MODELFORGE_E2E_FAKE_LLAMACPP check that already gates the fake module swap itself -- a no-op in every real launch, same as before. fake-llamacpp.ts now just reads the global instead of require()-ing anything. Verified locally: all 25 e2e specs pass (temporarily launched Electron with --headless=new instead of relying on Xvfb, which isn't installable in this sandbox without root -- reverted that diagnostic tweak before committing). app's own 1018-test suite and frontend's 101-test suite both still pass after the llamacpp-manager.ts change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent e6b84e5 commit 082f877

2 files changed

Lines changed: 30 additions & 21 deletions

File tree

app/src/llamacpp-manager.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,25 @@ export function __getFakeLlamaCppLastContextSizeForTests(): number | "auto" | nu
121121
return fakeState.lastContextSize;
122122
}
123123

124+
// e2e/fixtures/fake-llamacpp.ts drives these test hooks from OUTSIDE this
125+
// process via Playwright's ElectronApplication.evaluate() — that callback
126+
// runs in a bare V8 execution context with no `require` in scope (Electron/
127+
// Playwright stopped exposing one; re-require()-ing this compiled file from
128+
// there throws "require is not defined"), so the hooks are exposed as a
129+
// global instead. Node's module cache still guarantees this is the exact
130+
// module instance chat-dispatch.ts already loaded — only *how* the outside
131+
// test process reaches it changed, not the "same instance" guarantee.
132+
// Gated the same as the fake module swap itself: absent in every real
133+
// launch, so this is a complete no-op outside an e2e run.
134+
if (process.env[FAKE_MODULE_ENV_VAR] === "1") {
135+
(globalThis as unknown as { __modelforgeFakeLlamaCppTestHooks?: unknown }).__modelforgeFakeLlamaCppTestHooks = {
136+
__setFakeLlamaCppNextChatTurnForTests,
137+
__getFakeLlamaCppChatRequestCountForTests,
138+
__getFakeLlamaCppContextCreationCountForTests,
139+
__getFakeLlamaCppLastContextSizeForTests,
140+
};
141+
}
142+
124143
function sleep(ms: number): Promise<void> {
125144
return new Promise((resolve) => setTimeout(resolve, ms));
126145
}

e2e/fixtures/fake-llamacpp.ts

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import * as path from "node:path";
21
import type { ElectronApplication } from "@playwright/test";
32

43
// llama.cpp runs in-process (node-llama-cpp, no spawned server, no HTTP
@@ -9,14 +8,11 @@ import type { ElectronApplication } from "@playwright/test";
98
// process's environment (see that file's own comment on why this is safe —
109
// nothing in a real launch ever sets that variable). This fixture's job is
1110
// just wiring: setting the env var at launch (via LaunchOptions.env), and
12-
// controlling the fake's next response from the Playwright test process by
13-
// re-requiring the same compiled module inside the already-running main
14-
// process via ElectronApplication.evaluate() — Node's require cache
15-
// guarantees that resolves to the exact same module instance
16-
// chat-dispatch.ts already loaded, not a second copy.
17-
18-
const APP_DIR = path.resolve(__dirname, "../../app");
19-
const LLAMACPP_MANAGER_JS = path.join(APP_DIR, "dist", "llamacpp-manager.js");
11+
// controlling the fake's next response from the Playwright test process via
12+
// ElectronApplication.evaluate() reading a global llamacpp-manager.ts
13+
// exposes for exactly this purpose (that callback runs in a bare V8 context
14+
// with no `require` in scope, so re-require()-ing the compiled module from
15+
// there — this fixture's old approach — no longer works).
2016

2117
/** Environment for launchApp({ env: FAKE_LLAMACPP_ENV }) to activate the fake
2218
* module. Combine with settings like `{ preferredRuntime: "llamacpp" }` /
@@ -41,20 +37,14 @@ export interface FakeLlamaCppController {
4137
export function controlFakeLlamaCpp(app: ElectronApplication): FakeLlamaCppController {
4238
return {
4339
async setNextChatTurn(turn: FakeLlamaCppChatTurn): Promise<void> {
44-
await app.evaluate(
45-
(_electron, args: { managerPath: string; turn: FakeLlamaCppChatTurn }) => {
46-
const manager = require(args.managerPath);
47-
manager.__setFakeLlamaCppNextChatTurnForTests(args.turn);
48-
},
49-
{ managerPath: LLAMACPP_MANAGER_JS, turn }
50-
);
40+
await app.evaluate((_electron, nextTurn: FakeLlamaCppChatTurn) => {
41+
(globalThis as unknown as { __modelforgeFakeLlamaCppTestHooks: { __setFakeLlamaCppNextChatTurnForTests(t: FakeLlamaCppChatTurn): void } }).__modelforgeFakeLlamaCppTestHooks.__setFakeLlamaCppNextChatTurnForTests(nextTurn);
42+
}, turn);
5143
},
5244
async getChatRequestCount(): Promise<number> {
53-
return app.evaluate((_electron, managerPath: string) => {
54-
// eslint-disable-next-line @typescript-eslint/no-var-requires
55-
const manager = require(managerPath);
56-
return manager.__getFakeLlamaCppChatRequestCountForTests();
57-
}, LLAMACPP_MANAGER_JS);
45+
return app.evaluate(() => {
46+
return (globalThis as unknown as { __modelforgeFakeLlamaCppTestHooks: { __getFakeLlamaCppChatRequestCountForTests(): number } }).__modelforgeFakeLlamaCppTestHooks.__getFakeLlamaCppChatRequestCountForTests();
47+
});
5848
},
5949
};
6050
}

0 commit comments

Comments
 (0)