Skip to content

fix: fail fast when no device can match the allocation criteria - #309

Merged
gmegidish merged 1 commit into
mobile-next:mainfrom
rahul-system:fix/fail-fast-on-unmatchable-device-criteria
Sep 16, 2026
Merged

gmegidish merged 1 commit into
mobile-next:mainfrom
rahul-system:fix/fail-fast-on-unmatchable-device-criteria

Conversation

@rahul-system

Copy link
Copy Markdown
Contributor

Fixes #237

What was happening

deviceName: /PixelPro_AVD/ (a pattern written against the Android emulator's id, PixelPro_AVD, rather than its display name, PixelPro AVD) made the @mobilewright/test fixture hang instead of reporting an error.

MobilecliDriver.allocate() threw NoDeviceAvailableError for the no-match case. The device pool treats that error as a temporary condition — "every matching device is currently taken" — and re-queues the waiter, so an unsatisfiable filter silently waited for a device that could never appear, until the 10-minute allocation timeout. From the test's point of view it just hung.

What this changes

allocate() now applies the criteria filters first, then subtracts the taken devices:

  • No device can ever satisfy the criteria → a plain Error, which the pool does not re-queue, so the run fails immediately. The message lists every device that was seen as name (id, state), which makes a name-vs-id or typo mismatch obvious at a glance.
  • Matching devices exist but are all taken → still NoDeviceAvailableError, still retriable. That behaviour is unchanged.

resolveDevice()'s "Available:" list (the launch() path) now prints name (id) instead of just names, for the same reason.

What this deliberately does not change

The issue also suggested matching deviceName against device.id so that /PixelPro_AVD/ would resolve. I've left that out: deviceName means the display name, and deviceId already exists for selecting a device by its exact id. Making the regex match both would blur the two options. With this change the original config fails in a second with a message showing PixelPro AVD (PixelPro_AVD, online), so the fix is self-evident from the error.

One note for maintainers: the issue reports that android.launch() accepted /PixelPro_AVD/. That path (findDevice()) also matches on d.name only, so either mobilecli reports the underscored form as the name in that setup, or the launch call used a different pattern. I couldn't verify which without the reporter's environment — if mobilecli's naming really does differ between the two paths, that's a separate issue.

Tests

  • an unmatchable deviceNamePattern throws a non-retriable error listing the available devices
  • an unmatchable deviceType does the same
  • the only matching device being taken still throws NoDeviceAvailableError

npm run build, npm run lint and npm test all pass locally (642 passed, 1 skipped).

@coderabbitai

coderabbitai Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d9cae529-984f-4d82-8453-f1d16ed857f2

📥 Commits

Reviewing files that changed from the base of the PR and between 1257f0e and eb2a698.

📒 Files selected for processing (2)
  • packages/driver-mobilecli/src/driver.test.ts
  • packages/driver-mobilecli/src/driver.ts

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Walkthrough

The mobile CLI driver now includes device names and IDs in device-resolution errors. allocate() first identifies online devices that satisfy the requested criteria, then excludes occupied devices. It throws a non-retriable error when no device matches. It retains NoDeviceAvailableError when matching devices exist but are occupied. Tests cover device type, name patterns, occupied devices, and missing OS versions.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to eb2a6

No merge-blocking issue was identified in the allocation change.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning Issue #237 requires consistent device resolution with deviceName matching both device IDs and display names. This PR implements immediate non-retriable errors for unmatched criteria, preserves retry… Update fixture allocation so deviceName matches both device.id and device.name. Add or retain tests for ID matching, display-name matching, unmatched criteria, and all-taken retry behavior.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: allocation now fails immediately when no device matches the criteria.
Description check ✅ Passed The description directly explains the allocation hang, the fail-fast behavior, retained retry behavior, error details, scope limits, and tests.
Out of Scope Changes check ✅ Passed The changes stay within issue #237. Allocation filtering, error reporting, and tests directly address device resolution and the fixture hang. No unrelated change is identified.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Full details: Linked Issues check

Explanation

Issue #237 requires consistent device resolution with deviceName matching both device IDs and display names. This PR implements immediate non-retriable errors for unmatched criteria, preserves retry behavior when matching devices are taken, includes names and IDs in errors, and adds tests. However, deviceName still matches display names only, so /PixelPro_AVD/ remains unresolved by the fixture. This leaves the primary consistency and ID-matching requirement unmet.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@gmegidish

Copy link
Copy Markdown
Member

@rahul-system thank you. I'm just looking into the bug you reported and I understand the issue. There was a confusion between id and name, and mobilewright couldn't distinguish between the device-is-not-available-right-now (for example, capacity reached on cloud provider), and device-is-never-going-to-be-available-locally.

I'll review soon. Thanks again!

@gmegidish

Copy link
Copy Markdown
Member

@rahul-system any chance you can sign the commit?

@gmegidish
gmegidish force-pushed the fix/fail-fast-on-unmatchable-device-criteria branch from eb2a698 to 2c6116a Compare September 16, 2026 15:39
@gmegidish

Copy link
Copy Markdown
Member

@rahul-system I managed to force-push a signature. Trying to merge now.

@gmegidish
gmegidish merged commit 42b87b4 into mobile-next:main Sep 16, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

deviceName regex works with android.launch() but silently hangs with @mobilewright/test fixture due to inconsistent device resolution

2 participants