Improved external storage permission handling #157 #163
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
@davidjiagoogle can you guide me regarding what i should do to make these tests pass ? |
| @@ -25,10 +24,17 @@ | |||
| android:required="false" /> | |||
|
|
|||
| <uses-permission android:name="android.permission.CAMERA" /> | |||
| <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" | |||
There was a problem hiding this comment.
Hi Anshu, you removed writing permission and replaced it with reading permissions. Won't this cause a problem with image capturing and recording where we save an image or video file onto disk?
There was a problem hiding this comment.
More on this: Image capture is tested in ImageCaptureDeviceTest, where the WRITE_EXTERNAL_STORAGE permission is specified again in the test. I believe if you change AppTestUtil.APP_REQUIRED_PERMISSIONS to reflect your changes here, the test will fail, as the app wouldn't be able to save onto storage without WRITE permission
|
@Anshu-Bijarnia Have you tried running the tests locally with a connected device or emulator? |
I will try fixing the errors and test the application locally. |
Can you help me with this, even adding the write permission. The ImageCaptureDeviceTest fails. Write permission is only required till SDK - 28, correct? Higher than that the device already has access to scoped storage by default. Guide me on where i am going wrong. |
|
I can't be sure the exact problem you had after adding these permissions to AppTestUtil.APP_REQUIRED_PERMISSION the PR is not updated with them, but I'm guessing you ran into this error: This could be because you are trying to grant a permission passed a build version on which it can be granted. For example, if you try to grant "READ_EXTERNAL_STORAGE" on 32 and above, this error would show. Did you version gate the permissions like you did in JcaApp when adding them to AppTestUtil? Could you update the PR with the latest change and let's see what the errors are? |
| <uses-permission android:name="android.permission.READ_MEDIA_IMAGES" /> | ||
| <uses-permission android:name="android.permission.READ_MEDIA_VIDEO" /> | ||
| <!-- To handle the reselection within the app on devices running Android 14 | ||
| or higher if your app targets Android 14 (API level 34) or higher. --> | ||
| <uses-permission android:name="android.permission.READ_MEDIA_VISUAL_USER_SELECTED" /> |
There was a problem hiding this comment.
these 3 permissions should be added in AppTestUtil with the same version-checking logics they as in JcaApp
There was a problem hiding this comment.
I will do that and make a PR
This pull request provides a good solution for the external storage handling based on different android versions. Closing -> Improve external storage handling for external intent Image Capture #157