Android sys img picker by moondial-pal · Pull Request #2843 · beeware/briefcase

moondial-pal · 2026-05-19T01:26:42Z

Added two step selection flow that fetches all available compatible images from sdkmanager --list.
Hard coded version is set if default is selected.

This enables the user to select an android version when creating a new emulator.

Fixes #737

PR Checklist:

I will abide by the BeeWare Code of Conduct
I have read and have followed the CONTRIBUTING.md file
This PR was generated or assisted using an AI tool

Assisted-by: Claude Sonnet 4.6

moondial-pal · 2026-05-19T01:31:48Z

The approach was revised during the PyCon sprint after a discussion with @mhsmith.
Am going to work on aligning tests and see if I can nail this down.

… and no system image.

…rmed versions in list_available_system_images

…e across platforms

moondial-pal · 2026-05-23T16:35:56Z

Are the 3 CI failures pre-existing or am I goofing something here?

docs-lint broken https://www.x.org/ link in x11passthrough.md, not touched by my PR
Windows ARM PySide6 build, similar failure visible in recent main branch CI runs
ReadTheDocs appears to be an infrastructure build failure?

I checked recent CI runs on main and found some of these issues crop up there.
I'm not sure how to get them to pass at this point 😅

mhsmith · 2026-05-25T13:40:05Z

Yes, the failures aren't related to this PR, so you don't need to worry about them.

Thanks for the updates, I'll look at this in a couple of days.

mhsmith · 2026-06-03T17:31:45Z


+    def list_available_system_images(
+        self, min_version: int = ANDROID_MIN_OS_VERSION
+    ) -> list[str]:


There's no need to declare a default argument which is never used in production code; this only obscures the actual source of the default.

On the other hand, the test code should test the behavior of the argument; it looks like it currently doesn't.

mhsmith · 2026-06-03T17:33:52Z

 from briefcase.integrations.subprocess import SubprocessArgT

 DEVICE_NOT_FOUND = re.compile(r"^error: device '[^']*' not found")
+ANDROID_MIN_OS_VERSION = 26


Why 26? The template currently defaults to 24.

Got it. I chose 26 because of a comment Russell made in issue #737: #737 (comment)
Would you recommend I stick with the template default of 24?

That's a typo on my part - we should definitely stick to the 24 minimum.

mhsmith · 2026-06-03T17:41:37Z

+        :param min_version: The minimum Android version to include. Defaults to
+        ``ANDROID_MIN_OS_VERSION``.
+        e.g., ``["system-images;android-31;default;x86_64",   "system-
+        images;android-34;default;x86_64",   "system-
+        images;android-34;google_apis;x86_64"]``


If the default argument is removed, it should be removed from the docstring as well.

Our docstrings now use Markdown, so code should be marked with single backquotes. The spacing and line breaking in the code probably won't render correctly either, but RTD obviously has a problem with the current commit, so we can't see for sure. If you merge from the main branch, that will probably fix the current error.

Also, if you intend the "e.g." paragraph to be part of the main docstring, then it should go before the param list, not after.

mhsmith · 2026-06-03T17:46:32Z

+        for line in output.splitlines():
+            package = line.split("|")[0].strip()
+            if not package.startswith("system-images"):
+                continue


The code above this point could be replaced with a call to list_installed_system_images.

Sorry, not replaced exactly, but the duplicate code should be factored out.

mhsmith · 2026-06-03T17:51:55Z

-        system_image = self.DEFAULT_SYSTEM_IMAGE
+        # Get available images, raise an error if none found.
+        available_images = self.list_available_system_images(
+            min_version=getattr(app, "min_os_version", ANDROID_MIN_OS_VERSION)


Recommend factoring out this getattr expression into a function, and using it in output_format_template_context. That would eliminate any risk of the two locations going out of sync.

mhsmith · 2026-06-03T17:59:01Z

+            )
+
+        # Ask the user to select an Android version.
+        versions = sorted({img.split(";")[1].split("-")[1] for img in available_images})


The split expressions in this function are difficult to understand. Suggest creating a parsing function which converts a system image name into an object with fields for Android version, image type and ABI. Then we can either use that function both here and in list_available_system_images, or maybe even make list_available_system_images return the parsed objects.

mhsmith · 2026-06-03T18:01:09Z

+        versions = sorted({img.split(";")[1].split("-")[1] for img in available_images})
+        version = self.tools.console.selection_question(
+            intro="Select the Android version for the emulator:",
+            description="Android version",


To avoid confusion, should say "API level" rather than "Android version".

mhsmith · 2026-06-03T18:05:08Z

+            {
+                f"{img.split(';')[1]};{img.split(';')[2]}"
+                for img in available_images
+                if img.split(";")[1].startswith(f"android-{version}")


Why is the version included in the type here, and why are we using startswith? I thought it might be because of decimal versions, but that doesn't quite seem right either. For example, if the version question has choices for 36 and 36.1, and the user chose 36, the second question will still include the images for 36.1. It's probably better not to do anything clever about that, and treat every version independently.

mhsmith · 2026-06-03T18:13:06Z

        skin = self.DEFAULT_DEVICE_SKIN

-        # TODO: Provide a list of options for system images.
-        system_image = self.DEFAULT_SYSTEM_IMAGE


DEFAULT_SYSTEM_IMAGE is still used in one other place, but it should be updated to make sure it's consistent with the default Android version and image type used in this function. This would probably involve creating DEFAULT properties for those two things, and using them in both locations.

mhsmith · 2026-06-03T18:16:27Z


+    # Create a mock app
+    app = MagicMock()
+    del app.min_os_version  # ensure getattr fallback is used


There should be at least one test of the attribute being present.

moondial-pal · 2026-06-04T03:36:45Z

Thanks for the detailed feedback. I'll work through each point.
The suggestion to factor out the parsing logic and treat versions independently makes a lot of sense. I'll get back to you with updates.

moondial-pal and others added 4 commits May 14, 2026 17:26

Add system image selection to create_emulator

a545d22

Add tests for system image selection in create_emulator

f60e924

Add changelog entry for issue 737

11da731

Provide selector for available android images

40a4f9e

moondial-pal added 6 commits May 19, 2026 12:54

Filter all available system images by architecture and minimum version.

88ce516

Update change file to reflect new selection in wizard.

48fccb9

Update tests for missing coverage of dotted versions, named versions,…

2a5dc7a

… and no system image.

Correct misplaced tests and add coverage for dotted, named, and malfo…

11ab15c

…rmed versions in list_available_system_images

Add arm64-v8a variants to ensure converage on ARM Mac

6f79400

Add mock_tools fixture to lock architecture for deterministic coverag…

5644e62

…e across platforms

moondial-pal marked this pull request as ready for review May 23, 2026 16:36

mhsmith self-requested a review May 25, 2026 13:40

mhsmith reviewed Jun 3, 2026

View reviewed changes

Uh oh!

Conversation

moondial-pal commented May 19, 2026

PR Checklist:

Uh oh!

moondial-pal commented May 19, 2026

Uh oh!

moondial-pal commented May 23, 2026

Uh oh!

mhsmith commented May 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

moondial-pal Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

moondial-pal commented Jun 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

moondial-pal Jun 4, 2026 •

edited

Loading