mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
172e4d3a64
Part of #609. ## What was wrong On a rotated Android device `screenshot` returned a portrait-framed image with the content lying sideways. `describe` frames and gesture coordinates were **already upright** — uiautomator measures against the rotated display — so the capture was the one surface out of step. An agent reading that image saw geometry that disagreed with every coordinate it was given. Measured on a landscape Pixel_9 (API 36, `mCurrentOrientation=1`): | | space | |---|---| | `describe` | upright — `"Search Settings"` centre `(0.530, 0.234)` | | taps | upright — tapping describe's `"Display & touch"` centre opens it | | `screenshot` (default) | **portrait-native, sideways** | ## The fix simulator-server already applies `rotation` correctly; nothing was telling it which one. The rotation is now queried from the device per capture, so `screenshot`, `screenshot-diff`'s live captures and the auto-attached "Screen after action" screenshot all land in `describe`/tap space. **Nothing is tracked.** A cached orientation could only be staler than the ~40 ms probe, and serving a stale one is exactly the failure being fixed. When the rotation cannot be read the request is left byte-identical to before rather than guessing — a wrong orientation is a confidently wrong image, which is worse than a sideways one. **iOS is deliberately untouched.** There the whole surface is consistently portrait-native, so rotating only the capture would *break* an agreement rather than restore one. The iOS half of #609 needs a different change and is not in this PR. ### Also fixed: the legacy uiautomator describe path Same root cause, worse symptom. `wm size` reports the **unrotated** size — on the landscape Pixel_9 it still answered `Physical size: 1080x2424`. Dividing rotated bounds by that does not merely squash frames: `isVisibleRect` prunes nodes whose left edge is past the screen width, so **the right-hand half of a landscape screen vanished from the tree**. The dump carries `<hierarchy rotation="N">`, so the divisor is oriented from bytes already in hand — no extra round-trip on the path that runs precisely when the helper could not be reached. ## On the rotation-name table Pinned by measurement, never by what the names mean — the reporter of #609 found the same names are 180° inverted on iOS. `adb exec-out screencap` is rotation-aware, so it is ground truth; each candidate was scored as mean absolute difference against it both as-is and rotated 180°, so an inverted mapping could not pass: ``` rotation 1 LandscapeLeft 2.19 (vs 6.54 for its 180° twin) rotation 3 LandscapeRight 2.19 (vs 6.34) rotation 2 PortraitUpsideDown 2.67 (vs 13.13) rotation 0 Portrait identity ``` This table is the **inverse** of simulator-server's own `90° → LandscapeRight` convention, because it compensates for a rotation that layer applies while decoding the scrcpy stream. That makes it a property of a particular simulator-server build rather than of Android — which is why a table test alone is not enough, and the delivered PNG's aspect is checked against the rotation requested, falling back to an unrotated capture if they disagree. ## Verified live Branch tool-server against a real emulator: - landscape, default capture → **2424×1080 upright**, MAD 2.19 vs ground truth (6.54 vs its 180° twin) - cropping `describe`'s frame for the toolbar out of that default capture shows exactly the "Display & touch" toolbar — the two spaces agree - rotation 0 → 1080×2424, MAD 2.82, unchanged - rotated **iPad** → still 2064×2752 sideways, i.e. iOS provably untouched Plus 27 new unit tests, including one that asserts the legacy path *drops* the right-hand node before the fix and keeps it after. Full suite 3125 passing; lint, prettier, `typecheck:tests` and the tool-description gate all clean. ## Not in scope - The iOS half of #609. - `rotate` on Android silently no-ops when auto-rotate is enabled (it returned `{orientation:"LandscapeLeft"}` while `dumpsys` still read `cur=1080x2424`). Filing separately. - `screen-recording` still records sideways — same root cause, different pipeline. Worth noting a session's screenshots are now upright while its video is not. - Existing visual baselines captured on a rotated Android device will stop matching, since the baseline key embeds capture dimensions and those now swap. This fails loudly as a missing baseline rather than silently comparing the wrong thing — and those baselines were sideways captures of an upright UI. --- > **#673 and #658 are both stacked on this branch** — each conflicts with this one (`screenshot/index.ts` and `screenshot-diff/index.ts` respectively) but not with each other, so both were rebased onto this branch as siblings rather than chained. Merge this first; GitHub then retargets both to `main`. --------- Co-authored-by: Filip131311 <f.kaminski2000@gmail.com>
145 lines
5.3 KiB
TypeScript
145 lines
5.3 KiB
TypeScript
import { describe, it, expect, afterEach } from "vitest";
|
|
import * as net from "node:net";
|
|
import { adbServerAddress, adbServerShell } from "../src/utils/adb-server";
|
|
|
|
/**
|
|
* A fake adb server speaking just enough of the real protocol (4-hex-digit
|
|
* length prefix, OKAY/FAIL statuses, `host:transport:` then `shell:`) to drive
|
|
* `adbServerShell` end to end over a real socket. Every scenario is a script of
|
|
* how the server reacts to each message it receives.
|
|
*/
|
|
type Script = (message: string, socket: net.Socket, index: number) => void;
|
|
|
|
const servers: net.Server[] = [];
|
|
|
|
async function fakeAdbServer(script: Script): Promise<number> {
|
|
const server = net.createServer((socket) => {
|
|
let pending = Buffer.alloc(0);
|
|
let index = 0;
|
|
socket.on("data", (data: Buffer) => {
|
|
pending = Buffer.concat([pending, data]);
|
|
while (pending.length >= 4) {
|
|
const len = parseInt(pending.subarray(0, 4).toString("latin1"), 16);
|
|
if (pending.length < 4 + len) return;
|
|
const message = pending.subarray(4, 4 + len).toString("utf8");
|
|
pending = pending.subarray(4 + len);
|
|
script(message, socket, index++);
|
|
}
|
|
});
|
|
});
|
|
servers.push(server);
|
|
await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve));
|
|
return (server.address() as net.AddressInfo).port;
|
|
}
|
|
|
|
const fail = (socket: net.Socket, message: string) =>
|
|
socket.write(`FAIL${message.length.toString(16).padStart(4, "0")}${message}`);
|
|
|
|
afterEach(async () => {
|
|
await Promise.all(servers.splice(0).map((s) => new Promise((r) => s.close(r))));
|
|
delete process.env.ADB_SERVER_SOCKET;
|
|
delete process.env.ANDROID_ADB_SERVER_ADDRESS;
|
|
delete process.env.ANDROID_ADB_SERVER_PORT;
|
|
});
|
|
|
|
describe("adbServerShell", () => {
|
|
it("selects the device, runs the command and returns its output", async () => {
|
|
const seen: string[] = [];
|
|
const port = await fakeAdbServer((message, socket) => {
|
|
seen.push(message);
|
|
if (message.startsWith("host:transport:")) socket.write("OKAY");
|
|
else if (message.startsWith("shell:")) {
|
|
socket.write("OKAY");
|
|
socket.end("mCurrentOrientation=1\n");
|
|
}
|
|
});
|
|
process.env.ANDROID_ADB_SERVER_PORT = String(port);
|
|
|
|
const out = await adbServerShell("emulator-5554", "dumpsys display | grep mCurrentOrientation");
|
|
|
|
expect(out).toBe("mCurrentOrientation=1\n");
|
|
expect(seen).toEqual([
|
|
"host:transport:emulator-5554",
|
|
"shell:dumpsys display | grep mCurrentOrientation",
|
|
]);
|
|
});
|
|
|
|
it("handles output that arrives in the same packet as the OKAY", async () => {
|
|
const port = await fakeAdbServer((message, socket) => {
|
|
if (message.startsWith("host:transport:")) socket.write("OKAY");
|
|
else socket.end("OKAYmRotation=3\n");
|
|
});
|
|
process.env.ANDROID_ADB_SERVER_PORT = String(port);
|
|
|
|
expect(await adbServerShell("emulator-5554", "x")).toBe("mRotation=3\n");
|
|
});
|
|
|
|
it("rejects when the server does not know the serial", async () => {
|
|
const port = await fakeAdbServer((message, socket) => {
|
|
if (message.startsWith("host:transport:")) fail(socket, "device 'nope' not found");
|
|
});
|
|
process.env.ANDROID_ADB_SERVER_PORT = String(port);
|
|
|
|
await expect(adbServerShell("nope", "true")).rejects.toThrow(/device 'nope' not found/);
|
|
});
|
|
|
|
it("rejects when nothing is listening, so the caller falls back to the client", async () => {
|
|
const port = await fakeAdbServer(() => {});
|
|
await new Promise((r) => servers.pop()!.close(r));
|
|
process.env.ANDROID_ADB_SERVER_PORT = String(port);
|
|
|
|
await expect(adbServerShell("emulator-5554", "true")).rejects.toThrow();
|
|
});
|
|
|
|
it("rejects when the server hangs past the timeout", async () => {
|
|
const port = await fakeAdbServer(() => {});
|
|
process.env.ANDROID_ADB_SERVER_PORT = String(port);
|
|
|
|
await expect(adbServerShell("emulator-5554", "true", { timeoutMs: 50 })).rejects.toThrow(
|
|
/timed out/
|
|
);
|
|
});
|
|
|
|
it("rejects when the connection closes before the command was accepted", async () => {
|
|
const port = await fakeAdbServer((message, socket) => {
|
|
if (message.startsWith("host:transport:")) socket.write("OKAY");
|
|
else socket.destroy();
|
|
});
|
|
process.env.ANDROID_ADB_SERVER_PORT = String(port);
|
|
|
|
await expect(adbServerShell("emulator-5554", "true")).rejects.toThrow(/before answering/);
|
|
});
|
|
});
|
|
|
|
describe("adbServerAddress", () => {
|
|
it("defaults to the client's default server", () => {
|
|
expect(adbServerAddress({})).toEqual({ host: "127.0.0.1", port: 5037 });
|
|
});
|
|
|
|
it("honours ANDROID_ADB_SERVER_PORT and ANDROID_ADB_SERVER_ADDRESS", () => {
|
|
expect(
|
|
adbServerAddress({ ANDROID_ADB_SERVER_PORT: "6000", ANDROID_ADB_SERVER_ADDRESS: "10.0.0.2" })
|
|
).toEqual({ host: "10.0.0.2", port: 6000 });
|
|
});
|
|
|
|
it("lets ADB_SERVER_SOCKET win, with and without a host", () => {
|
|
expect(adbServerAddress({ ADB_SERVER_SOCKET: "tcp:localhost:5555" })).toEqual({
|
|
host: "localhost",
|
|
port: 5555,
|
|
});
|
|
expect(
|
|
adbServerAddress({ ADB_SERVER_SOCKET: "tcp:5555", ANDROID_ADB_SERVER_PORT: "1" })
|
|
).toEqual({
|
|
host: "127.0.0.1",
|
|
port: 5555,
|
|
});
|
|
});
|
|
|
|
it("ignores a malformed port rather than connecting somewhere surprising", () => {
|
|
expect(adbServerAddress({ ANDROID_ADB_SERVER_PORT: "lots" })).toEqual({
|
|
host: "127.0.0.1",
|
|
port: 5037,
|
|
});
|
|
});
|
|
});
|