Currently we assume the SD Card location is: /sdcard/ On devices with multiple storage locations (eg Peak, Flame) it is: /storage/sdcard0/ (internal storage) /storage/sdcard1/ (removable sdcard) We need to update a few snippets in gaia_test.py to detect the location and push/pull appropriately. The storage location in use must also be the one configured in the Gaia settings for the tests to see the files. There may be some features in mozdevice we can use to find the location of the storage.

Ahal, wlach, should getDeviceRoot return a path to the sdcard on firefox OS devices? The two examples in comment #0 are not covered, presently. At the moment with mozdevice==0.33 the value returned for getDeviceRoot is /data/local/tests but we don't use the value. We work straight from /sdcard/. Alternatively I just put this logic straight into the test framework itself by checking with dirExists or something like that.

We can make getDeviceRoot do whatever we want. :) I think we should make it return a path to the sdcard on firefox os devices, indeed. Do you need/want to be able select between removable and non-removable storage somehow? (In reply to Zac C (:zac) from comment #1) > Ahal, wlach, should getDeviceRoot return a path to the sdcard on firefox OS > devices? > > The two examples in comment #0 are not covered, presently. > > At the moment with mozdevice==0.33 the value returned for getDeviceRoot is > /data/local/tests but we don't use the value. We work straight from /sdcard/. > > Alternatively I just put this logic straight into the test framework itself > by checking with dirExists or something like that.

Ping on the above question.

Sure I know we can do it, but *should* we..! At present we just need a safe way to get the default location. mozdevice may need to make assumptions, whereas in the test framework can probe the device and its settings a bit more to get the real location perhaps. At present we don't need to switch between removable and non-removable. If we did we would probably want to explicitly target that directory so we'd know the location of it.

Just to clarify, for "default location" I meant "default storage location" which should be non-removable if it exists, or removable if it does not. That's the assumption I'm concerned about; is it safe. Anyway, should I file a bug against mozdevice for this feature?

(In reply to Zac C (:zac) from comment #4) > Sure I know we can do it, but *should* we..! At present we just need a safe > way to get the default location. mozdevice may need to make assumptions, > whereas in the test framework can probe the device and its settings a bit > more to get the real location perhaps. > > At present we don't need to switch between removable and non-removable. If > we did we would probably want to explicitly target that directory so we'd > know the location of it. Really getDeviceRoot() is a shortcut for finding some scratch space that various test harnesses can use to store stuff on the device. We should make it return a reasonable value on all platforms. I think a simple patch that does something like this should probably do the trick. Give it a try maybe?

Comment on attachment 8410402 [details] [diff] [review] Add /sdcard to list of directories we search for a testroot in For Firefox OS devices without internal storage: '/sdcard' for firefox OS devices with internal storage: '/storage/sdcard0' '/storage/sdcard1' It's the latter we need to support, without blocking the former. Also should this be above '/data/local' ?

Does /data/local exist in devices with internal storage? If not then it will be ignored anyway, so doesn't really matter. If yes, then why can't we continue using that instead of /storage/sdcard0? Sounds like the only ordering that's important here is that '/storage/sdcard0' and '/storage/sdcard1' appear before '/sdcard'

/data/local exists on all devices. This is where we store 3rd party apps, indexedDB data, etc.

I'll have to check tomorrow, but I'm not sure that if [for example] we push a jpg file to /data/local/tests the Gallery app will find the file there.

Ok just tested with Hamachi and Flame device. The existing logic is OK on Hamachi because /mnt/sdcard maps to /sdcard (or vice versa). For the Flame /data/local exists, but if I push a file to it (/data/local/tests) the Gallery app won't detect it. I suppose the API does not scan this directory for media. Thus we need to push directly to one of the sdcards.

Testing using getDeviceRoot, I've struck a new problem here. "/mnt/sdcard" appears to be a symlink and we can push files to that fine, but when we find that file back using the WebAPI it returns the "/sdcard" path to the file so we get a few test failures.

I did experiment using getDeviceRoot but because the location of the push changes (based on mozdevice's logic) there needs to be a change to the unit tests and to the atoms. Thus I decided to experiment with the logic inside gaiatest for the time being, hence the f? This method requires minimal changes to the atoms and unit tests. Of course we can still move this logic into mozdevice if it doesn't affect other projects. I really just wanted to see how blocked we were on the flame for now. I tested this on both hamachi and flame.

So changing the ordering to: - /storage/sdcard0 - /storage/sdcard1 - /sdcard - /mnt/sdcard - /data/local should work? Also note that there's a 'deviceRoot' option in the dm constructor that can be used to override this default.

That would work ahal. Would that affect other mozdevice users?

I don't think so.. but it's possible. I wouldn't worry about it, if something does come up it will be easy to fix.

OK! I'm glad you can't edit bugzilla comments ;)

(In reply to Zac C (:zac) from comment #13) > Created attachment 8410955 [details] > github pr > > I did experiment using getDeviceRoot but because the location of the push > changes (based on mozdevice's logic) there needs to be a change to the unit > tests and to the atoms. > > Thus I decided to experiment with the logic inside gaiatest for the time > being, hence the f? This method requires minimal changes to the atoms and > unit tests. > > Of course we can still move this logic into mozdevice if it doesn't affect > other projects. I really just wanted to see how blocked we were on the flame > for now. > > I tested this on both hamachi and flame. I re-did this to take into account the changes ahal proposed in comment #14. Obviously it won't work until a new mozdevice is released :)

Comment on attachment 8410955 [details] github pr Dave, I had to change the atom to take into account the tests/ suffix that getDeviceRoot adds. but it was due for an r? anyway. It can wait until a new mozdevice is released.

Comment on attachment 8410955 [details] github pr See outstanding request for clarification of the atom change.

Comment on attachment 8410955 [details] github pr I've clarified the atom change in the Github comments.

Comment on attachment 8410955 [details] github pr I'm still unsure about the atom change. It doesn't seem to be necessary for this bug, but so long as we test that b2gpopulate doesn't regress then it's okay with me.

(In reply to Andrew Halberstadt [:ahal] (PTO until May 6th) from comment #14) > So changing the ordering to: > > - /storage/sdcard0 > - /storage/sdcard1 > - /sdcard > - /mnt/sdcard > - /data/local > > should work? Also note that there's a 'deviceRoot' option in the dm > constructor that can be used to override this default. So, personally, I don't think that hardcoding paths is the right thing to do. A couple of corrections from previous comments: 1 - /sdcard is almost always a symlink to the real sdcard volume location. 2 - /mnt/sdcard should always be a mount point and not a symlink. On devices which have sharable sdcards, (i.e. sdcard is in a separate partition), then you should be able to do: adb shell vdc volume list | grep sdcard On devices like the hamachi, this will return something like this: 110 sdcard /mnt/sdcard 4 which tells us that /mnt/sdcard is the mount point. Some devices have sdcard and extsdcard volumes, so you'll probably need to refine the grep to make sure you're getting the right place. /storage/sdcard0 and /storage/sdcard1 is peak/keon specific, not generic. Helix and Leo both use different schemes from that and from each other IIRC. On devices, like the Nexus-4, which have no sharable sdcard, you should probably just use /sdcard. On the nexus4, /sdcard is a symlink to /storage/emulated/legacy and /storage/emulated/legacy is a symlink to /mnt/shell/emulated/0

Oh yeah, /data/local is never a place that device storage would look for files, except if someone sets up a fake volume to allow that to happen (I've done this for testing, but it doesn't happen on normal phones).

:dhylands, those comments are probably better placed in bug https://bugzilla.mozilla.org/show_bug.cgi?id=1000918 As in this bug we agreed to cede the logic to mozdevice!

Comment on attachment 8410955 [details] github pr Bebe and Bob, this pull should work seamlessly with Flame and Hamachi now that mozdevice 0.34 has been released. Can you give it a r? thanks!

Comment on attachment 8410955 [details] github pr this is failing on Flame (z1)florinstrugariu@P5171:~/gaia/gaia/tests/python/gaia-ui-tests$ gaiatest --testvars=/home/florinstrugariu/testvars.json --address=localhost:2828 --type=b2g --timeout=10000 --html-output=results/index.htm --restart /home/florinstrugariu/gaia/gaia/tests/python/gaia-ui-tests/gaiatest/tests/unit/test_cleanup_sdcard.pystarting httpd running webserver on http://192.168.77.192:48097/ TEST-START test_cleanup_sdcard.py test_cleanup_scard (test_cleanup_sdcard.TestCleanupSDCard) ... FAIL ====================================================================== FAIL: None ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/florinstrugariu/.virtualenvs/z1/local/lib/python2.7/site-packages/marionette_client-0.7.7-py2.7.egg/marionette/marionette_test.py", line 163, in run testMethod() File "/home/florinstrugariu/gaia/gaia/tests/python/gaia-ui-tests/gaiatest/tests/unit/test_cleanup_sdcard.py", line 11, in test_cleanup_scard self.assertEqual(len(self.data_layer.media_files), 0) TEST-UNEXPECTED-FAIL | test_cleanup_sdcard.py test_cleanup_sdcard.TestCleanupSDCard.test_cleanup_scard | AssertionError: 11 != 0 ---------------------------------------------------------------------- Ran 1 test in 29.737s FAILED (failures=1) SUMMARY ------- passed: 0 failed: 1 todo: 0 FAILED TESTS ------- test_cleanup_sdcard.py test_cleanup_sdcard.TestCleanupSDCard.test_cleanup_scard mozversion.mozversion.RemoteB2GVersion INFO | Unable to find system/sources.xml (z1)florinstrugariu@P5171:~/gaia/gaia/tests/python/gaia-ui-tests$ pip freeze ManifestDestiny==0.6 argparse==1.2.1 gaiatest==0.23 marionette-client==0.7.7 marionette-transport==0.1 mozcrash==0.12 mozdevice==0.34 mozfile==1.1 mozhttpd==0.7 mozinfo==0.7 mozlog==1.7 moznetwork==0.24 mozprocess==0.19 mozprofile==0.21 mozrunner==5.36 moztest==0.3 mozversion==0.4 plivo==0.9.6 requests==2.2.1 wsgiref==0.1.2 yoctopuce==1.01.12553

Comment on attachment 8410955 [details] github pr Bebe, re-filed it with some modifications. The mozdevice solution doesn't work perfectly because we need a quite high level use of the sdcard rather than just a folder on it. :( sorry I did not notice this use case earlier.

+sdcard adhoc run: http://selenium.qa.mtv2.mozilla.com:8080/job/b2g.hamachi.mozilla-central.ui.adhoc/394/

Comment on attachment 8410955 [details] github pr Woks fine now

Comment on attachment 8410955 [details] github pr This is fine on my Hamachi, but fails on my Flame. I believe this is because I have an older Flame that is unable to see the sdcard after flashing gaia and gecko, so I don't think it's a valid failure of this patch. So consider this an r+ for Hamachi only.

Merged: https://github.com/mozilla-b2g/gaia/commit/cf485fc000d9dae5ad545a3643ffc6a7eecb2d48 v1.4: https://github.com/mozilla-b2g/gaia/commit/2f0c49e1561720628179d83a68b2abdec78abd51