mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
2be58a5cf1
## Summary Five issues found in the post-merge code review of #249 (Linux support). All confirmed present on main; none fixed by #194. ### H1 — `awaitFirstRealFrame` timeout leaves emulator running `awaitFirstRealFrame` is called after the cold-boot `try/catch` closes. On a SurfaceFlinger timeout the emulator process stayed alive: `inFlightBoots.finally` cleared the map entry, so the next `boot-device` call found the stale serial via the `alreadyRunning` fast path, had to run `assertScreencapAlive`, killed it there, and then fell through to another full cold boot — doubling latency and surfacing a confusing "wedged framebuffer" error instead of the original timeout. **Fix:** wrap the `awaitFirstRealFrame` call in a try/catch that calls `killEmulatorQuietly` before re-throwing. ### H2 — `awaitFirstRealFrame` budget not clamped against `overallDeadline` Every `attemptBoot` stage clamps via `Math.min(params.attemptDeadline, …)`. `awaitFirstRealFrame` received the fixed `STAGE_BUDGET.firstRealFrame` (90 s) with no knowledge of `overallDeadline`, allowing total wall time to silently exceed `bootTimeoutMs` by up to 90 s — violating the documented contract and the Zod schema's `max(900_000)`. **Fix:** pass `Math.min(STAGE_BUDGET.firstRealFrame, Math.max(0, overallDeadline - Date.now()))`. ### H3 — `ARGENT_SIMULATOR_SERVER_DIR` semantic changed without migration hint The env var previously pointed directly at the directory containing `simulator-server`; it now points at the root of the per-platform tree. A user with the old flat layout gets a bare "binary not found" with no explanation of what changed. **Fix:** detect the old flat layout and include a migration hint in the error message; update the env var's doc comment. ### M4 — `ARGENT_EMULATOR_NO_WINDOW=false` silently enables headless mode `selectExtraEmulatorArgs` used a deny-list (`!== "0"`) so `"false"` and `"no"` — natural shell idioms for "disabled" — both mapped to `-no-window`. Test suite covered `""`, `" "`, `"0"` but not `"false"`. **Fix:** switch to an allowlist `["1", "true", "yes"]`; add `"false"` and `"no"` to the parameterized off-test. ### M5 — `bootstrapDylibPathTcp` / `nativeDevtoolsDylibPathTcp` missing `requireDarwin` All six non-TCP dylib/binary exports gained explicit `requireDarwin()` guards in #249. The two TCP dylib exports did not. A Linux caller gets "dylib not found" instead of "requires macOS host". **Fix:** add `requireDarwin()` as the first statement in both TCP dylib functions; add a test. ### S7 (bonus) — `selectGpuMode` validation fires mid-function after slow I/O `selectGpuMode()` and `selectExtraEmulatorArgs()` were called inside deep nested code after several seconds of I/O. A typo in `ARGENT_EMULATOR_GPU_MODE` surfaced late with a spurious "emulator has been terminated" suffix appended. **Fix:** call both once at the top of `bootAndroidImpl`, just after `ensureDep`, and reuse the captured values — eliminates the late error and the redundant double-call. ## Test plan - `npm run test -w @argent/tool-server` — **847 passed** (was 845; +2 for `false`/`no` NO_WINDOW cases) - `npm run test -w @argent/native-devtools-ios` — **10 passed** (was 8; +2 for TCP dylib darwin gate and migration hint)
184 lines
8.5 KiB
TypeScript
184 lines
8.5 KiB
TypeScript
import * as fs from "node:fs";
|
|
import * as os from "node:os";
|
|
import * as path from "node:path";
|
|
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
|
// Static import primes vite's module graph so the dynamic re-import below
|
|
// resolves to the same node; we re-import via the SAME specifier after
|
|
// vi.resetModules() to reset captured env per-test.
|
|
import type * as ResolverModule from "../src/index";
|
|
|
|
// Unit tests for the resolver in this package. The resolver is what gates
|
|
// iOS-only binaries from being looked up on Linux callers, and what joins
|
|
// `process.platform` into the simulator-server bin path. Both behaviors
|
|
// only execute on resolver invocation, so we test the exported functions
|
|
// directly. We override ARGENT_NATIVE_DEVTOOLS_DIR /
|
|
// ARGENT_SIMULATOR_SERVER_DIR per-test so the resolver looks at a tmpdir we
|
|
// control rather than the real packages/native-devtools-ios layout.
|
|
|
|
let tmpRoot = "";
|
|
const originalPlatform = process.platform;
|
|
const originalDevtoolsDir = process.env.ARGENT_NATIVE_DEVTOOLS_DIR;
|
|
const originalSimulatorDir = process.env.ARGENT_SIMULATOR_SERVER_DIR;
|
|
|
|
function setPlatform(value: NodeJS.Platform) {
|
|
Object.defineProperty(process, "platform", { value, configurable: true });
|
|
}
|
|
|
|
beforeAll(() => {
|
|
tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), "argent-resolver-"));
|
|
});
|
|
|
|
afterAll(() => {
|
|
if (tmpRoot) fs.rmSync(tmpRoot, { recursive: true, force: true });
|
|
setPlatform(originalPlatform);
|
|
if (originalDevtoolsDir === undefined) delete process.env.ARGENT_NATIVE_DEVTOOLS_DIR;
|
|
else process.env.ARGENT_NATIVE_DEVTOOLS_DIR = originalDevtoolsDir;
|
|
if (originalSimulatorDir === undefined) delete process.env.ARGENT_SIMULATOR_SERVER_DIR;
|
|
else process.env.ARGENT_SIMULATOR_SERVER_DIR = originalSimulatorDir;
|
|
});
|
|
|
|
afterEach(() => {
|
|
setPlatform(originalPlatform);
|
|
});
|
|
|
|
/**
|
|
* Re-import the resolver after env / platform mutations. The module captures
|
|
* ARGENT_NATIVE_DEVTOOLS_DIR / ARGENT_SIMULATOR_SERVER_DIR at top-level on
|
|
* first load; vi.resetModules() clears vite's cache so the next import()
|
|
* re-executes the module body and picks up the current env. The dynamic
|
|
* specifier must match the static import above byte-for-byte so vite's
|
|
* module graph resolves to the same node.
|
|
*/
|
|
async function loadResolver(): Promise<typeof ResolverModule> {
|
|
vi.resetModules();
|
|
return await import("../src/index");
|
|
}
|
|
|
|
describe("requireDarwin gating", () => {
|
|
it("throws with a root-cause message on linux for iOS-only dylibs", async () => {
|
|
// A Linux caller asking for an iOS Simulator dylib should get the root
|
|
// cause ("requires a macOS host") not a misleading "file not found",
|
|
// which is what happened before this PR and led to confused bug reports.
|
|
setPlatform("linux");
|
|
const r = await loadResolver();
|
|
expect(() => r.bootstrapDylibPath()).toThrow(/requires a macOS host/);
|
|
expect(() => r.nativeDevtoolsDylibPath()).toThrow(/requires a macOS host/);
|
|
expect(() => r.keyboardPatchDylibPath()).toThrow(/requires a macOS host/);
|
|
});
|
|
|
|
it("throws with a root-cause message on linux for TCP iOS-only dylibs", async () => {
|
|
// TCP variants are equally iOS-Simulator-only; the guard must fire before
|
|
// the file-existence check so Linux callers get the platform error, not
|
|
// a misleading "dylib not found" for the tcp/ subdirectory.
|
|
setPlatform("linux");
|
|
const r = await loadResolver();
|
|
expect(() => r.bootstrapDylibPathTcp()).toThrow(/requires a macOS host/);
|
|
expect(() => r.nativeDevtoolsDylibPathTcp()).toThrow(/requires a macOS host/);
|
|
});
|
|
|
|
it("throws with a root-cause message on linux for ax-service", async () => {
|
|
// ax-service is darwin-only (runs INSIDE the iOS Simulator), so the
|
|
// resolver must throw the platform error BEFORE the file-existence check.
|
|
setPlatform("linux");
|
|
process.env.ARGENT_SIMULATOR_SERVER_DIR = tmpRoot;
|
|
const r = await loadResolver();
|
|
expect(() => r.axServiceBinaryPath()).toThrow(/requires a macOS host/);
|
|
});
|
|
|
|
it("throws with a root-cause message on linux for ax-service (tcp)", async () => {
|
|
setPlatform("linux");
|
|
process.env.ARGENT_SIMULATOR_SERVER_DIR = tmpRoot;
|
|
const r = await loadResolver();
|
|
expect(() => r.axServiceBinaryPathTcp()).toThrow(/requires a macOS host/);
|
|
});
|
|
});
|
|
|
|
describe("simulator-server path resolution", () => {
|
|
it("joins process.platform into the bin path", async () => {
|
|
// The whole point of the per-platform bin layout. We're verifying that
|
|
// the resolver constructs <BIN_ROOT>/<platform>/simulator-server, not
|
|
// <BIN_ROOT>/simulator-server.
|
|
const dir = fs.mkdtempSync(path.join(tmpRoot, "platform-join-"));
|
|
const platDir = path.join(dir, process.platform);
|
|
fs.mkdirSync(platDir, { recursive: true });
|
|
const binPath = path.join(platDir, "simulator-server");
|
|
fs.writeFileSync(binPath, "", { mode: 0o755 });
|
|
process.env.ARGENT_SIMULATOR_SERVER_DIR = dir;
|
|
const r = await loadResolver();
|
|
expect(r.simulatorServerBinaryPath()).toBe(binPath);
|
|
expect(r.simulatorServerBinaryDir()).toBe(platDir);
|
|
});
|
|
|
|
it("throws when the per-platform binary is missing", async () => {
|
|
// Empty per-platform dir: the resolver should fail loudly with both the
|
|
// platform name and the expected path in the message so the user can
|
|
// act on it (re-run the download script, switch hosts, etc).
|
|
const dir = fs.mkdtempSync(path.join(tmpRoot, "missing-"));
|
|
fs.mkdirSync(path.join(dir, process.platform), { recursive: true });
|
|
process.env.ARGENT_SIMULATOR_SERVER_DIR = dir;
|
|
const r = await loadResolver();
|
|
expect(() => r.simulatorServerBinaryPath()).toThrow(/simulator-server binary not found/);
|
|
expect(() => r.simulatorServerBinaryPath()).toThrow(new RegExp(process.platform));
|
|
});
|
|
|
|
it("includes a migration hint when ARGENT_SIMULATOR_SERVER_DIR points at the old flat layout", async () => {
|
|
// Prior to the Linux per-platform layout, ARGENT_SIMULATOR_SERVER_DIR
|
|
// was the directory that contained simulator-server directly. A user
|
|
// with that setup now gets a "not found" error; the migration hint in
|
|
// the message tells them exactly what changed and how to fix it.
|
|
const dir = fs.mkdtempSync(path.join(tmpRoot, "flat-layout-"));
|
|
fs.mkdirSync(path.join(dir, process.platform), { recursive: true });
|
|
// Place the binary at the flat (old) path.
|
|
const flatBin = path.join(dir, "simulator-server");
|
|
fs.writeFileSync(flatBin, "", { mode: 0o755 });
|
|
process.env.ARGENT_SIMULATOR_SERVER_DIR = dir;
|
|
const r = await loadResolver();
|
|
expect(() => r.simulatorServerBinaryPath()).toThrow(/old flat path/);
|
|
expect(() => r.simulatorServerBinaryPath()).toThrow(new RegExp(process.platform));
|
|
});
|
|
});
|
|
|
|
describe("ax-service path resolution", () => {
|
|
it("joins process.platform into the ax-service bin path", async () => {
|
|
if (originalPlatform !== "darwin") return;
|
|
const dir = fs.mkdtempSync(path.join(tmpRoot, "ax-platform-"));
|
|
const platDir = path.join(dir, "darwin");
|
|
fs.mkdirSync(platDir, { recursive: true });
|
|
const binPath = path.join(platDir, "ax-service");
|
|
fs.writeFileSync(binPath, "", { mode: 0o755 });
|
|
process.env.ARGENT_SIMULATOR_SERVER_DIR = dir;
|
|
const r = await loadResolver();
|
|
expect(r.axServiceBinaryPath()).toBe(binPath);
|
|
});
|
|
|
|
it("joins process.platform/tcp into the ax-service TCP bin path", async () => {
|
|
if (originalPlatform !== "darwin") return;
|
|
const dir = fs.mkdtempSync(path.join(tmpRoot, "ax-tcp-"));
|
|
const tcpDir = path.join(dir, "darwin", "tcp");
|
|
fs.mkdirSync(tcpDir, { recursive: true });
|
|
const binPath = path.join(tcpDir, "ax-service");
|
|
fs.writeFileSync(binPath, "", { mode: 0o755 });
|
|
process.env.ARGENT_SIMULATOR_SERVER_DIR = dir;
|
|
const r = await loadResolver();
|
|
expect(r.axServiceBinaryPathTcp()).toBe(binPath);
|
|
});
|
|
});
|
|
|
|
describe("ARGENT_NATIVE_DEVTOOLS_DIR override", () => {
|
|
it("uses the env-var dir for dylib resolution on darwin", async () => {
|
|
// The launcher script swaps the dylib dir at runtime via this env var
|
|
// (matches the ARGENT_SIMULATOR_SERVER_DIR pattern). Linux callers can't
|
|
// exercise the happy path because requireDarwin throws first, so this
|
|
// test only meaningfully runs on darwin — skip elsewhere rather than
|
|
// pretend to test it.
|
|
if (originalPlatform !== "darwin") return;
|
|
const dir = fs.mkdtempSync(path.join(tmpRoot, "dylib-override-"));
|
|
const dylib = path.join(dir, "libNativeDevtoolsIos.dylib");
|
|
fs.writeFileSync(dylib, "");
|
|
process.env.ARGENT_NATIVE_DEVTOOLS_DIR = dir;
|
|
setPlatform("darwin");
|
|
const r = await loadResolver();
|
|
expect(r.nativeDevtoolsDylibPath()).toBe(dylib);
|
|
});
|
|
});
|