mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
Ignacy Łątka
8c8cdb885e
Found while running the full E2E harness against the packed 0.17.0 bundle for the release check. All of these are harness bugs, not product bugs. ## The core problem The harness reported results it had not actually measured. `assert_reject` looked for zod issues on **stdout**, but the CLI prints them on stderr and leaves stdout empty on a rejection. Both arms of the check called `pass()`, so the miss was invisible: the zod-path argument every case passes in was decorative, and a CLI that simply exited 1 for every input would have scored the same 88/88. **All 88 validation cases asserted nothing beyond "exit code was non-zero."** Once that check could fail, it exposed the rest: - **awk has no `\b` word boundary.** `/\bnumber\b/` matches nothing, so every flag kind resolved to `unknown`, `model_number_flags` was always empty, and **the bad-type matrix never emitted a single case** - while `_build_args` handed the string `"x"` to every required numeric/boolean/object field. - The describe parser matched `--[a-zA-Z0-9-]+`, **truncating every snake_case flag at the underscore** (`--device_id` → `device`), and kept the `-json` display suffix (`--selector-json`) that is not part of the schema key. Both produced payloads carrying keys no schema knows. - `missing-required` asserted `invalid_type` for enum flags, which zod reports as `invalid_value`. Parsed kinds went from **270 unknown / 21 enum / 1 array** to 131 string, 68 number, 28 boolean, 25 integer, 21 enum, 16 array, 3 object, 2 unknown (the last two are `any`, which is correct). ## Teardown and the exit code `run-e2e.sh` had no `trap`. An unbound variable inside a phase is fatal to the whole shell - `run_phase || warn` does not catch it - so an abort skipped the cleanup phase, the report, and `rm -rf $E2E_WORK`, leaking a ~500MB sandbox plus a tool-server, Electron, Metro and a fixture http server. Report generation, teardown and the exit code now live in an EXIT trap, with a completion flag: without it an aborted run exits **0** on "0 failures", which is verified below by mutation. Teardown runs *before* the sandbox is removed - `$E2E_HOME` holds the detached tool-server's pid file and that server is started with no idle timeout, so deleting the sandbox first strands a process `argent server stop` can no longer find. `90-cleanup.sh` gated `stop-all-simulator-servers` and `stop-metro` on `ARGENT_TOOLS_URL`, which the harness never sets (`ensure_server` deliberately does not pin it), so **neither ever ran** and every run left a simulator-server and Metro alive - while `teardown-complete` passed regardless. ## Other findings fixed - **`--android-avd` could never work**: `boot-device` was asked for 840s inside `run_tool`'s 120s timeout, and the booted serial stayed in a `local`, so the RN tier blanket-skipped all 24 debugger/profiler tools for want of a device that was right there. Nothing shut the emulator down either - the tool-server only reaps devices Lens booted, not ones `boot-device` started. - **The validation tier was byte-identical green against a dead server.** `argent run` does no client-side validation, so every call failed with a transport error that no one distinguished from a rejection. It refuses to run instead, and no longer counts `timeout(1)`'s 124-127 as a rejection. - **The post-uninstall restore** hardcoded `--omit=optional` and the sandbox prefix - dropping electron on a full run (the chromium tier then skipped itself as "electron not installed"), and under `--system` restoring the machine's real global `argent` into the sandbox instead of putting it back. - **The Android teardown claimed to leave an injected device alone** and then issued the same `stop-simulator-server` call as the self-booted branch, discarded the result, and recorded a skip saying it had not - tearing down the allocator's device service immediately before the RN tier drove that same serial. - **The chromium port-owner fallback** matched `:$port` as a substring and took the first pid anywhere in `ss -ltnp` output, so cleaning up `:1234` would kill a process listening on `:12345`. - **The RN tier's `stop-metro`** sat after every early return (a failed `debugger-connect` left Metro running) and ran unconditionally, killing a developer's own Metro on :8081 that the harness never started. - **The chromium display gate** accepted an installed `xvfb-run` as a substitute for `DISPLAY`, but nothing wraps the Electron spawn in it - so a headless box with it installed cleared the gate and failed `boot-device`, turning the gate red for exactly the environment it exists to skip. - A phase file with a syntax error silently re-ran the **previous** phase under the new phase's name; an unknown `--phase` selected nothing and exited 0; `-h` printed 7 lines of source code; a value-taking flag with no value died on an unbound `$2`; two runs in the same second shared one results file. - `flow-add-step` declares its own `args` property, which makes `--args` that field's value rather than the whole-payload escape hatch, so `run_tool` cannot address its fields. Skipped with that reason rather than failing on a payload the tool never received. ## Verification Against `swmansion-argent-0.17.0.tgz`, `introspection,validation`: | | before | after | |---|---|---| | totals | 177 pass / 0 fail / 5 skip | 221 pass / 0 fail / 6 skip | | rejection cases naming the field they test | **0 of 88** | **130 of 131** | | bad-type cases emitted | **0** | **43** | - `assert_reject` checked against right-field, wrong-field, wrong-code, no-path and unreachable-binary inputs. Every one of those cases passes on the un-fixed code. - Abort path: injected an unbound variable into a phase - cleanup phase runs, report written, sandbox removed, exit 1, `PARTIAL` reported. With the completion flag mutated to always-set, the same abort exits **0**, which is what pins it. - awk `\b`, the `-json` suffix, the underscore truncation, the `ss` port collision and the `timeout` 124-127 contract were each reproduced standalone before being fixed. ## Found by running against a live Android emulator The android tier executed end-to-end for the first time: **23 pass / 1 fail / 10 skip**, including the `screenshot-diff` pair that the original unbound-variable bug aborted on. Three further defects only surfaced by running: - **Nine `pass`/`fail`/`skip` calls passed their message as the third argument**, which `_record()` takes as the *case name* — so the detail column was empty and the case string changed every run. The chromium boot failure was the visible cost: it recorded `case=" "` with **no diagnostic whatsoever**. - **A call killed by `timeout(1)` writes nothing**, so `RT_OUT` is empty exactly when the operator most needs to know what happened. A hung `boot-device` was recorded as a failure with a blank detail. All nine ad-hoc detail formatters now share `rt_detail()`, which names the timeout (124) and a command `timeout` could not run (125-127). - **The chromium display gate tested that `$DISPLAY` was non-empty, not that it was usable.** A display-manager session switch on this box left `$DISPLAY` pointing at a server the process holds no cookie for; Electron then bound its CDP port and never answered on it (verified: the TCP connection is accepted, the HTTP response never arrives), so the tier hard-failed instead of skipping. Moving `profiler-stack-query` and `profiler-combined-report` into the native block also closed a hole their removal had opened: both were listed in `RN_TOOLS` and in the failure-arm skips but had no case on the success path, so a successful `react-profiler-start` left them with no pass, fail or skip at all. ## Not covered here - The **postinstall assertion** is left alone - #556 already fixes it, and this branch does not touch it. Until that merges the install phase still records that one failure. - The **RN tier still has not executed.** The emulator itself is fine — it cold-boots in ~24-30s and ran the whole android tier — but it dies from CPU starvation (`detected a hanging thread 'QEMU2 main loop'`) whenever this machine's load average climbs, which it does to 90-120 on 16 cores from concurrent work. Four attempts: it survived the android tier at load ~10, and was starved each time by a competing `npm install` or by Metro's own bundler startup. The four RN profiler fixes are verified against the tools' source contracts (the throw sites and their required params), not against a live run. - The **chromium tier still has not executed** past `boot-device`, for the display reason above. - Known remaining, deliberately left: `assert_ok` cannot fail for calls whose result is a constant `true`; the cookie/storage set→get pair never verifies the round-trip value; a tier-level skip writes one row so `report.py`'s coverage matrix silently drops the tools that tier would have recorded; `report.py` renders an empty or partial log as a clean green report.
99 lines
5.3 KiB
Markdown
99 lines
5.3 KiB
Markdown
# Full Argent E2E harness
|
|
|
|
A release-gating end-to-end test that starts from **nothing but a
|
|
`swmansion-argent-*.tgz` bundle** and exercises the whole product: the install
|
|
flow, every CLI command, every tool's argument validation, and a happy-path run
|
|
of every tool that applies against real devices.
|
|
|
|
Run it on the real Linux box and the real Mac before a release.
|
|
|
|
## Quick start
|
|
|
|
```bash
|
|
# from the repo root, with a swmansion-argent-*.tgz present there:
|
|
bash scripts/e2e-full/run-e2e.sh
|
|
|
|
# a subset of phases:
|
|
bash scripts/e2e-full/run-e2e.sh --phase install,introspection,validation
|
|
|
|
# offline core only, driving the unpacked bundle (no npm install, fast):
|
|
bash scripts/e2e-full/run-e2e.sh --skip-install --phase introspection,validation
|
|
```
|
|
|
|
A markdown report is written to `scripts/e2e-full/results/report-<ts>.md` and the
|
|
raw per-case log to `results/e2e-<ts>.jsonl`. The process exits non-zero if any
|
|
hard assertion failed (skips do not fail the run).
|
|
|
|
## What it does (phases)
|
|
|
|
| phase | needs | covers |
|
|
| --------------- | ------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------- |
|
|
| `install` | npm + network | `npm i -g <tgz>`, bundled binaries, `init` (global + `--local`), `update`, `uninstall`, telemetry, MCP-config generation |
|
|
| `introspection` | — | `--version/--help`, `tools`, `tools describe` for **every published** tool, feature flags, `server start/status/logs/stop`, `link/unlink` |
|
|
| `validation` | — | for every tool: missing-required / bad-enum / bad-type rejection (deterministic, no hardware) |
|
|
| `android` | Android emulator | happy-path of every touch/gesture/screenshot/app-lifecycle tool |
|
|
| `chromium` | Electron (bundled optional dep) + a display | boots a generated Electron app; drives CDP tools (scroll/drag/tabs/cookies/storage) |
|
|
| `rn` | `~/dev/bluesky` + Android device | debugger + react/native profiler + network chain against the real Bluesky app |
|
|
|
|
Tiers auto-skip (with a recorded reason) when their prerequisites are missing, so
|
|
a partial run still produces a meaningful report. iOS / tvOS / Vega tiers are
|
|
intentionally out of scope.
|
|
|
|
## Isolation
|
|
|
|
Everything runs under a throwaway `HOME` and npm prefix (`$(mktemp -d)`), so the
|
|
real machine's `~/.argent`, editor MCP configs, and global packages are never
|
|
touched — safe to run on a shared box. Add `--keep` to inspect the sandbox after.
|
|
|
|
## Providing a device
|
|
|
|
The device tiers need a booted device. Two ways:
|
|
|
|
- **Inject** an already-booted one (recommended on shared/CI machines):
|
|
`--android-serial emulator-5554` (the harness attaches, doesn't boot/teardown it).
|
|
- **Let the harness boot it**: `--android-avd Pixel_9a` (uses `boot-device`).
|
|
The booted serial is published to the RN tier, so it runs against the same device,
|
|
and the emulator is shut down in the cleanup phase — after every tier that uses it,
|
|
and on an aborted run too.
|
|
|
|
The Chromium tier needs no device — it generates and boots its own Electron app.
|
|
It requires `DISPLAY`; on a headless Linux box run the whole harness under
|
|
`xvfb-run`, which supplies one. Having `xvfb-run` merely installed is not enough,
|
|
because nothing wraps the Electron spawn in it.
|
|
|
|
## RN (Bluesky) tier
|
|
|
|
```bash
|
|
E2E_RN_DIR=~/dev/bluesky E2E_RN_PKG=xyz.blueskyweb.app \
|
|
bash scripts/e2e-full/run-e2e.sh --phase rn --android-serial <serial>
|
|
```
|
|
|
|
Assumes the Bluesky dev-client is already built and installed on the device; the
|
|
tier skips itself if the package is absent. Pass `E2E_RN_BUILD=1` to let it run
|
|
`expo run:android` first (slow). It starts Metro if nothing is serving the port
|
|
and tears down only a Metro it started — one you were already running is left
|
|
alone.
|
|
|
|
## Flags
|
|
|
|
```
|
|
--tgz PATH tarball to test (default: newest swmansion-argent-*.tgz at repo root)
|
|
--phase a,b,c subset of: install introspection validation android chromium rn
|
|
--skip-install drive the unpacked bundle directly (offline phases only; skips `install`)
|
|
--system install to the REAL global prefix (dedicated release machine only)
|
|
--android-serial S use an already-booted Android device
|
|
--android-avd NAME boot this AVD via boot-device
|
|
--keep leave the sandbox dir for inspection
|
|
```
|
|
|
|
## Layout
|
|
|
|
```
|
|
run-e2e.sh orchestrator: env setup, phase dispatch, report, exit code
|
|
lib/common.sh logging, argent_cli/run_tool, assert_* helpers, server + screenshot helpers
|
|
lib/discover-tools.sh parses `argent tools describe` into per-tool arg models
|
|
lib/report.py JSONL -> markdown (per-phase + per-tool coverage matrix + failures)
|
|
phases/*.sh one run_phase() per phase
|
|
results/ generated JSONL + report (gitignored)
|
|
```
|