814805 - Android xpcshell tests: chmod permission problems on rooted device

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Geoff Brown [:gbrown]]

On some of my devices, make xpcshell-tests-remote sets up the remote xpcshell environment correctly, but on other devices, I run into permission problems:

...
Unable to chmod /data/local/tests/xpcshell/b/xpcshell: Operation not permitted
chmod /data/local/tests/xpcshell/b/xpcshell
/local/tests/xpcshell/b/xpcw']
Unable to chmod /data/local/tests/xpcshell/b/xpcw: Operation not permitted
chmod /data/local/tests/xpcshell/b/xpcw
chmod /data/local/tests/xpcshell/b
TEST-INFO | /media/extra/objdir-native-droid/_tests/xpcshell/gfx/tests/unit/test_nsIScriptableRegion.js | running test ...
TEST-UNEXPECTED-FAIL | /media/extra/objdir-native-droid/_tests/xpcshell/gfx/tests/unit/test_nsIScriptableRegion.js | test failed (with xpcshell return code: 126), see following log:
>>>>>>>
/system/bin/sh: /data/local/tests/xpcshell/b/xpcw: cannot execute - Permission denied
<<<<<<<

It looks to me like some file operations are executing with run-as and some are not. This problem is happening on a rooted device and I have verified that devicemanagerADB._haveRootShell == True.

chmod is executed with run-as org.mozilla.fennec, but the files are owned by root:

-rwSr-Sr-T root     root       533527 2012-11-24 00:07 xpcshell
-rwSr-Sr-T root     root          538 2012-11-24 00:11 xpcw

Comment 1

[Geoff Brown [:gbrown]]

_useRunAs is True, but the files are owned by root because we recently changed checkCmdAs() calls to shellCheckOutput() calls:

https://hg.mozilla.org/mozilla-central/file/0d373cf880fd/testing/mozbase/mozdevice/mozdevice/devicemanagerADB.py#l177

shellCheckOutput uses shell(), which does not insert run-as.

Comment 2

[Geoff Brown [:gbrown]]

That change was made in bug 800655. Reverting the shellCheckOutput calls to checkCmdAs effectively fixes this problem for me.

(There is a similar issue introduced by bug 810546, which speeds up pushDir when zip is not available. The optimized pushDir code creates files owned by root, which might not be chmod'able by the run-as package user. Thankfully, xpcshell tests do not use pushDir when populating the xpcshell/b directory, which requires chmod; xpcshell tests use pushDir to push the test directories, but the permissions on these files are not modified -- so there appears to be no practical problem created by the patch for bug 810546.)

Comment 3

[Geoff Brown [:gbrown]]

Attachment: revert to use _checkCmdAs again

I scanned bug 800655, but do not understand why the _checkCmdAs calls were changed to shellCheckOutput. This patch fixes my problem...will it cause something else to fail?

Comment 4

[William Lachance (:wlach)]

(In reply to Geoff Brown [:gbrown] from comment #3)
> Created attachment 685836 [details] [diff] [review]
> revert to use _checkCmdAs again
> 
> I scanned bug 800655, but do not understand why the _checkCmdAs calls were
> changed to shellCheckOutput. This patch fixes my problem...will it cause
> something else to fail?

I think I did this because shellCheckOutput has better error checking.

I think we need to take a step back and think about what we're trying to do here. If the goal is to eventually get these xpcshell tests running in automation, we're going to need to do so via SUT, where this run-as trick is not going to work. What do you think about adding a step to fix the permissions to what we want before trying to run the xpcshell binary?

Comment 5

[William Lachance (:wlach)]

Comment on attachment 685836 [details] [diff] [review]
revert to use _checkCmdAs again

I'm going to cancel this review until we figure out what we really want to do here.

Comment 6

[Geoff Brown [:gbrown]]

Attachment: do not use run-as for android xpcshell tests

Thinking about this some more, I wondered why are we using run-as at all? I can see an argument for using run-as when running browser tests, but xpcshell tests are quite removed from org.mozilla.fennec. We just need permissions to populate /data/local and execute a binary; running as the adb shell user is sufficient for that, and avoids these run-as complications. devicemanagerADB already supports this -- we just need to initialize it correctly.

Tested on my tegra.

Comment 7

[William Lachance (:wlach)]

Comment on attachment 693094 [details] [diff] [review]
do not use run-as for android xpcshell tests

If this works for you, awesome.

Comment 8

[Geoff Brown [:gbrown]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f6d35a8b7dad

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f6d35a8b7dad