Closed Bug 1456520 Opened Last year Closed Last year

testing/marionette/harness/marionette_harness/tests/unit/test_capabilities.py TestCapabilities.test_additional_capabilities | AssertionError: u'/storage/sdcard/tests/profile' != '/sdcard/tests/profile' with adb.py

Categories

(Testing :: Mozbase, defect)

Version 3
defect
Not set

Tracking

(firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: gbrown, Assigned: gbrown)

References

Details

Attachments

(1 file)

When I convert Android Marionette tests to use adb.py instead of devicemanager, one test fails:

https://treeherder.mozilla.org/logviewer.html#?job_id=175209487&repo=try&lineNumber=1874

[task 2018-04-23T23:19:41.157Z] 23:19:41  WARNING -  TEST-UNEXPECTED-FAIL | testing/marionette/harness/marionette_harness/tests/unit/test_capabilities.py TestCapabilities.test_additional_capabilities | AssertionError: u'/storage/sdcard/tests/profile' != '/sdcard/tests/profile'
[task 2018-04-23T23:19:41.157Z] 23:19:41     INFO -  Traceback (most recent call last):
[task 2018-04-23T23:19:41.159Z] 23:19:41     INFO -    File "/builds/worker/workspace/build/venv/lib/python2.7/site-packages/marionette_harness/marionette_test/testcases.py", line 159, in run
[task 2018-04-23T23:19:41.159Z] 23:19:41     INFO -      testMethod()
[task 2018-04-23T23:19:41.159Z] 23:19:41     INFO -    File "/builds/worker/workspace/build/tests/marionette/tests/testing/marionette/harness/marionette_harness/tests/unit/test_capabilities.py", line 62, in test_additional_capabilities
[task 2018-04-23T23:19:41.160Z] 23:19:41     INFO -      self.assertEqual(self.caps["moz:profile"].lower(), current_profile.lower())
[task 2018-04-23T23:19:41.160Z] 23:19:41     INFO -  TEST-INFO took 18899ms
The difference between adb.py and devicemanager goes back to the test root. 

On the 4.3 emulator, /sdcard is a link to /storage/sdcard and devicemanagerADB.getDeviceRoot() returns '/storage/sdcard/tests' while ADBDevice.test_root is '/sdcard/tests'.
Looking at the test:

https://dxr.mozilla.org/mozilla-central/rev/dfb15917c057f17e5143f7d7c6e1972ba53efc49/testing/marionette/harness/marionette_harness/tests/unit/test_capabilities.py#62

  self.assertEqual(self.caps["moz:profile"].lower(), current_profile.lower())

current_profile starts with ADBDevice.test_root.

I think self.caps["moz:profile"] comes from

https://dxr.mozilla.org/mozilla-central/rev/dfb15917c057f17e5143f7d7c6e1972ba53efc49/testing/marionette/session.js#378

  ["moz:profile", maybeProfile()],

https://dxr.mozilla.org/mozilla-central/rev/dfb15917c057f17e5143f7d7c6e1972ba53efc49/testing/marionette/session.js#545

  return Services.dirsvc.get("ProfD", Ci.nsIFile).path;

and that starts with '/storage/sdcard'.


:bc -- Thoughts?
Flags: needinfo?(bob)
Geoff, `self.caps["moz:profile"]` is the profile path as used internally by Firefox (like what you see in about:support).
nexus one/cyanogenmod/sdk 10
$ adb -s HT9CPP809750 shell readlink /sdcard
/mnt/sdcard

gs3/lineageOS/sdk 25
$ adb -s 501a2aa7 shell readlink /sdcard
/storage/self/primary

if current_profile.startswith('/sdcard'):
    target = adb.shell_output("readlink /sdcard", ....)
    current_profile = current_profile.replace('/sdcard', target, 1)    
current_profile = current_profile.lower()
Flags: needinfo?(bob)
Assignee: nobody → gbrown
This is pretty hacky, but it works: on Android, when the mozdevice profile path does not match the firefox profile path, and if the root of the mozdevice profile path is a symlink, substitute the linked directory for the symlink. I had to resort to 'ls -l' to identify the symlink, since readlink is not available on sdk 18.

I am left wondering, should I instead skip this test on Android?

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cab9a5fb53a18ab4d8196f21fa00dac2e03bbdfe
Attachment #8970710 - Flags: review?(hskupin)
Attachment #8970710 - Flags: review?(bob)
Comment on attachment 8970710 [details] [diff] [review]
update test_additional_capabilities to allow for fennec profile on /sdcard

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

::: testing/marionette/harness/marionette_harness/tests/unit/test_capabilities.py
@@ +60,5 @@
> +            match = re.match(r'.*->\s(.*)', ls_out)
> +            if match:
> +                new_root = match.group(1)
> +                profile = profile.replace(root, new_root)
> +        return profile

For this test I would be happy if we could just do a check with `assertIn`. Wouldn't that work too in all the cases?
assertIn wouldn't work in the case I mentioned in comment 4 would it?

gs3/lineageOS/sdk 25
$ adb -s 501a2aa7 shell readlink /sdcard
/storage/self/primary
Comment on attachment 8970710 [details] [diff] [review]
update test_additional_capabilities to allow for fennec profile on /sdcard

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

wfm. The only other way I can think of is to create a dummy file using one path and retrieving it via the other.

I can see the potential for this test to catch issues, but it is unfortunate we have to do this. Henrik, have you ever seen this fail on Android? If not, maybe drop the test on Android.
Attachment #8970710 - Flags: review?(bob) → review+
(In reply to Bob Clary [:bc:] from comment #7)
> assertIn wouldn't work in the case I mentioned in comment 4 would it?
> 
> gs3/lineageOS/sdk 25
> $ adb -s 501a2aa7 shell readlink /sdcard
> /storage/self/primary

What does Firefox report as profile path in that case?
Not sure how to get the value the framework gives. Logcat shows paths with /sdcard not /storage. Marionette crashes.
It is kind-of reassuring to have a test that verifies that fennec is actually using the same profile directory as specified by the harness with -profile, and I doubt we test for that anywhere else: let's keep the test.

I imagine there are other cases like comment 7 where an assertIn solution would give us trouble: for whatever reason,  /sdcard's physical location seems to move around to various odd places on different devices.

I'd like to go ahead with the existing patch, as a least-worst solution. whimboo?
Comment on attachment 8970710 [details] [diff] [review]
update test_additional_capabilities to allow for fennec profile on /sdcard

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

r=wc addressed.

::: testing/marionette/harness/marionette_harness/tests/unit/test_capabilities.py
@@ +47,4 @@
>      def test_supported_features(self):
>          self.assertIn("rotatable", self.caps)
>  
> +    def get_fennec_profile(self):

Please move this helper before `test_mandated_capabilities`.

@@ +54,5 @@
> +            # universally available (missing from sdk 18)
> +            import posixpath
> +            import re
> +            device = self.marionette.instance.runner.device.app_ctx.device
> +            root = posixpath.sep.join(profile.split(posixpath.sep)[0:2])

Please add an example here what specifically is getting changed for the path.
Attachment #8970710 - Flags: review?(hskupin) → review+
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d1c3d342559
Make test_capabilities.py more resilient to mozdevice changes; r=bc,whimboo
https://hg.mozilla.org/mozilla-central/rev/9d1c3d342559
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
See Also: → 1526084
You need to log in before you can comment on or make changes to this bug.