test: fix e2e coverage collection and expand device and rpc coverage - #315
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR expands Go unit coverage across commands, RPC, and media codecs; introduces shared JSON shape validators; integrates coverage environment setup across test suites; refactors Android and iOS device discovery; validates server RPC dispatch and live-device methods; and updates E2E execution and CI wiring. ChangesGo unit tests for commands, RPC, and media codec
Test shape contracts and environment setup
iOS and Android device workflows
Server RPC validation
E2E execution wiring
Estimated code review effort: 4 (Complex) | ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
test/emulator.spec.ts (1)
315-325: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
mobilecliJsonloses failure diagnostics thatmobilecliprovides.
mobilecli(lines 298-313) logs the command args and stderr/stdout on failure before rethrowing;mobilecliJsondoesn't, so a raw process failure (as opposed to a bad envelope, whichexpectOkEnvelopealready reports clearly) surfaces with no command context, making local/CI debugging harder.♻️ Proposed fix
function mobilecliJson(args: string[]): any { - const result = execFileSync(mobilecliBinary, args, { - encoding: 'utf8', - timeout: 60000, - stdio: ['pipe', 'pipe', 'pipe'], - env: coverageEnv(), - }); - const parsed = JSON.parse(result); - expectOkEnvelope(parsed); - return parsed; + try { + const result = execFileSync(mobilecliBinary, args, { + encoding: 'utf8', + timeout: 60000, + stdio: ['pipe', 'pipe', 'pipe'], + env: coverageEnv(), + }); + const parsed = JSON.parse(result); + expectOkEnvelope(parsed); + return parsed; + } catch (error: any) { + console.log(`Command failed: ${mobilecliBinary} ${args.join(' ')}`); + if (error.stderr) console.log(`stderr: ${error.stderr}`); + if (error.stdout) console.log(`stdout: ${error.stdout}`); + throw error; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/emulator.spec.ts` around lines 315 - 325, Update mobilecliJson to catch failures from execFileSync, log the command arguments and captured stdout/stderr using the same diagnostics pattern as mobilecli, then rethrow the original error; leave expectOkEnvelope handling unchanged for successfully executed commands.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@commands/keys_test.go`:
- Around line 57-63: Update TestKeysCommandRejectsEmptyKeyList to also assert
that the response error message equals "at least one key combo is required",
while retaining the existing status assertion, so the test specifically verifies
empty-key-list validation.
In `@pkg/avc2mp4/nalparser_test.go`:
- Around line 63-76: Update TestParseNALUnitsMasksRefIdcOutOfType to use a valid
H.264 header such as 0x45, which sets nal_ref_idc without setting
forbidden_zero_bit; keep the expected parsed type and original Data header
assertions aligned with the replacement header.
In `@rpc/rpc_test.go`:
- Around line 89-99: Replace the port-1 dependency in
TestCallReportsConnectionFailure with a deterministic injectable failing dialer
or transport supported by Call, ensuring the connection attempt reliably returns
the expected “failed to connect to fleet server” error without relying on an
unused local port.
In `@test/server.spec.ts`:
- Around line 299-309: Update the missing-required-field cases for
device.apps.path, device.crashes.get, and device.fs.* in
methodsMissingARequiredField to include deviceId: UNKNOWN_DEVICE_ID while still
omitting the field named by each test. Ensure these cases reach field-level
validation rather than failing during device lookup, matching the existing
device.io.swipe and device.webview.goto setup.
- Line 274: Update the `device.apps.uninstall` test request to pass the required
`bundleId` parameter instead of `packageName`, preserving the intended
unknown-device test path.
In `@test/shapes.ts`:
- Around line 32-36: Update expectForegroundAppShape to validate the required
version field from ForegroundAppResponse.data: assert foreground.version is a
string, alongside the existing packageName and appName checks.
---
Nitpick comments:
In `@test/emulator.spec.ts`:
- Around line 315-325: Update mobilecliJson to catch failures from execFileSync,
log the command arguments and captured stdout/stderr using the same diagnostics
pattern as mobilecli, then rethrow the original error; leave expectOkEnvelope
handling unchanged for successfully executed commands.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a6730140-52da-4199-8c23-52d6c41d7b88
⛔ Files ignored due to path filters (1)
test/README.mdis excluded by!**/*.md
📒 Files selected for processing (12)
Makefilecommands/keys_test.gopkg/avc2mp4/mp4writer_test.gopkg/avc2mp4/nalparser_test.gorpc/rpc_test.gotest/coverage.tstest/emulator.spec.tstest/server.spec.tstest/shapes.tstest/simctl.tstest/simulator.spec.tstest/types.ts
💤 Files with no reviewable changes (1)
- test/simctl.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/android.spec.ts (2)
54-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the underlying error before returning null.
The catch block discards the failure reason entirely. A missing/broken
mobileclibinary, a malformed JSON response, or a non-zero exit code will all be reported identically to "no device found" (line 72), making CI failures harder to diagnose.♻️ Proposed fix
} catch (error) { + console.error(`getFirstAndroidDevice(${deviceType}) failed:`, error); return null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/android.spec.ts` around lines 54 - 64, Update the catch block in getFirstAndroidDevice to log the caught error with sufficient context before returning null, while preserving the existing null fallback for command or JSON parsing failures.
76-304: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated
test.skip(!device, 'No Android device found')boilerplate across ~20+ tests.Nearly every test in this describe block opens with the identical skip call. Since
deviceis set once inbeforeAlland doesn't vary per test, this can be centralized.♻️ Proposed refactor
test.describe('Android Tests', () => { let device: Device | null; test.beforeAll(({deviceType}) => { device = getFirstAndroidDevice(deviceType); if (!device) { console.log(`No Android ${deviceType} device found. See test/README.md for setup instructions.`); } }); + + test.beforeEach(() => { + test.skip(!device, 'No Android device found'); + }); test('should take screenshot', ({deviceType}) => { - test.skip(!device, 'No Android device found'); // ... });Note: nested
test.describeblocks (e.g. the/sdcard/Downloadand playground-container groups) would need the samebeforeEachadded locally, or hoisted to the outer scope if applicable to all.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/android.spec.ts` around lines 76 - 304, Centralize the repeated missing-device skip by adding a shared `beforeEach` in the outer Android test scope that skips when `device` is unavailable, then remove the redundant `test.skip(!device, 'No Android device found')` calls from individual tests. Add equivalent hooks within nested `test.describe` blocks such as the `/sdcard/Download` and app-container groups if the outer hook does not cover them, while preserving unrelated skips and test setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/android.spec.ts`:
- Line 1: Update the recordThenInterruptWithCtrlC helper to bound its wait after
sending SIGINT: if mobilecli has not closed before the timeout, escalate by
sending SIGKILL or reject the operation, while preserving normal resolution when
the process exits promptly.
---
Nitpick comments:
In `@test/android.spec.ts`:
- Around line 54-64: Update the catch block in getFirstAndroidDevice to log the
caught error with sufficient context before returning null, while preserving the
existing null fallback for command or JSON parsing failures.
- Around line 76-304: Centralize the repeated missing-device skip by adding a
shared `beforeEach` in the outer Android test scope that skips when `device` is
unavailable, then remove the redundant `test.skip(!device, 'No Android device
found')` calls from individual tests. Add equivalent hooks within nested
`test.describe` blocks such as the `/sdcard/Download` and app-container groups
if the outer hook does not cover them, while preserving unrelated skips and test
setup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b42b4846-18b3-4571-a9b9-c1b32dfa7bea
⛔ Files ignored due to path filters (1)
test/README.mdis excluded by!**/*.md
📒 Files selected for processing (6)
.github/workflows/build.ymlMakefiletest/android.spec.tstest/fixtures.tstest/package.jsontest/playwright.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
| @@ -0,0 +1,501 @@ | |||
| import {test, expect} from './fixtures'; | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
ast-grep run --pattern 'spawn($$$)' --lang typescript test/android.spec.ts
rg -n -B2 -A15 'function mobilecli\b|spawn\(' test/android.spec.tsRepository: mobile-next/mobilecli
Length of output: 1937
🏁 Script executed:
sed -n '430,520p' test/android.spec.ts
printf '\n--- usages ---\n'
rg -n 'recordThenInterruptWithCtrlC|screenrecord|mobilecliBinary|setTimeout\\(|timeout' test/android.spec.tsRepository: mobile-next/mobilecli
Length of output: 3008
Add a force-kill fallback to the screenrecord helper
test/android.spec.ts:458-469 — recordThenInterruptWithCtrlC only sends SIGINT and waits for close; if mobilecli never exits, the test run can hang indefinitely. Add a timeout that escalates to SIGKILL (or rejects) if the process does not terminate.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 1-1: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import {execFileSync, spawn} from 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/android.spec.ts` at line 1, Update the recordThenInterruptWithCtrlC
helper to bound its wait after sending SIGINT: if mobilecli has not closed
before the timeout, escalate by sending SIGKILL or reject the operation, while
preserving normal resolution when the process exits promptly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/android.spec.ts`:
- Around line 253-315: Add an afterAll hook to the agent lifecycle suite that
attempts to uninstall the agent whenever a device is available, ensuring cleanup
runs even if a test fails or execution stops before the final uninstall test.
Reuse the existing runAgentCommand(device.id, 'uninstall') cleanup behavior from
beforeAll and keep the existing test assertions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ded08c3e-0f96-4b7c-86ed-936555cf9219
📒 Files selected for processing (2)
test/android.spec.tstest/shapes.ts
Comment on lines
+253
to
+315
| test.describe('agent lifecycle', () => { | ||
| test.beforeAll(() => { | ||
| if (!device) return; | ||
| // the device may arrive with an agent from an earlier run, so start clean. | ||
| // the response is ignored: "not installed" is a perfectly good outcome here | ||
| runAgentCommand(device.id, 'uninstall'); | ||
| }); | ||
|
|
||
| test('status should report no agent before installation', () => { | ||
| test.skip(!device, 'No Android device found'); | ||
|
|
||
| const response = runAgentCommand(device!.id, 'status'); | ||
| expect(response.status).toBe('fail'); | ||
| expect(response.data.message).toBe(AGENT_MISSING_MESSAGE); | ||
| }); | ||
|
|
||
| test('uninstall should report no agent when none is installed', () => { | ||
| test.skip(!device, 'No Android device found'); | ||
|
|
||
| const response = runAgentCommand(device!.id, 'uninstall'); | ||
| expect(response.status).toBe('fail'); | ||
| expect(response.data.message).toBe(AGENT_MISSING_MESSAGE); | ||
| }); | ||
|
|
||
| test('install should report a successful installation', () => { | ||
| test.skip(!device, 'No Android device found'); | ||
|
|
||
| const response = runAgentCommand(device!.id, 'install'); | ||
| expect(response.status).toBe('ok'); | ||
| expect(response.data.message).toBe('Agent installed successfully'); | ||
| expectAgentShape(response.data.agent); | ||
| expect(response.data.agent.bundleId).toBe(AGENT_BUNDLE_ID); | ||
| }); | ||
|
|
||
| test('installing again should report the agent is already installed', () => { | ||
| test.skip(!device, 'No Android device found'); | ||
|
|
||
| const response = runAgentCommand(device!.id, 'install'); | ||
| expect(response.status).toBe('ok'); | ||
| expect(response.data.message).toBe('Agent is already installed'); | ||
| expectAgentShape(response.data.agent); | ||
| expect(response.data.agent.bundleId).toBe(AGENT_BUNDLE_ID); | ||
| }); | ||
|
|
||
| test('status should report the installed agent', () => { | ||
| test.skip(!device, 'No Android device found'); | ||
|
|
||
| const response = runAgentCommand(device!.id, 'status'); | ||
| expect(response.status).toBe('ok'); | ||
| expectAgentShape(response.data.agent); | ||
| expect(response.data.agent.bundleId).toBe(AGENT_BUNDLE_ID); | ||
| // derived from the reported version rather than a literal, so bumping the | ||
| // pinned agent in cli/agent.go does not require editing this test | ||
| expect(response.data.message).toBe(`Agent version ${response.data.agent.version} is installed on device`); | ||
| }); | ||
|
|
||
| test('uninstall should succeed once an agent is installed', () => { | ||
| test.skip(!device, 'No Android device found'); | ||
|
|
||
| const response = runAgentCommand(device!.id, 'uninstall'); | ||
| expect(response.status).toBe('ok'); | ||
| expect(response.data.message).toBe('Agent uninstalled successfully'); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Always clean up the installed agent.
Cleanup only occurs in the final test. A failure or early stop after installation can leave the emulator modified and affect later runs. Add an afterAll cleanup attempt.
Proposed fix
test('uninstall should succeed once an agent is installed', () => {
// ...
});
+
+ test.afterAll(() => {
+ if (device) runAgentCommand(device.id, 'uninstall');
+ });
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| test.describe('agent lifecycle', () => { | |
| test.beforeAll(() => { | |
| if (!device) return; | |
| // the device may arrive with an agent from an earlier run, so start clean. | |
| // the response is ignored: "not installed" is a perfectly good outcome here | |
| runAgentCommand(device.id, 'uninstall'); | |
| }); | |
| test('status should report no agent before installation', () => { | |
| test.skip(!device, 'No Android device found'); | |
| const response = runAgentCommand(device!.id, 'status'); | |
| expect(response.status).toBe('fail'); | |
| expect(response.data.message).toBe(AGENT_MISSING_MESSAGE); | |
| }); | |
| test('uninstall should report no agent when none is installed', () => { | |
| test.skip(!device, 'No Android device found'); | |
| const response = runAgentCommand(device!.id, 'uninstall'); | |
| expect(response.status).toBe('fail'); | |
| expect(response.data.message).toBe(AGENT_MISSING_MESSAGE); | |
| }); | |
| test('install should report a successful installation', () => { | |
| test.skip(!device, 'No Android device found'); | |
| const response = runAgentCommand(device!.id, 'install'); | |
| expect(response.status).toBe('ok'); | |
| expect(response.data.message).toBe('Agent installed successfully'); | |
| expectAgentShape(response.data.agent); | |
| expect(response.data.agent.bundleId).toBe(AGENT_BUNDLE_ID); | |
| }); | |
| test('installing again should report the agent is already installed', () => { | |
| test.skip(!device, 'No Android device found'); | |
| const response = runAgentCommand(device!.id, 'install'); | |
| expect(response.status).toBe('ok'); | |
| expect(response.data.message).toBe('Agent is already installed'); | |
| expectAgentShape(response.data.agent); | |
| expect(response.data.agent.bundleId).toBe(AGENT_BUNDLE_ID); | |
| }); | |
| test('status should report the installed agent', () => { | |
| test.skip(!device, 'No Android device found'); | |
| const response = runAgentCommand(device!.id, 'status'); | |
| expect(response.status).toBe('ok'); | |
| expectAgentShape(response.data.agent); | |
| expect(response.data.agent.bundleId).toBe(AGENT_BUNDLE_ID); | |
| // derived from the reported version rather than a literal, so bumping the | |
| // pinned agent in cli/agent.go does not require editing this test | |
| expect(response.data.message).toBe(`Agent version ${response.data.agent.version} is installed on device`); | |
| }); | |
| test('uninstall should succeed once an agent is installed', () => { | |
| test.skip(!device, 'No Android device found'); | |
| const response = runAgentCommand(device!.id, 'uninstall'); | |
| expect(response.status).toBe('ok'); | |
| expect(response.data.message).toBe('Agent uninstalled successfully'); | |
| }); | |
| test.describe('agent lifecycle', () => { | |
| test.beforeAll(() => { | |
| if (!device) return; | |
| // the device may arrive with an agent from an earlier run, so start clean. | |
| // the response is ignored: "not installed" is a perfectly good outcome here | |
| runAgentCommand(device.id, 'uninstall'); | |
| }); | |
| test('status should report no agent before installation', () => { | |
| test.skip(!device, 'No Android device found'); | |
| const response = runAgentCommand(device!.id, 'status'); | |
| expect(response.status).toBe('fail'); | |
| expect(response.data.message).toBe(AGENT_MISSING_MESSAGE); | |
| }); | |
| test('uninstall should report no agent when none is installed', () => { | |
| test.skip(!device, 'No Android device found'); | |
| const response = runAgentCommand(device!.id, 'uninstall'); | |
| expect(response.status).toBe('fail'); | |
| expect(response.data.message).toBe(AGENT_MISSING_MESSAGE); | |
| }); | |
| test('install should report a successful installation', () => { | |
| test.skip(!device, 'No Android device found'); | |
| const response = runAgentCommand(device!.id, 'install'); | |
| expect(response.status).toBe('ok'); | |
| expect(response.data.message).toBe('Agent installed successfully'); | |
| expectAgentShape(response.data.agent); | |
| expect(response.data.agent.bundleId).toBe(AGENT_BUNDLE_ID); | |
| }); | |
| test('installing again should report the agent is already installed', () => { | |
| test.skip(!device, 'No Android device found'); | |
| const response = runAgentCommand(device!.id, 'install'); | |
| expect(response.status).toBe('ok'); | |
| expect(response.data.message).toBe('Agent is already installed'); | |
| expectAgentShape(response.data.agent); | |
| expect(response.data.agent.bundleId).toBe(AGENT_BUNDLE_ID); | |
| }); | |
| test('status should report the installed agent', () => { | |
| test.skip(!device, 'No Android device found'); | |
| const response = runAgentCommand(device!.id, 'status'); | |
| expect(response.status).toBe('ok'); | |
| expectAgentShape(response.data.agent); | |
| expect(response.data.agent.bundleId).toBe(AGENT_BUNDLE_ID); | |
| // derived from the reported version rather than a literal, so bumping the | |
| // pinned agent in cli/agent.go does not require editing this test | |
| expect(response.data.message).toBe(`Agent version ${response.data.agent.version} is installed on device`); | |
| }); | |
| test('uninstall should succeed once an agent is installed', () => { | |
| test.skip(!device, 'No Android device found'); | |
| const response = runAgentCommand(device!.id, 'uninstall'); | |
| expect(response.status).toBe('ok'); | |
| expect(response.data.message).toBe('Agent uninstalled successfully'); | |
| }); | |
| test.afterAll(() => { | |
| if (device) runAgentCommand(device.id, 'uninstall'); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/android.spec.ts` around lines 253 - 315, Add an afterAll hook to the
agent lifecycle suite that attempts to uninstall the agent whenever a device is
available, ensuring cleanup runs even if a test fails or execution stops before
the final uninstall test. Reuse the existing runAgentCommand(device.id,
'uninstall') cleanup behavior from beforeAll and keep the existing test
assertions unchanged.
This was referenced Aug 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
E2E coverage was being collected incorrectly, so
cover.outunderstated reality and Android contributed almost nothing. Fixing the harness first, then filling the gaps it revealed.1. E2E coverage collection was broken
Four independent breaks, all fixed:
Makefile—export GOCOVERDIR=test/coveragesat on its own recipe line, so it ran in a shell that exited immediately and never reached thenpm run test:*lines. Removed (collection is spec-side) and addedmkdir -pafter therm -rf.emulator.spec.ts:mobilecliJson()— passedenv: {ANDROID_HOME}, droppingGOCOVERDIR. Every JSON-returning Android call emitted nothing.emulator.spec.ts:mobilecli()— never setGOCOVERDIRat all.test/coverage.ts.Net effect: five
commands/fs.gofunctions that read 0.0% are now covered by the Android tests that were already exercising them.2. Android was missing app-lifecycle coverage
Six tests added to
emulator.spec.ts, mirroring what the iOS spec already had: list installed apps, launch + verify foreground, terminate + verify launcher, launch-twice idempotency, press HOME, and tap an element located via UI dump.Android's
dump uireturns a nested tree withtext/identifier(iOS useslabel/name), so this needed its own flatten + find helpers.3. The JSON-RPC dispatch table was almost untested
server.spec.tswent 10 → 59 tests, split so CI gets value:rpc method validation— no device required, so it runs on the bare CI runner wheretest:serverexecutes. Drives 26 methods with an unresolvable device id, 9 missing-required-field cases, and the twohandleFsPushbranches (invalid base64, >1MB) that run before device lookup.rpc methods against a live device— 11 happy-path tests covering handler success paths; skips cleanly without a device.Measured contribution of the live group, in isolation:
server34.3% → 37.1%.4. Protocol assertions
Tests asserted their own payload fields but nothing pinned the contract. Now enforced centrally:
rpc()checksjsonrpc: "2.0", thatidis echoed, and that exactly one ofresult/erroris present — on every call.rpcExpectError()pins the error code and requires a non-empty message. It also rejectsMethodNotFound, so a handler dropped from the registry fails instead of passing as "an error was returned".mobilecli()in both device specs now parses stdout and asserts the{status, data}envelope. Previouslylaunch,terminate,tap,button,url,screenshotand thefscommands checked only the exit code.test/shapes.tsdefines each entity once (device, app, foreground app, UI element, fs entry) and is reused across all three specs.Verified by renaming
json:"status"→json:"state"incommands/commands.goand confirming the suite fails.5. Device selection
Specs now ask mobilecli to filter instead of scanning results:
devices --platform android --type emulatordevices --platform ios --type simulator(replaces thesimctl-based name lookup;findSimulatorByNamedeleted)server.spec.tsexcludestype === "real"This is not cosmetic. With a physical Android phone attached, the unfiltered selection picked it and three tests failed while driving the real handset. Since
devicesomits offline entries by default, a shut-down simulator now skips instead of failing partway through.6. Go unit tests
rpc/had no tests → 52.6%pkg/avc2mp4/25.6% → 76.0% (NAL parsing, access-unit grouping, Annex-B,Converterror paths)commands/keys.go— added the twoKeysCommandvalidation branchesMuxing itself is left to the e2e screenrecord tests rather than synthesizing a valid SPS.
Coverage
servercommandsdevicesTest plan
go test ./...— all packages passgolangci-lint run— 0 issues in changed files (329 pre-existing onmain, unchanged)test.skipon the device-lifecycle test)android_emulator_testremainsif: falsein CI, so the Android suite still does not run thereNot covered
webview(11 RPC methods) needs a page fixture and is untouched.Summary by CodeRabbit