Address Copilot review comments

Author: hl662

core/electron/src/test/frontend/utils/backend.ts

@@ -6,7 +6,7 @@
 import { ElectronHost } from "../../../ElectronBackend";
 
 async function init() {
-  const cacheDir = process.env.ELECTRON_CACHE_DIR;
+  const cacheDir = process.env.VITEST_ELECTRON_CACHE_DIR ?? process.env.ELECTRON_CACHE_DIR;
   await ElectronHost.startup(cacheDir ? {
     iModelHost: {
       cacheDir,

tools/vitest-certa-bridge/README.md

@@ -58,7 +58,7 @@ The not-yet-supported areas are specifically `@vitest/browser` command APIs that
 
 The provider also starts with `supportsParallelism = false`. That keeps one Electron session active at a time while the callback bridge, preload composition, cache isolation, and teardown behavior are proven. Parallel sessions can be enabled later after adding dedicated coverage for concurrent BrowserWindows/processes and any shared backend callback state.
 
-Test-file isolation is still controlled by Vitest browser-mode config. The provider loads its preload into Vitest's tester iframe, so isolated test iframes are a supported direction, but each consuming package should validate its own setup/teardown assumptions when enabling or changing isolation.
+Test-file isolation is still controlled by Vitest browser-mode config. Vitest executes tests inside a same-origin tester iframe, so the provider loads its preload into subframes while keeping `nodeIntegration` disabled and `contextIsolation` enabled. Each consuming package should validate its own setup/teardown assumptions when enabling or changing isolation.
 
 ## Backend callbacks
 

tools/vitest-certa-bridge/src/electron/provider-session.ts

@@ -137,8 +137,9 @@ async function main() {
     webPreferences: {
       preload,
       nodeIntegration: false,
-      // Vitest browser mode executes test modules inside a tester iframe. Load the preload
-      // in subframes too so renderer tests can reach the callback bridge from that iframe.
+      // Vitest browser mode executes tests inside a same-origin tester iframe. Electron only
+      // loads preload scripts into that iframe when this option is enabled; nodeIntegration
+      // itself remains disabled above, so tests still rely on contextBridge-exposed APIs.
       nodeIntegrationInSubFrames: true,
       contextIsolation: true,
       sandbox: false,

tools/vitest-certa-bridge/src/electron/provider.ts

@@ -7,11 +7,13 @@ import { type ChildProcess, spawn } from "child_process";
 import * as fs from "fs";
 import * as os from "os";
 import * as path from "path";
+import { createRequire } from "module";
 import { playwright as PlaywrightBrowserProvider } from "@vitest/browser/providers";
 import type { BrowserProviderInitializationOptions, TestProject } from "vitest/node";
 import type { ElectronBrowserProviderOptions } from "./types.js";
 
 const playwrightBrowserProviderBase: any = PlaywrightBrowserProvider;
+const moduleRequire = createRequire(path.join(process.cwd(), "package.json"));
 
 interface ElectronSession {
   process: ChildProcess;
@@ -20,9 +22,8 @@ interface ElectronSession {
 
 /** Resolve the Electron provider main-process entry point from this package's compiled CJS output. */
 function getProviderSessionEntryPath(): string {
-  // __dirname is either lib/cjs/electron or lib/esm/electron. Electron main runs under CJS,
-  // so always target the CJS provider-session entry.
-  const pkgRoot = path.resolve(__dirname, "../../..");
+  const providerEntry = moduleRequire.resolve("@itwin/vitest-certa-bridge/electron-provider");
+  const pkgRoot = path.resolve(path.dirname(providerEntry), "../../..");
   return path.join(pkgRoot, "lib", "cjs", "electron", "provider-session.js");
 }
 
@@ -120,7 +121,7 @@ export default class ElectronBrowserProvider extends playwrightBrowserProviderBa
     }
 
     const cacheDir = fs.mkdtempSync(path.join(os.tmpdir(), `vitest-electron-${sessionId}-`));
-    const electronBin = require("electron/index.js");
+    const electronBin = moduleRequire("electron/index.js") as string;
     const sessionEntry = getProviderSessionEntryPath();
     const electronArgs = [...(this._options.electronArgs ?? []), sessionEntry];
 
@@ -134,6 +135,8 @@ export default class ElectronBrowserProvider extends playwrightBrowserProviderBa
       // eslint-disable-next-line @typescript-eslint/naming-convention
       VITEST_ELECTRON_CACHE_DIR: cacheDir,
       // eslint-disable-next-line @typescript-eslint/naming-convention
+      ELECTRON_CACHE_DIR: cacheDir,
+      // eslint-disable-next-line @typescript-eslint/naming-convention
       VITEST_ELECTRON_HEADLESS: String(this._project.config.browser.headless !== false),
     };
 
@@ -198,8 +201,8 @@ export default class ElectronBrowserProvider extends playwrightBrowserProviderBa
       electronProcess.on("exit", (code, signal) => {
         this._sessions.delete(sessionId);
         cleanupCacheDir(cacheDir);
-        if (!settled && code !== 0 && code !== null)
-          settleReject(new Error(`Electron provider session ${sessionId} exited before tests completed: code=${code}, signal=${signal ?? "none"}`));
+        if (!settled)
+          settleReject(new Error(`Electron provider session ${sessionId} exited before becoming ready: code=${code ?? "none"}, signal=${signal ?? "none"}`));
       });
     });
   }

tools/vitest-certa-bridge/src/test/electron-provider-smoke.test.ts

@@ -7,14 +7,29 @@ import { describe, expect, it, vi } from "vitest";
 
 type SendToBackend = (name: string, args: unknown[]) => Promise<unknown>;
 
+function getBridgeGlobal(name: string): unknown {
+  for (const candidateWindow of [window, window.parent, window.top]) {
+    try {
+      const candidate = (candidateWindow as unknown as Record<string, unknown> | null)?.[name];
+      if (candidate !== undefined)
+        return candidate;
+    } catch {
+      // Ignore inaccessible frames. Vitest's tester iframe is same-origin in this provider,
+      // but keep the helper defensive in case that changes.
+    }
+  }
+
+  return undefined;
+}
+
 function getSendToBackend(): SendToBackend {
-  const candidate = (window as unknown as Record<string, unknown>)._CertaSendToBackend;
+  const candidate = getBridgeGlobal("_CertaSendToBackend");
   expect(candidate).toBeTypeOf("function");
   return candidate as SendToBackend;
 }
 
 function getUserPreloadState() {
-  return (window as unknown as Record<string, unknown>).__electronProviderUserPreload;
+  return getBridgeGlobal("__electronProviderUserPreload");
 }
 
 describe("Electron Vitest browser provider", () => {