Android: Standardize the use of application id and namespace #5210
Conversation
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
|
I think the job failures are not related to these changes |
vaslabs
force-pushed
the
android/sort-out-application-namespace-and-id
branch
from
83932dd to
646b429
Compare
lihaoyi
approved these changes
May 28, 2025
Yes, it's ready |
lihaoyi
pushed a commit
that referenced
this pull request
May 31, 2025
Completes the missing features to allow for android tests (instrumented tests) to run for architecture-samples (hilt android Todo application) ## Changes These were the necessary changes needed to make instrumented tests for architecture samples work. ### Instrumented manifest AGP seems to be creating a hardcoded temporary file (which is easy to miss as the task is quite fast and the file is deleted once is finished), but I've managed to pick it up with some hackery and it looks like this ```XML <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="com.example.android.architecture.blueprints.main.test"> <uses-sdk android:minSdkVersion="21" android:targetSdkVersion="35"/> <application> <uses-library android:name="android.test.runner"/> </application> <instrumentation android:name="com.example.android.architecture.blueprints.todoapp.CustomTestRunner" android:targetPackage="com.example.android.architecture.blueprints.main" android:handleProfiling="false" android:functionalTest="false" android:label="Tests for com.example.android.architecture.blueprints.main"/> </manifest> ``` The manifest merging for instrumented tests, seem to only be picking manifest files from androidx.test packages, so I hardcoded that in and I ended up with the same manifest as in AGP . The task to get is is ```bash ./gradlew :app:processDebugAndroidTestManifest ``` ### Debug sources I've added the debug sources as part of the AndroidAppModule, to keep the scope of this PR small. I'll separate the debug from release similarly to how R8 was done in this PR in the future. In order to avoid making a lot of changes for the debug sources, I've included the debug sources as part of the build.mill, which will probably be removed when the release/debug separation work is completed. ### R8 I haven't managed to make the architecture-samples work with D8, whether some meta-inf is missing or not, with R8 and the same configuration choices as in the architecture-samples repo, everything works properly (app, unit tests, instrumented tests). I've then separated the R8 functionality into its own trait. It seems that by using R8 we can drop some of the manual code we have (like picking the meta-inf files manually) and creating other needed directories, as R8 seems to be doing all of that work. We can keep purely d8 builds for legacy purposes if we want to of course, but we'll see how it goes in the future when we integrate more apps. For R8, desugaring being enabled is also the sane default, so I switched that too, following up to failed builds of architecture-samples with R8. ### Cleanups - Some manifest generated values are not handled by the merger tool instead of manually constructing them - Printing out ignored for stream output of instrumented tests along with each line was confusing when dealing with failed tests, which I had initially, so I removed it. ### Fixes - Instrumented tests should run with androidApplicationId as the package, as that identifies the android app, was missed [from the previous PR](#5210) - Instrumented packaged class files, also need to include the classfiles of the modules they depend on (e.g. shared-tests in case of the architecture-samples). ## Running experience Performance-wise, this feels similar to AGP (I didn't do any benchmarks) https://github.com/user-attachments/assets/94e29098-91c8-4fb2-af62-8294833d261a ## Summary All in all, this is another milestone, where we have a fairly complex android app, which uses KSP and R8, can be built, run and tested with both unit and instrumented tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR standardises the use of application id and namespace. It is also one of the pre-requisites of making hilt instrumented tests to work.
The application id is a unique id for every android app , so the android manifest package depends on it.
The android manifest package is also used whenever there's a relative path for Applications and activities so
Becomes
During manifest merging. Because this happens when the manifest is loaded (before everything is merged) we start with the manifest package being the android namespace. We then pass the package property to the merger to be the application id (the package for each up is unique). This is what appears to be happening in AGP as well. (It is also the reason why I was confused when reverse engineering the AGP behaviour and was alternating between namespace and application id)
I've also cleaned up some of the xml manual modifications that are not needed anymore as are handled by the merger tool