Closed Bug 1046146 Opened 5 years ago Closed Last year
Change the way that mozdevice checks for the existence of a directory
46 bytes, text/x-phabricator-request
|Details | Review|
Right now, it looks pretty distracting since we try to list a dir and analyze the output. Using test -d should work. root@android:/ # test -d /armenzg/; echo $? 1 root@android:/ # test -d /sys/; echo $? 0 root@android:/ # test -d /sys; echo $? 0 root@android:/ # test -d /armenzg; echo $? 1
This seems like a great idea: More efficient and more self-documenting. I verified test -d/-f works fine on the android 4.3 and android 7.0 emulators. :bc - Any concerns? :egao - Do you have time/interest for this?
No concerns except that it would be nice if BogdanS can confirm this works for a representative sample of the devices he has available. Just a few of the older ones would be sufficient I think.
Flags: needinfo?(bob) → needinfo?(bogdan.surd)
gbrown: I will take this bug, looks like a good learning opportunity.
Devices: Phones - Prestigio Grace X5 (4.4.2) - Xiaomi Mi4i (5.0.2) - 2x Nexus 5 (6.0.1) - Samsung Galaxy S6 (7.0) - Nokia 6 (7.1.1) - Samsung Galaxy S8 (8) - Huawei P10 (8) - HTC 10 (8) - 2x Google Pixel (9) Tablets - Xiaomi Mi Pad 2 (5.1) x86 - Huawei MediaPad M2 (5.1.1) - Nexus 9 (6.0.1) Verified on Android devices ranging from Android 4 to Android 9, test -d worked perfectly on all of the devices.
Refactored the method by which ADBDevice checks for the presence of files or directories. - created helper method _test_path(), which accepts a path and command line argument to the test command. - modified existing methods to use the _test_path() method.
Comment on attachment 9006695 [details] Bug 1046146 - Change the way that mozdevice checks for the existence of a directory Geoff Brown [:gbrown] has approved the revision.
Attachment #9006695 - Flags: review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/e3cdc51cdc0a Change the way that mozdevice checks for the existence of a directory r=gbrown
You need to log in before you can comment on or make changes to this bug.