Repository navigation
fix: fail fast when no device can match the allocation criteria - #309
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. WalkthroughThe mobile CLI driver now includes device names and IDs in device-resolution errors. Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to No merge-blocking issue was identified in the allocation change. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation Issue ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@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! |
|
@rahul-system any chance you can sign the commit? |
eb2a698 to
2c6116a
Compare
|
@rahul-system I managed to force-push a signature. Trying to merge now. |
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/testfixture hang instead of reporting an error.MobilecliDriver.allocate()threwNoDeviceAvailableErrorfor 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:Error, which the pool does not re-queue, so the run fails immediately. The message lists every device that was seen asname (id, state), which makes a name-vs-id or typo mismatch obvious at a glance.NoDeviceAvailableError, still retriable. That behaviour is unchanged.resolveDevice()'s "Available:" list (thelaunch()path) now printsname (id)instead of just names, for the same reason.What this deliberately does not change
The issue also suggested matching
deviceNameagainstdevice.idso that/PixelPro_AVD/would resolve. I've left that out:deviceNamemeans the display name, anddeviceIdalready 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 showingPixelPro 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 ond.nameonly, 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
deviceNamePatternthrows a non-retriable error listing the available devicesdeviceTypedoes the sameNoDeviceAvailableErrornpm run build,npm run lintandnpm testall pass locally (642 passed, 1 skipped).