1147614 - [mozdevice] The property ADBDevice.test_root does not appear in the documentation

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Julien Pagès (:parkouss)]

The property ADBDevice.test_root, equivalent to old api DeviceManager.getDeviceRoot does not appear on the doc: http://mozbase.readthedocs.org/en/latest/mozdevice.html

This is implemented here:
https://dxr.mozilla.org/mozilla-central/source/testing/mozbase/mozdevice/mozdevice/adb.py?from=adb.py#645

Note that there is a docstring (could be more explicit I think, something like http://mozbase.readthedocs.org/en/latest/mozdevice.html#mozdevice.DeviceManager.getDeviceRoot) but this is not integrated in sphinx probably.

Comment 1

[Julien Pagès (:parkouss)]

Attachment: The property ADBDevice.test_root does not appear in documentation

I generated the documentation locally and this look fine.

Comment 2

[Bob Clary [:bc] (inactive)]

Comment on attachment 8583378 [details] [diff] [review]
The property ADBDevice.test_root does not appear in documentation

We need to mark attachements as patches before we can review them.

Comment 3

[Bob Clary [:bc] (inactive)]

Comment on attachment 8583378 [details] [diff] [review]
The property ADBDevice.test_root does not appear in documentation

Review of attachment 8583378 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/mozbase/mozdevice/mozdevice/adb.py
@@ +684,5 @@
>      @property
>      def test_root(self):
> +        """This property returns the root path on the device filesystem for
> +        putting temporary testing files.
> +

parkouss, we might as well include more detail in the docstring on how the test_root is determined. From https://dxr.mozilla.org/mozilla-central/source/testing/mozbase/mozdevice/mozdevice/adb.py?from=adb.py#684 we can see that test_root is not a simple property.

How about something like this:

The test_root property returns the directory on the device where temporary test files are stored. The first time test_root it is called it determines and caches a value for the test root on the device. It determines the appropriate test root by attempting to create a 'dummy' directory on each of a list of directories and returning the first successful directory as the test_root value.

The default list of directories checked by test_root are:

    /storage/sdcard0/tests
    /storage/sdcard1/tests
    /sdcard/tests
    /mnt/sdcard/tests
    /data/local/tests

You may override the default list by providing a test_root argument to the ADBDevice constructor which will then be used when attempting to create the 'dummy' directory.

Comment 4

[Julien Pagès (:parkouss)]

Attachment: The property ADBDevice.test_root does not appear in documentation

Thanks bob, I made the changes and this allows to understand really what happens under the hood. :)

I generated the doc and result looks fine.

Comment 5

[Bob Clary [:bc] (inactive)]

Comment on attachment 8583814 [details] [diff] [review]
The property ADBDevice.test_root does not appear in documentation

Review of attachment 8583814 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. r+

Will you need me to land this for you?

Comment 6

[Bob Clary [:bc] (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9685bea24eac

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9685bea24eac

Comment 8

[Bob Clary [:bc] (inactive)]

applied to autophone's instance as well.
https://github.com/mozilla/autophone/commit/480d42509fdc0275137b9b2877f49c7dfcea2221