Files
software-mansion__argent/packages/tool-server/test/linux-preflight.test.ts
Ignacy Łątka fa5af59931 refactor: replace hand-rolled parsers with standard packages (#373)
## What & why

Swept the repo for hand-rolled regex/string parsers that reimplement a
solved problem, and replaced the genuinely fragile ones with
battle-tested packages (or deps already in the tree). Each swap is
paired with a regression test for an input the old code got wrong.

| Site | Was | Now |
|------|-----|-----|
| `adb.ts` `resolveAvdPath`, `linux-preflight.ts` `readMb` | per-key
anchored regex over AVD `.ini` / `config.ini` | **`ini`** |
| `workspace-reader.ts` `extractEnvKeys` | split-on-first-`=` line loop
| **`dotenv`** (`parse`) |
| `uninstall.ts` `readBundledSkillName` | `^name:(.+)$` capture +
outer-quote strip | **`yaml`** (already a dep) |
| `mcp-configs.ts` `escapeRegExp` | hand-rolled char-class escape |
**`escape-string-regexp`** |
| `export.ts` xctrace version, `session-context.ts` RN minor |
`match(/(\d+)\./)` / `split(".")[1]` | **`semver`** `coerce` (already a
dep) |
| `source-resolver.ts` `parseDebugStack` | `at fn (file:l:c)` regex
(required both line+col) | **`stacktrace-parser`** |

### Bugs these fix (covered by new tests)
- **ini**: a quoted `path = "/Users/My Name/.../X.avd"` kept its quotes,
failed the `startsWith("/")` guard → AVD unresolved. Inline comments /
trailing junk after a sizing value made the anchored regex return `null`
→ sizing warning silently lost.
- **dotenv**: `export FOO=bar` produced the key `"export FOO"` (dropped
by the identifier filter); a multi-line quoted value leaked its
continuation lines (`FAKE=...`) as phantom keys.
- **yaml**: a `name:` value with a trailing `# comment` was taken
verbatim (comment included); odd quoting only had one outer pair
stripped.
- **stacktrace-parser**: a `:line`-only frame (no column, as Hermes/JSC
can emit) became an unusable `unknown` frame.

## Deliberately left alone (not "flaky", or no safe swap)

To avoid churn and regressions, I did **not** touch:
- **`uiautomator-parser.ts`** hand-rolled XML parser — a deliberate,
documented choice with a dedicated hardening suite, including a
*leniency* test for malformed dumps (stray `</node>`). A spec XML parser
would reject inputs this is built to recover from.
- **`ios-profiler/pipeline/xml-parser.ts`** (xctrace XML) — the most
tangled parser, but it has no real `.trace` fixtures and only thin
entity-decode tests. A rewrite can't be proven equivalent without
device-generated traces, so swapping it blind risks silently breaking
iOS CPU/hang/leak reports. Best done separately with a captured fixture.
- **`http.ts` `extractHostname`** — DNS-rebinding guard; `URL.hostname`
keeps the IPv6 brackets the loopback check compares without, so the
"fix" isn't simpler and the path is security-sensitive.
- **`adb.ts` `shellQuote`** — the POSIX single-quote escape is the
standard correct form.
- **`parseAdbDevices`, `android-screen.ts` `wm size`, `detect-app.ts`
dumpsys** — tool-specific CLI-output scraping with no clean package
equivalent (and documented edge-case handling).
- **`workspace-reader.ts` `extractMetroPort`** — a best-effort heuristic
over JS config; a correct replacement needs an AST parse, out of scope
here.
- **`config-parse.ts` Yarn/npm age-gate parsing** — a precise,
unit-tested domain parser; `ms` would introduce a bare-number ambiguity.

## Verification
- `tsc --build` clean across the workspace graph.
- Tests: tool-server **1205**, installer **264**, update-core **31** —
all pass (incl. the new regression tests).
- `prettier --check` clean on all changed files.
- Production esbuild bundle (`bundle-tools.cjs`) emits a valid
`tool-server.cjs`/`mcp-server.mjs` with
`ini`/`dotenv`/`stacktrace-parser` inlined (`node --check` OK). *(The
script's final step errored only on the auth-gated
`trace-processor.wasm` download, unrelated to bundling.)*

New runtime deps: `ini`, `dotenv`, `stacktrace-parser` (tool-server),
`escape-string-regexp` (installer) — all small, pure-JS. Lockfile change
is purely additive.
2026-06-25 13:29:10 +02:00

92 lines
3.9 KiB
TypeScript

// Unit tests for linuxBootDiagnostics. The KVM branch is small enough that
// we test the two deterministic shapes (null on non-linux, array on linux)
// and trust the try/catch from reading the code. AVD sizing is exercised
// via the pure `diagnoseAvdSizing` helper so we don't need fs mocking.
import { describe, it, expect, afterEach } from "vitest";
import { diagnoseAvdSizing, linuxBootDiagnostics } from "../src/utils/linux-preflight";
const originalPlatform = process.platform;
function setPlatform(p: NodeJS.Platform) {
Object.defineProperty(process, "platform", { value: p, configurable: true });
}
afterEach(() => setPlatform(originalPlatform));
describe("linuxBootDiagnostics", () => {
it("returns null on non-linux", () => {
setPlatform("darwin");
expect(linuxBootDiagnostics()).toBeNull();
});
it("returns an array on linux", () => {
setPlatform("linux");
const result = linuxBootDiagnostics();
expect(result).not.toBeNull();
expect(Array.isArray(result)).toBe(true);
// On a host where /dev/kvm is RW, the array is empty; where it isn't,
// it contains a single string with `/dev/kvm` somewhere in it. We
// accept either to make this test independent of the host's KVM
// state — the actual fs branches are simple enough that reading the
// code is more reliable than mocking node:fs in vitest.
for (const msg of result!) {
expect(msg).toMatch(/\/dev\/kvm/);
}
});
});
describe("diagnoseAvdSizing", () => {
const PATH = "/home/test/.android/avd/argent-test.avd/config.ini";
it("flags hw.ramSize below 4096 MB (the 2G avdmanager default)", () => {
// 2G is the avdmanager default; covers the "fresh avdmanager AVD wedges
// under real load" case that motivates the diagnostic. The path is
// echoed back so the user knows exactly which file to edit.
const out = diagnoseAvdSizing("argent-test", "hw.ramSize = 2G\nvm.heapSize = 512\n", PATH);
expect(out).toMatch(/undersized.*hw\.ramSize=2048 MB/);
expect(out).not.toMatch(/vm\.heapSize=/);
expect(out).toContain(PATH);
});
it("flags vm.heapSize below 512 MB (the 228 avdmanager default)", () => {
// 228 is the avdmanager default; isolates the heap-only branch so a
// regression that conflates ram vs heap parsing surfaces here.
const out = diagnoseAvdSizing("argent-test", "hw.ramSize = 4096\nvm.heapSize = 228M\n", PATH);
expect(out).toMatch(/undersized.*vm\.heapSize=228 MB/);
expect(out).not.toMatch(/hw\.ramSize=/);
});
it("emits a single combined warning when BOTH ram and heap are low", () => {
const out = diagnoseAvdSizing("argent-test", "hw.ramSize = 2G\nvm.heapSize = 228M\n", PATH);
expect(out).toMatch(/hw\.ramSize=2048 MB/);
expect(out).toMatch(/vm\.heapSize=228 MB/);
});
it("returns null when both ram and heap meet the floor", () => {
expect(
diagnoseAvdSizing("argent-test", "hw.ramSize = 4096\nvm.heapSize = 512\n", PATH)
).toBeNull();
});
it("returns null when the keys are absent — partial configs aren't a fail signal", () => {
// A config.ini that omits hw.ramSize / vm.heapSize means the emulator
// will use its own defaults; we can't know the value without invoking
// the emulator, so we stay silent rather than guess.
expect(diagnoseAvdSizing("argent-test", "hw.cpu.ncore = 4\n", PATH)).toBeNull();
});
it("tolerates an inline comment after the value", () => {
// `ini.parse` strips a trailing `# …` / `; …` comment. The previous
// per-key `…\\s*$`-anchored regex matched the whole line and returned
// null the moment anything followed the value, so an annotated config
// silently lost its sizing signal.
const out = diagnoseAvdSizing(
"argent-test",
"hw.ramSize = 2G # bumped from 1G\nvm.heapSize = 228M\n",
PATH
);
expect(out).toMatch(/hw\.ramSize=2048 MB/);
expect(out).toMatch(/vm\.heapSize=228 MB/);
});
});