IssueTracker

I'm sure this is already well-known, but just for the record, even if you patch in cacheability to AndroidUnitTest via Gradle's runtime API, you'll still have an issue if you apply the `jacoco` plugin. The problem is that `append = true` by default. I'd suggest that any solution to this bug considers the jacoco use-case as well. (I think Gradle 5.0 removes that, but that doesn't help the current situation)

Duplicate here: https://issuetracker.google.com/issues/79122161

Starting with AGP 3.5.0-alpha03, unit tests are cacheable when includeAndroidResources=false, or not set (https://google.github.io/android-gradle-dsl/current/com.android.build.gradle.internal.dsl.TestOptions.UnitTestOptions.html#com.android.build.gradle.internal.dsl.TestOptions.UnitTestOptions:includeAndroidResources).

Fixed in commit: https://android.googlesource.com/platform/tools/base/+/518f9356956b7b3bc12985d7cf6488a3b178b6a3


Next, we are working on making unit tests cacheable when includeAndroidResources=true.

Thanks! Curious, what's your plan for "includeAndroidResources"? We switched from passing the file in the classpath to passing it through a system property that we set after inputs have been snapshotted, and then updated the Robolectric runner to use that instead. That's super hacky though! How are you planning to address that?

Using a system property might be good as a workaround, but we'll probably keep using files as I think files are a cleaner way of passing information among tasks/plugins.

The challenge is that Robolectric seems to rely on the test config file containing absolute paths, so changing to relative paths immediately would break it.

To address this, I'm planning to introduce a temporary flag to enable the new behavior: When this flag is true, the test config file will contain relative paths, and Robolectric needs to update to work with relative paths. Once Robolectric is updated without issues, we will flip the flag to true by default and eventually remove the flag. We might enforce a minimum version of Robolectric at that point.


@Christian: FYI on this upcoming change in the AGP. Please let me know if you anticipate any issues with this. Thanks!

I think we should consider making that temporary flag true by default, unless Robolectric is detected. Would that be possible? Otherwise projects that don't use Robolectric but _do_ include resources would lose relocatability. Or is that just not a valid use-case?

I wanted to first clarify why we must implement this transition through a flag (cc'ing Chris):
    - We've publicly documented that the test config file contains absolute paths: https://google.github.io/android-gradle-dsl/current/com.android.build.gradle.internal.dsl.TestOptions.UnitTestOptions.html#com.android.build.gradle.internal.dsl.TestOptions.UnitTestOptions:includeAndroidResources. So if any users of this includeAndroidResources property rely on the file containing absolute paths, changing to relative paths would break them. Because we don't know our users, we have to assume the worst.
    - Even if Robolectric is the only user of this property, I've checked (via a test project using this change) that it would also break Robolectric. To be clear, Robolectric can already work with the relative paths of most properties, except for the android_resource_apk property (not documented at the above link, but it exists). As soon as I change this property to relative path, Robolectric will fail. I've tracked that it is because this line of code requires the property to be an absolute path: https://github.com/robolectric/robolectric/blob/56681651992c3aa5e1cc08744a08a235a8481954/robolectric/src/main/java/org/robolectric/android/internal/ParallelUniverse.java#L277

Now, regarding the default value of the flag, because of the above breakage (to Robolectric) and risk (to users other than Robolectric), I believe it should be false by default during at least the first few canaries of 3.5.

At the end of the canary phase or when we enter the beta phase, we'll probably re-consider this default value. It will depend on factors such as: whether Robolectric has been updated to work with this change, how many bug reports we have received, how many users still use the old version of Robolectric (and have issues updating), etc.

That makes a lot of sense. Thanks for explaining.

Thanks for the explanation - using a flag makes sense. How does Robolectric figure out the base directory to use for obtaining the absolute paths? Would it work when running tests either in the command line or within Android Studio?

The base directory used by robolectric is the working directory of the test executor, which is the project root by default.
Making this not brittle to changing of the test executor current working directory might be useful, but perhaps can wait.

FYI, I've filed https://github.com/robolectric/robolectric/issues/4571 for Robolectric to add support for relative paths.

Also, just to be clear - it is the project directory, not the root directory, right? Would this work even if the build directory is not located under the project directory? (As an example, all our build directories are until one single /build directory under the root project)

All the paths will be relative to the root project's directory. I have highlighted that in https://github.com/robolectric/robolectric/issues/4571 so Robolectric authors are aware.

Test executors, however, seem to use the project directory, not the root directory, as the working directory (based on my testing on both gradle and AS). How would Robolectric obtain the root project directory, in order to obtain the absolute paths?

I don't know how Robolectric handles paths internally. I've forwarded the question to https://github.com/robolectric/robolectric/issues/4571, and will update the AGP if necessary so it can work for all sides.

Sorry my earlier observations were incomplete. I've updated it at https://github.com/robolectric/robolectric/issues/4571:
    - If I use the root project directory as base, Robolectric will fail.
    - However, if I use the current project directory as base, then Robolectric 4.0+ already support relative paths.

I'm planning to change the AGP to use the current project directory as base now.

@César: Would that work for your use case? If not, could you clarify the issue on the Robolectric bug so we can figure out a way forward?

Yeah, thanks, that's what I meant when I said earlier that both Gradle and AS seemed to run tests using the project directory. Using paths that are relative to the project directory sounds perfect to me.

That sounds good.

Starting with AGP 3.5.0-alpha05, the following Gradle property will be available to use:
    android.testConfig.useRelativePath
It currently defaults to false. When set to true, the test config file will contain all paths relative to the current project directory. It should already work with Robolectric 4.0+.

Relevant commits:
    - https://android.googlesource.com/platform/tools/base/+/e1d267e226d7c69cebe8f53a4df519ab658a1a67
    - https://android.googlesource.com/platform/tools/base/+/e80be3495eab92186ef955850ba2ada7ff514caa


Please try out this feature and let us know if you see any issues. I'm expecting to set this flag to true by default soon. Leaving this bug open to keep track of that.

When do you expect 3.5.0-alpha05 to be published? The latest I can find right now is alpha03.

I think it will be released at the end of next week.

Hi,

Is there a plan to make this available even in future versions of AGP 3.4 ?

I'm afraid not, as we have gone from beta to RC for 3.4, meaning it's a lot harder to cherry-pick fixes.

Starting with AGP 3.6.0-alpha02, this flag android.testConfig.useRelativePath will be enabled by default. Android unit tests should be fully cacheable now. Thanks for the reports and discussion!

Change: https://android.googlesource.com/platform/tools/base/+/e48c3831b9a9dd4829d63ba80fddc46ba7e67e61

I'd like to point out a pitfall with unit test caching that should probably be fixed in gradle: test ordering is currently indeterministic, and may affect the test results if tests happen to be coupled. I filed a ticket a while back with Gradle, but it's not getting much traction: https://github.com/gradle/gradle/issues/8520.

I see, hopefully the Gradle team will get to it soon.

Relating to #4 and `includeAndroidResources=false`, has this been addressed yet? If not, when do we think it will be?

Yes, when includeAndroidResources=false, unit tests are already cacheable since 3.5.0-alpha03 (that's what I meant when writing comment #4).

Oh, what I meant was, when will unit tests be cacheable when it's true? You said
> Next, we are working on making unit tests cacheable when includeAndroidResources=true.

:)

Ah I see, when includeAndroidResources=true, unit tests will be cacheable starting with 3.6.0-alpha02 (comment #24).

Got it. Thanks for the clarification!