Files
software-mansion__argent/packages/tool-server/test/native-profiler-analyze-failure.test.ts
Ignacy Łątka 5d8fc10799 test: stop tests binding machine-wide resources they do not own (#714)
## Why

Three flaky or failing tests were diagnosed separately this week, and
all three turned out to be one defect wearing different clothes: **a
test binding a machine-wide resource it does not own**, so a second
concurrent run — another checkout, a CI matrix cell, a developer's own
app — perturbs it. Those three are fixed in #707, #709 and #711.

Each of those was found by chasing a symptom, and each audit covered
only its own resource type. This is the systematic sweep: all **448 test
files across 13 packages**, plus the 4 `node --test` scripts. (The sweep
itself ran over the 400 files that existed when it started; merging
`main` brought the count to 448, and every category below was re-run
over the full set — 25 files assign `env.HOME` and none omit
`USERPROFILE`, one test file calls `pkill`/`pgrep`
(`vega-cli-timeout.test.ts`, scoped out to #711 below), and no
`listen()` names a fixed port.)

## Findings

| Category | Files checked | Real findings |
|---|---|---|
| Network endpoints (bind / probe / unix socket) | 448 | 4 |
| Filesystem — shared temp namespace (`os.tmpdir()` and literal `/tmp`)
| 448 | 8 |
| Filesystem — real `$HOME` / `~/.argent` | 448 | 4 (28 files) |
| Ambient environment | 448 + all `src` readers | 12 |
| Device / emulator / SDK state | 448 | 2 |
| Process table (`pgrep` / `pkill` / fixed-pattern sweeps) | 448 | **0**
|
| In-process globals leaking across files | 13 packages × 13 modes |
**0** |

Every finding was reproduced with a concrete instrument before being
touched, and the instrument re-run after. Representative:

- **`stop-tools.test.ts`** ran the real `stop-metro` against port 59999,
which `lsof`s and **SIGTERMs whatever it finds**. With a squatter there:
`16 passed` → `1 failed`, and the squatter logged `received SIGTERM #1`.
Now port 1.
- **`bind-failure-telemetry` / `lens-relay-telemetry`** were gated on
the real `~/.argent/flags.json`. Enabling the documented
`tool-server-event-log` flag: `4 passed` → **`4 failed`**, and records
were appended to the developer's real event log.
- **LogFileWriter (12 files)** — its constructor `mkdir -p`s
`os.homedir()/.argent/tmp`. Instrument: run each of the 16 test files
that reference a writer, or a blueprint that builds one, under a scratch
`HOME`/`USERPROFILE`, then test `$HOME/.argent/tmp` for existence. 4
created it; the 12 that actually construct a writer now all call
`scopeTempHome`, and none creates it.
- **`screen-recording.test.ts`** — the device ids are fixtures and
`vi.useFakeTimers()` seeds its clock from the real time, so only the
millisecond a run starts in separates two runs' `.mp4` paths, and each
test deletes the path it derived. Self-concurrent: **2 of 3 rounds
failed** → 0 of 4. Scoping `TMPDIR` removes the window rather than
narrowing it.
- **`DO_NOT_TRACK=1`** → telemetry `282 passed` → **`13 failed`**.
**`npm_config_user_agent=pnpm|yarn|bun`** → installer `535 passed` →
4/5/3 failed. **19 tests required a real `adb`** — PATH shadow + empty
HOME: `30 passed` → **`19 failed`**.
- A fixed `/tmp/argent-nd-<udid8>.sock` **destroyed a concurrent
holder's live socket** (proven by inode) in **3** files that run the
native-devtools factory. Two of them also fail outright:
`native-devtools-app-state` lost **3 of 3** rounds of two concurrent
runs on `expected 'unregistered' to be 'connected'`. All three now
derive the UDID from `process.pid`.
- `flow-visual`, `react-profiler/dump`, `file-inputs`, `http-upload` —
shared `/tmp` paths and prefix scans, each reproduced with a squatter.
In `http-upload` the collision surfaced *as a timeout*, inside
`vi.waitFor`. (`flow-chromium-boot` was fixed on this branch too, but
main's #585 rewrote that file afterwards and the merge kept main's copy,
so it is byte-identical to `origin/main` here and carries no fix.)

## Clean, with the evidence

- **Process table** — an `LD_PRELOAD` exec interposer over a full run
(507 execs) plus all 12 other suites. Excluding
`vega-cli-timeout.test.ts`, which is #711's and does sweep the table
(`pkill -f '^sleep 6910[0-9]$'` at line 134): no
`pgrep`/`pkill`/`killall`/fixed-pattern sweep anywhere, and **no test
signals a pid it did not spawn**.
- **Real device state** — **zero** exec attempts of real `adb` /
`simctl` / `emulator` / `avdmanager` / `vega`; no test asserts a device
list. That list was too narrow: `workspace-reader-integration` spawned
the real `yarn` / `pnpm` / `bun` / `pod` / `eas` / `expo`, which is the
second finding in this row.
- **Cross-file in-process leakage** — structurally impossible: vitest
4.1.9 resolves `pool: "forks"` / `isolate: true`, confirmed by a scratch
experiment (distinct pids, no env leak, module `Map` = 1,1,1). 120
default-mode runs across 12 packages produced 1 failure.
- **`scripts/`** — 4 files, 86 tests: `mkdtemp` throughout, no ports, no
sweeps, writes nothing.

## Verification

Each fix was mutated back and killed by a **named** test under its
instrument — e.g. `× reads a plain absolute path`, `× returns null when
only the extensionless binary exists`, `× pins both names os.homedir()
consults, at the same directory`. Restores verified with `git diff` /
`git status`, never a substring grep.

**Pinned by an instrument only:** the socket fix for
`ios-only-blueprint-gate`, the `~/.argent/tmp` and
`~/.argent/flags.json` fixes, the `/tmp` fixture cleanup, the
`stop-metro` port change (the test asserts `stopped: false` / `pids:
[]`, which hold for either port whenever nothing is listening) and the
`workspace-reader` `PATH` pin (only `node` is asserted; the other seven
probes are not) are not pinned by any suite assertion — reverting them
still passes, and only the external squatter / scratch-`HOME`
instruments catch them. They are pollution fixes. The
`native-devtools-app-state` socket fix is the exception in the other
direction: reverting it turns two concurrent runs red.

Gates, unpiped and read by exit code: `eslint .` 0, `prettier --check .`
0, `tsc --build` 0, `typecheck:tests` 0.
Suite, run as six shards: tool-server **4287 passed, 1 skipped, 0
failed**; argent-cli 462; telemetry 306; argent-tools-client 191;
configuration-core 122; argent-mcp 80; argent-installer 555. All other
packages and `test:scripts` green, identical to baseline.

`lens-tools-platform-gate.test.ts` times out at the 5s default under
full-suite load and passes at `--testTimeout=30000` (24.7s, almost all
of it transform). It predates this branch and is untouched by it.

## Scope

This branch is **test-only** — no `src/` file is touched. Both
production defects the sweep turned up are separated out, so each can be
reviewed as the user-facing fix it is: `ps` without `-ww` orphaning live
tool-servers is #713, and #852 is `ensureToolsServer` respawning on
every call when `ARGENT_HOST` is exported — `buildToolsServerEnv` passes
the inherited value to the child while the state records the literal
`127.0.0.1` the health check then probes. The second surfaced here as
eight stranded `fake-tool-server` processes, and pinning the environment
in this branch removes that signal, which is why it is filed rather than
merely worked around. A third, outside this branch entirely, is #853:
ten `tool-server` test servers omit the host on `listen(0)`, binding
every interface where twenty-two sibling sites pass `127.0.0.1`. It is
hardening, not a live exposure — `createHttpApp`'s Host allow-list
answers a LAN-addressed request with 403, and only a raw client forging
`Host: 127.0.0.1` reaches the stub registry behind it, inside the ~100
ms one `it()` holds the socket. It is also not a collision risk:
ephemeral ports are kernel-assigned and never reissued while in use,
which is why the fixed-port audit above is unaffected. All ten predate
this branch.

Left untouched on purpose: `boot-electron-spawn-error.test.ts`,
`chromium-discovery.test.ts` and `vega-cli-timeout.test.ts` — owned by
the still-open #709 and #711, and all three still carrying their defects
on this base. (#707 has since merged into this base, so
`boot-device-hotboot.test.ts` and `vitest.config.ts` carry its fix
rather than a defect; `test/setup/clear-argent-env.ts` is its work, and
this branch relies on it.)

**Not fixed, deliberately** — an adjacent class (test-*order*
dependence, not machine-global resources), reproducible if wanted:
`link-config.test.ts` (deterministic: `-t "returns null when required
field"` → 4 failed), `network/network-integration.test.ts` and
`android-perfetto/dispatch.test.ts` (fail under `--sequence.shuffle`
seeds).

**Also observed, not addressed:** one green tool-server run leaves
**56** entries behind in a scratch `TMPDIR` — **30** in the
`argent-screenshot-diff*` family, 4 `argent-events-*`, 3
`argent-flow-crop-*`, 3 `argent-tar-upload-*`, 2 `argent-flow-diff-*`, 2
`argent-file-input-*`, and 12 assorted singletons (`ast-index-*`,
`component-source-*`, `argent-profiler-cwd`, `argent-chromium-media`,
…). Measured per run against a scoped temp dir, so it ranks the
contributors rather than counting one machine's accumulated history —
screenshot-diff, not flow-crop, is the dominant one. A separate leak.
That instrument is blind to a *literal* path, which is how it missed the
two `/tmp/native-profiler-*-report.md` writers the fourth pass found.
Re-run across all 13 packages, `argent-mcp` is the only one outside
`tool-server` that leaves anything — two entries per run, filed as #854.


## Second pass

A review of this branch re-ran every category above with its own
instrument rather than reading the diff, and found the sweep had pinned
four resources in one file while leaving a sibling that does the same
thing live. Fixed in `4289e3d8f` and `8164955b4`; the table and bullets
above are the corrected totals.

- **`tool-server-event-log`** was mocked off in 2 of the 4 files that
call the real `start()`. With the flag on, `startup-telemetry` and
`tool-fail-invalid-params-emission` failed 3 tests in
`attachRegistryEventLogger` and truncated the real
`~/.argent/tool-server-events.jsonl` to 0 bytes first.
`ARGENT_EVENT_LOG` cannot redirect that:
`test/setup/clear-argent-env.ts` strips every `ARGENT_*` var before the
module graph loads.
- **`scopeTempHome`** reached 8 files; 4 more built a real
`LogFileWriter` through the JS-runtime-debugger blueprint.
- **`os.homedir()` on Windows** reads `USERPROFILE`, not `HOME`.
`android-binary-windows` and `adb-resolve-avd-path` pinned only `HOME`,
and both run on a real windows-latest host. Proven by running the
resolver under a `node:os` mock that reproduces the Windows lookup: with
`HOME` alone the stock Studio SDK under the profile resolves; with both
pinned it misses. `update-checker` had the same gap.
- **`getConsentState`** honours a falsy `ARGENT_TELEMETRY` at the same
precedence as `DO_NOT_TRACK`, ahead of the config override the suite
writes. Only `DO_NOT_TRACK` was pinned: `ARGENT_TELEMETRY=0` gave `17
failed | 6 passed`.
- **`workspace-reader-integration`** let the version probe spawn the
real package managers off `PATH`; their shims self-install, so the file
pulled **~44 MB** into `~/.cache/node/corepack` and
`~/Library/Caches/eas-cli`. `PATH` is now a directory holding only
`node`, the one version the test asserts — 44 MB → 4 KB.
- **`argent-mcp`** snapshotted `ARGENT_AUTO_SCREENSHOT_DELAY_MS` without
clearing it, and that package has no `ARGENT_*`-stripping setup file, so
3 tests asserted the developer's exported value.

Smaller items in the same commits: the `/tmp` socket fixtures now remove
the directories they create per run; `http-upload`'s pre-scan snapshot
is gone, since scoping `TMPDIR` to a fresh directory made it permanently
empty and its filter inert, as its two siblings already did; and two
`detectProjectPackageManager` assertions that enumerated the entire
return union are pinned to a single value.


The confirmation pass also caught a defect the first round of fixes
introduced: pinning `HOME`/`USERPROFILE` to the fixture root in
`android-binary-windows` let `<home>/Android/Sdk` shadow the
`%LOCALAPPDATA%` root the two positive tests are named after, so
deleting `roots.push(join(localAppData, …))` from the resolver left all
three green. Home has its own directory in `6aee3ee7a`, and that
mutation now fails both tests.


A third pass, running the nearest-twin move repo-wide, found the
`os.homedir()`-on-Windows gap in one more package: eight
`argent-tools-client` test files redirect only `HOME`, and four of them
recursively delete `STATE_DIR` — two before each test, two after — which
on Windows is the developer's real `%USERPROFILE%\.argent`. Fixed in
`139447d4a`, together with the same gap in `avd-snapshot`. Nothing runs
that package on Windows CI today, so no current run fires it.

## Fourth pass

A fourth pass audited the three fix commits and re-ran the sweep with
two instruments the earlier ones did not have. Both holes were in the
*measurement*, not the diff — which is why three passes missed what they
found. Fixed in `a67c43f8d`.

- **`os.tmpdir()` does not read the same variable on every platform.**
POSIX takes `TMPDIR`, `TMP`, `TEMP` in that order; Windows takes `TEMP`,
then `TMP`, and never looks at `TMPDIR`. The five pins this branch added
set `TMPDIR` alone, so on Windows production still materialized into the
machine-wide `%TEMP%` while the test scanned a scratch directory nothing
ever wrote to — and `file-inputs`, `http-upload` and `flow-visual` each
end in a leak assertion over that listing, which then passes whatever
the code does. Against `origin/main`, which diffed a before/after
listing of the real tmpdir, that is a regression rather than a wash.
`redirectTmpdir` (`test/helpers/tmpdir-env.ts`) sets all three and
returns the restore; the **7** files that scope a tmpdir use it,
including the two `android-perfetto` ones that predate this branch.
`test/tmpdir-env.test.ts` pins both lookup chains — reverting the helper
to `TMPDIR` alone fails two of its three tests. All three leak
assertions were re-confirmed live on POSIX by mutating the production
cleanup each one covers.
- **A hardcoded absolute path is invisible to a `TMPDIR`-scoped
instrument.** `session-correctness` and
`ios-instruments/analyze-freshness` passed a literal
`/tmp/native-profiler-{x,ios}.trace` as the session's trace file, and
the renderer writes its report beside it (`deriveReportPath` =
`dirname(traceFile)/<base>-report.md`). Every run left
`native-profiler-x-report.md` and `native-profiler-ios-report.md` in the
shared `/tmp`; nothing removed them, and two concurrent runs write the
same names. Reproduced with `TMPDIR` pointed at a scratch dir — the
files still landed in `/tmp`, which is exactly why the `os.tmpdir()` row
and the leftover census below both read clean here. Both tests now own
the directory.
- **A prefix-scanned env var is invisible to a grep for variable
names.** `secretSources` enumerates `process.env` by the
`ARGENT_SECRET_` prefix and ranks it *above* every file the test writes,
so `secrets-command.test.ts` — which already pins `HOME`, `USERPROFILE`
and cwd — still read the developer's shell. `ARGENT_SECRET_FOO=bar`,
argent's own documented way to supply a secret: `6 passed` → **`3 failed
| 3 passed`**, deterministic. `argent-cli` has no `ARGENT_*`-stripping
setup file, so the prefix is cleared per test. Its sibling
`configuration-core/tests/secrets.test.ts` was already immune — it
threads an explicit `env: NO_ENV` into every call.
- **`android-binary` and `vega-cli-fallback`** were the last two files
redirecting `HOME` without `USERPROFILE` — the former the direct twin of
`android-binary-windows`, in the same directory, driving the same
resolver. Under a `node:os` mock reproducing the Windows lookup with an
SDK planted in the profile, it loses **5 of 11** tests. No test file in
the repo redirects `HOME` alone any more. (`6fbb66ce1` later skipped
both of this file's describes on win32 — its fixtures are extensionless
and the win32 resolver only takes `.exe` — so its own `USERPROFILE` pin
is now symmetry with the sibling files that do run there, not a live
guard.)

Two smaller items in the same commit. `native-devtools-socket-bind`'s
ENOENT fixture still rooted at `os.tmpdir()` while the comment three
screens above explains why the others do not: with a 142-character
ambient `$TMPDIR` that path exceeds `sun_path` and `listen` rejects it
for *that* reason, which the assertions cannot tell from the
missing-directory one they were written for. And
`<home>\AppData\Local\Android\Sdk` — the resolver's fallback for a
GUI-spawned server that never inherited `%LOCALAPPDATA%` — could be
deleted with the whole suite green; it now has a test, and deleting
either win32 root fails exactly the tests named for it.

## Fifth pass

A fifth pass took the platform-branch shape the fourth pass exposed and
ran it as its own move: *for every function that branches on
`process.platform`, does the test's guard cover every arm, or only the
one the author's machine takes?* Fixed in `e08b466b6`.

- **`init-stale-config.test.ts` deletes the developer's real VS Code MCP
config on Windows.** The file mocks `os.homedir()` and says it does so
"so hidden-scope probes (`~/.claude.json`, VS Code user profile) never
touch the real home directory". `vscodeUserDirs()` branches `darwin →
homedir()/Library/Application Support`, `win32 → %APPDATA%`, `else →
homedir()/.config` — the win32 arm never calls `homedir()`, so the mock
is inert there, and the test's own `userMcpJsonPath()` mirrors the same
branch. Both user-profile tests then write a one-entry fixture over
`%APPDATA%\Code\User\mcp.json` with a whole-file `writeFileSync`, and
assert that `remove()` deleted it. Reproduced by forcing
`process.platform` to `win32` with a two-server `mcp.json` planted at a
probe `%APPDATA%`: **`3 passed`, and the file and its directory were
gone.** With `APPDATA` pinned, the whole file passes under the same
instrument — 31 tests — and the planted config is byte-identical.
- **`argent-tools-client` hands its ambient `ARGENT_*` to a real spawned
child.** The launcher spawns an actual tool-server, and
`buildToolsServerEnv` overrides only the keys it is given — everything
else rides in on `...baseEnv`. `ARGENT_HOST` is the one that bites: the
fixture binds the host it names while the launcher records and
health-checks `127.0.0.1`, so reuse misses and a second server is
spawned. With six overrides exported: **`10 failed | 181 passed`, and
eight stray `fake-tool-server` processes still holding TCP listen
sockets after the run.** With the prefix stripped: `191 passed`, none
surviving. The package now carries the same setup file `tool-server`
has, for the same reason — and it is the second of the two packages
whose missing `ARGENT_*` strip this review found.

## Later passes

A sixth pass audited the fix commits themselves and bounded the SDK
probe (`6fbb66ce1`); a seventh ran the moves against the diff one more
time and turned up the two below, plus the `listen(0)` defect filed as
#853.

- **The `PATH` pin made `workspace-reader-integration` POSIX-only
without the guard its siblings got.** The shim it puts on `PATH` is an
extensionless `node`, and `symlink()` to a file needs an elevated
process or Developer Mode on Windows, so `beforeAll` throws `EPERM`
there before any assertion runs; a copy in place of the symlink would
not rescue it either, since a bare command name resolves through
`PATHEXT`. This sweep already skips `android-binary` and
`native-devtools-socket-bind` on win32 for exactly this class, and the
Windows job runs a curated list of seven `tool-server` files that
excludes this one, so nothing was catching it. Fixed in `095d75bd8` —
verified POSIX `2 passed`, and under a forced win32 platform `2 skipped`
with the top-level `beforeAll` never entered.
- **A rationale that rested on a case the file excludes.**
`android-binary`'s header justified pinning `USERPROFILE` by what
`os.homedir()` reads on Windows, two sentences before noting that both
describes skip on win32. Fixed in `f37b3605e`; the variable stays in the
snapshot list, the justification no longer claims a platform this file
never reaches.

A seventh pass also re-ran the sweep against a hostile machine: every
feature flag on in a scratch `~/.argent/flags.json`, no `adb` on `PATH`,
and a `TMPDIR` long enough to overrun the 104-byte `sun_path` limit. It
found one resource this branch had not closed.

- **A red `launcher-version-gate` run strands a real tool-server.**
`ensureToolsServer` spawns a fixture server, but its pid only reached
the reaper after three or four assertions on the state it returned; any
of those throwing left the pid unrecorded, and the same `afterEach` then
deleted `STATE_DIR`, the only other record of what to kill. **Eight such
processes were alive on the development machine**, spread from 14:33 to
18:23, every one from an `argent-version-gate-test-*` fixture and none
from any other family — each still holding a TCP listen socket with its
fixture directory long gone. Forcing one assertion to fail took the
`tool-server.cjs start` count from 10 to 11. The nearest twin,
`launcher-duplicate-spawn`, guards this twice: it records the pid
straight after the null check and sets `FAKE_TTL_MS` so an escaped
server self-exits. Fixed in `26c2c60ec` with both, the TTL also covering
the window between the spawn and the state read where no pid exists yet
— the same forced failure now leaves **zero** strays.

An eighth pass audited the seventh's own fixes, and found the
version-gate fix had been applied to one file when the defect was a
property of four.

- **Three more files strand a real tool-server on a red run.**
`launcher-spawn` and `launcher-exit` set `FAKE_TTL_MS` nowhere;
`launcher-duplicate-spawn` — the file `26c2c60ec` cites as the model —
armed it inside one test's `try/finally`, so eight of its nine spawns
ran without it and the `finally` disarmed it for the remainder of that
test. Reproduced per file by forcing a failure before the pid is
recorded: a detached node process survives, reparented to init, holding
a listening socket, with its `TEST_HOME` removed and its state record
already cleared. Fixed in `3bb69ded6` by moving the net to a file-wide
`beforeEach` in all three. Verified end to end — the child carries
`FAKE_TTL_MS=60000`, and a stranded server spawned at 21:28:36 exited by
itself at 21:29:36 with nothing signalling it; three consecutive green
runs stay at 191 passed.
- **A 29th file writes the real `~/.argent`.**
`boot-electron-spawn-error`'s success-path test completes a boot, and a
completed boot calls `trackChromiumPort`, persisting to
`os.homedir()/.argent/chromium-cdp-ports.json`. The real file on the
development machine had accumulated **106 dead ephemeral ports**. Its
two siblings in the same directory already mock `chromium-discovery` for
exactly this reason; this was the third, with no mock. Fixed in
`3306b6f40`.
- **`scopeTempHome` was itself unpinned.** Deleting its `USERPROFILE`
line — the Windows half, the same property the sibling helper's test
pins — left all twelve consumers green at 48 passed. `3306b6f40` adds
the missing test; it kills that mutation plus dropping the `HOME` pin
and replacing the per-test `mkdtemp` with one shared directory.

Two findings from that pass were refuted on verification and are
recorded rather than acted on: the tool-server tmpdir leftovers
(mechanism real, but they are the separate leak this body already scopes
out) and a claim that `26c2c60ec`'s message overstated its twin.

Four comments were also corrected against their code: `deriveReportPath`
joins `<base>-report.md` *inside* the trace's directory rather than
appending to the directory's name, which is what makes the
`rm(traceDir)` added in `a67c43f8d` cover it; two `android-perfetto`
comments still named `TMPDIR` as the mechanism after the switch to
`redirectTmpdir`; and `android-binary`'s header claimed the home pin
lets its two positive tests resolve "on either platform", though its
fixtures are extensionless and the win32 resolver only accepts `.exe`.
2026-08-19 23:06:48 +02:00

298 lines
12 KiB
TypeScript
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
/**
* Bug 4 — `native-profiler-analyze` must not return "All clear" when every
* trace_processor query failed. The result envelope must carry a machine-
* readable `status` field (`"ok"` vs `"analysis_failed"`) plus the per-query
* `exportErrors` so MCP / CLI callers can tell a truly clean trace apart from
* a run where the analyzer itself blew up.
*
* Shape mirrors `native-profiler-missing-trace.test.ts`: stub the lowest-level
* driver (`runTpQuery`) at the module boundary, then drive the analyze tool
* with a synthetic session whose `exportedFiles.pftrace` points at a real
* (empty-but-readable) file so the existence guard passes.
*/
import { describe, it, expect, vi, beforeEach } from "vitest";
import { ArtifactStore } from "@argent/registry";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { mkdtemp, rm, writeFile } from "node:fs/promises";
// Mocking runTpQuery is enough to control the entire Android pipeline output —
// `runAndroidProfilerPipeline` is just `Promise.allSettled` over three calls
// plus per-hang follow-ups. Any rejected promise becomes an `exportErrors`
// entry; resolved rows feed into the bottleneck transformers.
vi.mock("../src/utils/android-profiler/pipeline/run-tp", () => ({
runTpQuery: vi.fn(),
}));
// The Android pipeline pre-warms the in-process WASM engine up front via
// ensureTraceProcessorReady(). Keep every real export (notably the
// TraceProcessorUnavailableError class the analyze path branches on with
// `instanceof`) but stub the probe so it doesn't boot a real engine —
// individual tests then drive it to resolve (engine ready) or reject
// (wasm-load failure) the banner branch.
vi.mock("@argent/native-devtools-android", async (importOriginal) => {
const actual = await importOriginal<typeof import("@argent/native-devtools-android")>();
return {
...actual,
ensureTraceProcessorReady: vi.fn().mockResolvedValue(undefined),
};
});
import { runTpQuery } from "../src/utils/android-profiler/pipeline/run-tp";
import {
ensureTraceProcessorReady,
TraceProcessorUnavailableError,
} from "@argent/native-devtools-android";
import { nativeProfilerAnalyzeTool } from "../src/tools/profiler/native-profiler/native-profiler-analyze";
import { __primeDepCacheForTests, __resetDepCacheForTests } from "../src/utils/check-deps";
import type { NativeProfilerSessionApi } from "../src/blueprints/native-profiler-session";
const runTpQueryMock = runTpQuery as unknown as ReturnType<typeof vi.fn>;
const ensureReadyMock = ensureTraceProcessorReady as unknown as ReturnType<typeof vi.fn>;
interface RunTpQueryOpts {
tracePath: string;
query: string;
substitutions: Record<string, string>;
}
interface QueryRouter {
"trace-bounds.sql"?: unknown[] | (() => Promise<unknown[]>);
"cpu-hotspots.sql"?: unknown[] | (() => Promise<unknown[]>);
"ui-hangs.sql"?: unknown[] | (() => Promise<unknown[]>);
"memory-rss.sql"?: unknown[] | (() => Promise<unknown[]>);
}
function routeQueries(router: QueryRouter): void {
runTpQueryMock.mockImplementation(async (opts: RunTpQueryOpts) => {
const entry = router[opts.query as keyof QueryRouter];
if (entry === undefined) {
throw new Error(`unrecognized option: ${opts.query}`);
}
if (typeof entry === "function") return entry();
return entry;
});
}
async function buildSessionWithTrace(): Promise<{
session: NativeProfilerSessionApi;
cleanup: () => Promise<void>;
}> {
const dir = await mkdtemp(join(tmpdir(), "native-profiler-failure-"));
const tracePath = join(dir, "fake.pftrace");
// Just needs to exist — runTpQuery is mocked, no real parsing happens.
await writeFile(tracePath, "", "utf8");
const session: NativeProfilerSessionApi = {
deviceId: "emulator-5554",
platform: "android",
appProcess: "com.example.app",
capturePid: null,
captureProcess: null,
traceFile: tracePath,
exportedFiles: { pftrace: tracePath },
disposed: false,
profilingActive: false,
wallClockStartMs: null,
parsedData: null,
cpuFilterPid: null,
recordingMallocStackLogging: null,
mallocStackLogging: null,
recordingTimeout: null,
recordingTimedOut: false,
recordingExitedUnexpectedly: false,
lastExitInfo: null,
androidOnDeviceTracePath: null,
};
return {
session,
cleanup: () => rm(dir, { recursive: true, force: true }),
};
}
describe("native-profiler-analyze: status + exportErrors envelope (Bug 4)", () => {
beforeEach(() => {
// The analyze path's dependency gate resolves a real `adb` off PATH,
// $ANDROID_HOME and the default SDK locations, so without priming these
// tests pass only on a machine that has the Android SDK installed.
__resetDepCacheForTests();
__primeDepCacheForTests(["adb"]);
runTpQueryMock.mockReset();
// Default: the engine loads fine. The banner test overrides this.
ensureReadyMock.mockReset();
ensureReadyMock.mockResolvedValue(undefined);
});
it("status=ok with empty exportErrors when all queries succeed and find nothing", async () => {
const { session, cleanup } = await buildSessionWithTrace();
try {
// Pipeline guards against a "blank" trace by synthesising a
// `no CPU samples` exportError when BOTH cpu and hang queries return
// zero rows (see pipeline/index.ts: the `<profileable>` hint). To test
// the truly-clean case we feed one CPU row whose leaf_function is
// null — the aggregator drops it, so we get bottlenecksTotal === 0
// without tripping the fallback.
routeQueries({
"trace-bounds.sql": [{ start_ts: 0 }],
"cpu-hotspots.sql": [
{
thread_name: "main",
is_main_thread: 1,
leaf_function: null,
sample_count: 0,
first_ts_ns: 0,
last_ts_ns: 0,
burst_windows: null,
total_samples: 0,
},
],
"ui-hangs.sql": [],
"memory-rss.sql": [],
});
const result = await nativeProfilerAnalyzeTool.execute(
{ session },
{ device_id: "emulator-5554" },
{ artifacts: new ArtifactStore() }
);
expect(result.status).toBe("ok");
expect(result.exportErrors).toEqual({});
expect(result.bottlenecksTotal).toBe(0);
// Renderer keeps the verbatim "All clear" sentence on the happy path.
expect(result.report).toContain(
"All clear — no CPU hotspots, UI hangs, or memory issues detected."
);
expect(result.report).not.toContain("Analysis failed");
} finally {
await cleanup();
}
});
it("status=analysis_failed with per-query errors when every query rejects, and no 'All clear'", async () => {
const { session, cleanup } = await buildSessionWithTrace();
try {
// trace-bounds.sql failure is swallowed by `getTraceStartNs` (returns 0)
// so it does NOT land in exportErrors — that's by design. The three
// pipeline queries are the ones the user-facing failure banner tracks.
const reject = (q: string) => () =>
Promise.reject(new Error(`unrecognized option: --query-output=json (${q})`));
routeQueries({
"trace-bounds.sql": reject("trace-bounds.sql"),
"cpu-hotspots.sql": reject("cpu-hotspots.sql"),
"ui-hangs.sql": reject("ui-hangs.sql"),
"memory-rss.sql": reject("memory-rss.sql"),
});
const result = await nativeProfilerAnalyzeTool.execute(
{ session },
{ device_id: "emulator-5554" },
{ artifacts: new ArtifactStore() }
);
expect(result.status).toBe("analysis_failed");
expect(result.bottlenecksTotal).toBe(0);
expect(Object.keys(result.exportErrors).sort()).toEqual(["cpu", "hangs", "rss"]);
expect(result.exportErrors.cpu).toMatch(/unrecognized option/);
expect(result.exportErrors.hangs).toMatch(/unrecognized option/);
expect(result.exportErrors.rss).toMatch(/unrecognized option/);
// The misleading "All clear" line must not appear.
expect(result.report).not.toContain(
"All clear — no CPU hotspots, UI hangs, or memory issues detected."
);
// The new banner must appear, plus the existing Export-warnings block.
expect(result.report).toContain("Analysis failed");
expect(result.report).toContain("Export warnings");
} finally {
await cleanup();
}
});
it("status=analysis_failed (partial) when only some queries fail but others return data", async () => {
const { session, cleanup } = await buildSessionWithTrace();
try {
// CPU + hangs succeed, rss fails. The renderer's full-report path will
// still produce a `## Summary` table because bottlenecksTotal > 0; the
// envelope's `status` flips to analysis_failed regardless.
routeQueries({
"trace-bounds.sql": [{ start_ts: 0 }],
"cpu-hotspots.sql": [
{
thread_name: "main",
is_main_thread: 1,
leaf_function: "doFrame",
sample_count: 50,
first_ts_ns: 0,
last_ts_ns: 500_000_000,
// SQL-side burst windows: one burst, ms 0–20, 3 samples.
burst_windows: "0:20:3",
total_samples: 500,
},
],
"ui-hangs.sql": [],
"memory-rss.sql": () =>
Promise.reject(new Error("unrecognized option: --query-output=json")),
});
const result = await nativeProfilerAnalyzeTool.execute(
{ session },
{ device_id: "emulator-5554" },
{ artifacts: new ArtifactStore() }
);
expect(result.status).toBe("analysis_failed");
expect(Object.keys(result.exportErrors)).toEqual(["rss"]);
expect(result.bottlenecksTotal).toBeGreaterThan(0);
// Both the Export-warnings block AND the normal Summary table must
// appear — the report is informative, not just a banner.
expect(result.report).toContain("Export warnings");
expect(result.report).toContain("## Summary");
// No misleading "All clear" + no full-failure banner (because we have
// real bottlenecks, the renderer never reaches the all-clear branch).
expect(result.report).not.toContain(
"All clear — no CPU hotspots, UI hangs, or memory issues detected."
);
expect(result.report).not.toContain("Analysis failed");
} finally {
await cleanup();
}
});
it("renders the engine-load banner when the bundled WASM engine fails to load", async () => {
const { session, cleanup } = await buildSessionWithTrace();
try {
// Probe fails: the bundled trace_processor.wasm couldn't load. This is a
// SEPARATE branch from per-query failures — it must surface as a prominent
// banner, NOT folded into exportErrors / "Export warnings".
ensureReadyMock.mockRejectedValue(
new TraceProcessorUnavailableError("wasm_load_failed", {
version: "v55.3",
})
);
const result = await nativeProfilerAnalyzeTool.execute(
{ session },
{ device_id: "emulator-5554" },
{ artifacts: new ArtifactStore() }
);
expect(result.status).toBe("analysis_failed");
expect(result.bottlenecksTotal).toBe(0);
// Empty exportErrors → the banner is the whole story, no per-query noise.
expect(result.exportErrors).toEqual({});
// The actionable recovery guidance must appear front-and-centre…
expect(result.report).toContain("Cannot Run");
expect(result.report).toContain("ARGENT_TRACE_PROCESSOR_WASM");
// …and the per-query "Export warnings" block must NOT.
expect(result.report).not.toContain("Export warnings");
// The queries are never even attempted once the probe rejects.
expect(runTpQueryMock).not.toHaveBeenCalled();
} finally {
await cleanup();
}
});
});