KT-64377 - automatically download and setup Android SDK in Gradle Integration Test

[adam-enko]

Automatically download and setup Android SDK so that the tests can be run on local machines without additional setup.

Downloading the Android SDK requires committing the license files, passing the directory via the ANDROID_SDK env var, and then AGP magic does the rest.

Depends on

• KT-64377 - remove gradle wrapper files for integration test projects #3418

[adam-enko]

I'll be using this task more in later PRs to help manage local Maven publishing of the included-builds.

[whyoleg]

Can you please clarify a bit: this is needed for cases, where android SDK is not installed locally, right?

I mean, Android SDK can be installed already on users machines. Even so, if someone will want to run Android integration tests locally they are or Dokka developers or doing something with Android - and so most likely they will have Android SDK installed :)
On CI - it's the same, we don't want to download Android SDK on every run - so it will be preinstalled somewhere already.

I'm not against this change, just want to better understand the reasoning, may be f.e. it's better to add a flag which will cause using this SDK, if it's really needed.
With current approach, I think it's just a matter of time when some day (after some changes to tests/build-logic) we will understand, that our CI (or even local machine) downloads Android SDK on every run.

[whyoleg]

Im not sure, just asking - is this ok to store such files in .git? :)
I mean, if I understand correctly after this any user will automatically agree to license terms even if they doesn't know about it
May be Im a bit overreacting, just mentioning some thoughts

[whyoleg]

May be Im missing something, but as we already provide ANDROID_HOME above as env variable, why do we need to additionally copy local.properties file to test project?

[adam-enko]

ANDROID_HOME is required by the integration tests - possibly it could be removed.

dokka/dokka-integration-tests/gradle/src/integrationTest/kotlin/org/jetbrains/dokka/it/gradle/Android0GradleIntegrationTest.kt

Lines 26 to 27 in 8899834

	private val isAndroidSdkInstalled: Boolean = System.getenv("ANDROID_SDK_ROOT") != null ||
	System.getenv("ANDROID_HOME") != null

local.properties is useful for if you want to manually open a project (in a temp dir) that's failed a test, and you want to check it manually.

[whyoleg]

Why do we need this task, as in the end we will just copy the text from this file? Can't we just have a property androidSdkDirPath which we will then use?

[adam-enko]

Good point, I've updated the task to do both in one. (I originally adapted it from a Dokkatoo task which needed the separate tasks.)

[whyoleg]

Is it possible to somehow easily add out-of-the-box support for reading this value from local.properties inside project? As I already have Android SDK installed at my side.

[adam-enko]

That's a good idea, I'd be happy to do it. I've done something similar in MockK before. However I'm a little bit reluctant because the Android SDK is only needed in integration tests, and scanning/parsing local.properties files would add some complexity & maintenance.

Since it's possible to set the Android SDK using an environment variable, I'd prefer not to. But what do you think?

[adam-enko]

So, just to summarise, the reason for this is so that AGP will automatically download the Android SDK when running integration tests. This means that anyone can run the Android tests without needing to manually setup and install the SDK, which is more convenient.

Adding the local.properties file is useful for opening projects manually. If a test fails then it's convenient to open the copied project and be able to run it as a normal project, again, without needing to manually setup Android SDK.

Committing these license files isn't common, but Mozilla and Google have done it. The android-actions/setup-android GitHub action will automatically accept the licenses too.

With current approach, I think it's just a matter of time when some day (after some changes to tests/build-logic) we will understand, that our CI (or even local machine) downloads Android SDK on every run.

Good point! I'm pretty sure that the AGP auto-downloader will pick up existing Android SDK dirs, since the Dokkatoo tests on GitHub download the Android SDK which the tests should use. I'm pretty sure it doesn't download the Android SDK every time. But it's been a while since I looked at it... Perhaps there's some logic in Dokkatoo that will defer to the pre-installed SDK.

[whyoleg]

On current moment when running android tests from this branch I see this error:

BTW, locally I have both ANDROID_SDK_ROOT and ANDROID_HOME defined, so I would expect no usage of projects/ANDROID_SDK

[adam-enko]

Thanks for checking @whyoleg 👍, I've updated the logic so the Android SDK dir will first use ANDROID_SDK_ROOT and ANDROID_HOME in both the local.properties file

https://github.com/Kotlin/dokka/pull/3419/files#diff-9cd6ff3290240988a2e812310b5c963500b89af00734469944b9e1a4dff06790R44-R49

and also pass it in as ANDROID_HOME in the tests https://github.com/Kotlin/dokka/pull/3419/files#diff-9cd6ff3290240988a2e812310b5c963500b89af00734469944b9e1a4dff06790R52 and the local.properties generation task https://github.com/Kotlin/dokka/pull/3419/files#diff-9cd6ff3290240988a2e812310b5c963500b89af00734469944b9e1a4dff06790R66.

Could you try checking it out and trying again please?

[whyoleg]

So, now everything works when ANDROID_HOME and ANDROID_SDK_ROOT are set at my side (pointing to ~/Library/Android/sdk). But, if I will remove them (and restart IDEA, gradle, to refetch envs) - it will not work and fail with the same error :)
Does it work at your side? Do you have android SDK installed at your side?

Also, I see that createAndroidLocalPropertiesFiles is not executed at all now, is it correct? You need to manually run it.
And even after running it I see that there is no local.properties file created in android-0 folder.

[adam-enko]

So, now everything works when ANDROID_HOME and ANDROID_SDK_ROOT are set at my side (pointing to ~/Library/Android/sdk). But, if I will remove them (and restart IDEA, gradle, to refetch envs) - it will not work and fail with the same error :) Does it work at your side? Do you have android SDK installed at your side?

Ah okay, there was a bug with the createAndroidLocalPropertiesFiles task (see below) so it should work now. (I don't have Android SDK variables set up, but probably the local.properties file was still created from before I refactored the createAndroidLocalPropertiesFiles task, and I didn't delete them after refactoring to verify.)

Also, I see that createAndroidLocalPropertiesFiles is not executed at all now, is it correct? You need to manually run it. And even after running it I see that there is no local.properties file created in android-0 folder.

createAndroidLocalPropertiesFiles should run before the integration tests run

https://github.com/Kotlin/dokka/pull/3419/files#diff-9cd6ff3290240988a2e812310b5c963500b89af00734469944b9e1a4dff06790R69-R71

However, I learned today that a FileTree cannot contain directories

gradle/gradle#27567

I've updated the logic to use walk() to filter the Android project dirs.

[Tapchicoma]

I don't like the approach to writing something into the repository state and would like to propose the following changes to use build directory for temporary things:

• configure Android SDK for all integration tests in the build/android-sdk directory (if the user does not have it installed)
• Generate local.properties file into the build/android-prepare/local.properties file and pass via system property the path to the test runner
• Add to this logic additionally coping local.properties file into test project directory. Maybe this logic could be generalized to be used in other android IT tests.

[Tapchicoma]

Nit: could be converted to SetProperty<File>:

    @get:OutputFiles
    val localPropertiesFilesDestinations: SetProperty<File> = objects.setProperty<File>()
        .value(
                androidProjectsDirectories
                    .elements
                    .map {
                        it.map { it.asFile.resolve("local.properties") }.toSet()
                    }
        )
        .apply { disallowChanges() }

We should use provider API in our tasks/build configuration to make our transition to Gradle 9 smoother.

[adam-enko]

Sure thing, done. I've updated it to a Provider<Set<File>> (which SetProperty<File> implements) because I think using Provider<> instead of Property<> makes it more clear that this is a read-only value.

[Tapchicoma]

Nit: could also add here .map { it.invariantSeparatorsPath } to avoid calling it in other places

[Tapchicoma]

It could be simplified to avoid unnecessary computation:

    val androidProjects = setOf(
        templateProjectsDir.dir("it-android-0"),
    )
    androidProjectsDirectories.from(androidProjects)

[adam-enko]

I don't like the approach to writing something into the repository state and would like to propose the following changes to use build directory for temporary things:

* configure Android SDK for all integration tests in the `build/android-sdk` directory (if the user does not have it installed)

Can do, but the set up would be a little more complicated because the license files would have to be copied into build/android-sdk first. Running ./gradlew clean would also really slow down the integration tests. So I'm reluctant to move it. Unless you insist?

* Generate `local.properties` file into the `build/android-prepare/local.properties` file and pass via system property the path to the test runner

* Add to [this logic](https://github.com/Kotlin/dokka/blob/eace407bec18ae6bb54d6be8ff9f1a4c38edde5f/dokka-integration-tests/gradle/src/integrationTest/kotlin/org/jetbrains/dokka/it/gradle/Android0GradleIntegrationTest.kt#L38-L49) additionally coping `local.properties` file into test project directory. Maybe this logic could be generalized to be used in other android IT tests.

The reason why I added local.properties into the local projects is useful in case you want to open up a project in an IDE to edit/debug it. (The files are .gitignore'd so they're not in the repo state.) Again, I can do it as you suggest, but I'm reluctant to because it would make working with the projects slightly more inconvenient.

[adam-enko]

I decided not to proceed with this PR.

• It's more complicated than instructing people to install the Android SDK
• When Dokka is merged into the Kotlin repo, it shouldn't store files outside of build directories (because non-Git teams, like Google, don't follow .gitignore files)
• There's a Kotlin repo util for downloading the Android SDK that could be used in the future https://github.com/JetBrains/kotlin/blob/v1.9.22/dependencies/android-sdk/build.gradle.kts

I'll update the other PRs that depend on this PR.