Better support additionalTestOutputDir default [172376362]

Fixed

Bug

Status Update

No update yet.

Description

cc...@google.com

created issue #1

Nov 3, 2020 11:03PM

Current default directory requires android:requestLegacyExternalStorage on Q+ devices. See b/145598917#comment7 for this hitting us in CI (not the first time).

Changing the default is difficult, since it's hard coded in older versions of AGP. We could change it, and give people an option to change it back via inst arg, e.g. androidx.benchmark.legacyOutputDirectory=true

The simpler approach would be to set android:requestLegacyExternalStorage=true in our manifest, and avoid the complexity for clients, at the cost of setting a legacy behavior flag without app involvement. As our library is only expected to be used for testing purposes, this may be fine.

Comments

cc...@google.com <cc...@google.com> #2Jan 26, 2021 08:37PM

While I wasn't able to reproduce the file access crash locally to validate the CL with this behavior change, I did perform 3 forrest runs:

Output Overhaul (passed!)
Remove requestLegacyExternalStorage (failed!)
Output Overhaul + Remove requestLegacyExternalStorage (passed!)

With the failing run throwing an exception in each test method:

java.io.IOException: Failed to create file for benchmark report in:
java.io.IOException: Failed to create file for benchmark report in:
/storage/emulated/0/Download
...

These runs show that removing the requestLegacyExternalStorage flag becomes safe once the overhaul CL lands.

cc...@google.com <cc...@google.com> #3Jan 27, 2021 06:49PM

Marked as fixed.

Fixed with https://android-review.googlesource.com/c/platform/frameworks/support/+/1556613 and https://android-review.googlesource.com/c/platform/frameworks/support/+/1559670

ap...@google.com <ap...@google.com> #4Jan 27, 2021 06:49PM

Project: platform/frameworks/support
Branch: androidx-main

commit 32bd7676632fdeb83900575f645454a3267f2cc9
Author: Chris Craik <ccraik@google.com>
Date: Mon Jan 25 14:59:41 2021

Remove requestLegacyExternalStorage flag from benchmarks

Bug: 172376362
Test: testing via forrest

Change-Id: I8b21b035106d7a4296495409c34f18b902ab9541

M ads/ads-identifier-benchmark/src/androidTest/AndroidManifest.xml
M appcompat/appcompat-benchmark/src/androidTest/AndroidManifest.xml
M benchmark/benchmark/src/androidTest/AndroidManifest.xml
M benchmark/integration-tests/macrobenchmark/src/androidTest/AndroidManifest.xml
M collection/collection-benchmark/src/androidTest/AndroidManifest.xml
M compose/integration-tests/benchmark/src/androidTest/AndroidManifest.xml
M compose/integration-tests/macrobenchmark/src/androidTest/AndroidManifest.xml
M compose/runtime/runtime/compose-runtime-benchmark/src/androidTest/AndroidManifest.xml
M navigation/benchmark/src/androidTest/AndroidManifest.xml
M recyclerview/recyclerview-benchmark/src/androidTest/AndroidManifest.xml
M room/benchmark/src/androidTest/AndroidManifest.xml
M slices/benchmark/src/androidTest/AndroidManifest.xml
M work/workmanager-benchmark/src/androidTest/AndroidManifest.xml

https://android-review.googlesource.com/1559670

Issue 172376362

Description

Issue summary

Comments

cc...@google.com <cc...@google.com> #2Jan 26, 2021 08:37PM

cc...@google.com <cc...@google.com> #3Jan 27, 2021 06:49PM

ap...@google.com <ap...@google.com> #4Jan 27, 2021 06:49PM

Add comment

Issue metadata