Android xpcshell tests: chmod permission problems on rooted device

RESOLVED FIXED in mozilla20

Status

Testing
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gbrown, Assigned: gbrown)

Tracking

unspecified
mozilla20
x86
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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
(Assignee)

Comment 1

5 years ago
_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.
(Assignee)

Comment 2

5 years ago
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.)
(Assignee)

Comment 3

5 years ago
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?
Attachment #685836 - Flags: feedback?(wlachance)
(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 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.
Attachment #685836 - Flags: feedback?(wlachance)
(Assignee)

Comment 6

5 years ago
Created attachment 693094 [details] [diff] [review]
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.
Attachment #685836 - Attachment is obsolete: true
Attachment #693094 - Flags: review?(wlachance)
Comment on attachment 693094 [details] [diff] [review]
do not use run-as for android xpcshell tests

If this works for you, awesome.
Attachment #693094 - Flags: review?(wlachance) → review+

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/f6d35a8b7dad
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in before you can comment on or make changes to this bug.