With bug 1680407 fixed we will have a proper way to auto-detect the correct location for the test_root directory on the device. Sadly we cannot remove it yet because test internal code in Raptor makes use of it and force it to internal.

As such bug 1680407 will take a minimal change that fixes the detection of the location for the sdcard option, and also makes it the default one. If no reports are coming back and everything works fine we can finally remove the argument once mozdevice (python) and Raptor have been updated first.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from bug 1680407 comment #38)

The problem here is that Browsertime uses --android-storage to force the internal value. And with that patch it's not available anymore.

Those lines need to be removed, but then also the crash report fetching code needs to be updated to point to the external storage. Problem here is that it is based on mozdevice.test_root, and changing that is quite an amount of work. :(

I will pick this up again once we are sure that the argument can be removed.

Before we can remove this argument we have to make sure to actually fix issue https://github.com/mozilla/geckodriver/issues/1885.

As mentioned in comment 0 since bug 1680407 has been fixed we make use of EXTERNAL_STORAGE to determine the actual location for the test profile and files. But as noticed when trying to get this environment variable from our internally used Android emulator, it's not existent there.

Agi, could you please give an advice to which folder we should actually fallback to in case the environment variable hasn't been set? Thanks.

Ideally, we should be using the return value of getExternalFilesDir which is accessible by anyone and always present: https://developer.android.com/reference/android/content/Context#getExternalFilesDir(java.lang.String)

But from what I understand we need this value before we run the app, correct? I'm not sure if anything like that is available from adb or similar, I'll try to find something.

Exactly. We do not have a helper application that could be used to run such code. Here some questions and comments:

1. Using adb shell echo $EXTERNAL_STORAGE the value is empty, but logging into the shell via adb shell first, and then calling echo $EXTERNAL_STORAGE actually returns /sdcard. Do you know why that happens? At least geckodriver doesn't seem to have an issue with that (anymore?):

1637589104370	mozdevice	DEBUG	execute_host_command: >> "shell:echo $EXTERNAL_STORAGE"
1637589104374	mozdevice	DEBUG	execute_host_command: << "/sdcard\n"

2. What if external storage is not available? Where should we fallback to? (Maybe in such a case the environment variable is empty?)

3. I can see that people reporting that the environment variable and Environment.getExternalStorageDirectory() could contain different values. Which one do we trust?

Thanks!

In general my understanding is that Android is trying to move away from giving apps access to the full filesystem, so we should try to move to using the app-specific path given by getExternalFilesDir, but unfortunately I don't think Android gives you access to this value outside of the app itself, so it's a little bit tricky to resolve it dynamically in our use case.

I think our best bet would be to hard-code /storage/emulated/0/Android/data/<app_id>/files (which is the output of getExternalFilesDir), this folder should work for most devices and will continue to work for the foreseeable future.

Alternatively we could output this path in the logs, grab it, restart the app? That would be the safest way I think, but I'm honestly not sure how many devices don't work with the path above.

Using adb shell echo $EXTERNAL_STORAGE the value is empty, but logging into the shell via adb shell first, and then calling echo $EXTERNAL_STORAGE actually returns /sdcard. Do you know why that happens? At least geckodriver doesn't seem to have an issue with that (anymore?)

Are you properly escaping $? If you literally run adb shell echo $EXTERNAL_STORAGE, your terminal will try to resolve $EXTERNAL_STORAGE in your host machine, so it would end up running adb shell echo "" (I only realized that because you mentioned that the two values are different :) ). adb shell echo "\$EXTERNAL_STORAGE" does the right thing for me.

What if external storage is not available? Where should we fallback to? (Maybe in such a case the environment variable is empty?)

The output of Context#getExternalFilesDir(java.lang.String). From what I can gather there is no way to get that value from adb or anything similar, but I think we could hard-code some paths that we can try to use in case EXTERNAL_STORAGE is missing.

I can see that people reporting that the environment variable and Environment.getExternalStorageDirectory() could contain different values. Which one do we trust?

Neither, getExternalStorageDirectory is deprecated, we should use getExternalFilesDir which will give you an app-specific external path that is safe to use.

There are still remaining issues so we may have to leave support for this argument a bit longer:
https://github.com/mozilla/geckodriver/issues?q=is%3Aissue+is%3Aopen+--android-storage

Removing from any milestone tracking for now.