Closed Bug 678385 Opened 8 years ago Closed 8 years ago

device managers probably need to be updated for package name + username

Categories

(Testing :: General, defect)

All
Android
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla9

People

(Reporter: dougt, Assigned: gbrown)

References

Details

Attachments

(1 file, 3 obsolete files)

see build/mobile/devicemanagerADB.py and similar.


blassey added support for package names to contain the developers username.  so, unofficial builds on my machine are:

org.mozilla.fennec_dougt


if I am running remote tests, we might need to update the devices managers to handle this cause.

i am unsure if it is a problem or not.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → blassey.bugs
Attachment #552536 - Flags: review?(doug.turner)
Blocks: 668349
Whiteboard: [needs-review (dougt)]
It is a problem for me! The remote xpcshell work (bug 668349) uses devicemanagerADB.getAppRoot, which fails if org.mozilla.fennec_unofficial|aurora|beta is not installed. The patch solves this problem.
Pushed to try, along with my remote xpcshell patches, but many "Android opt" tests failed:

http://tbpl.mozilla.org/?tree=Try&rev=55446b4a1b98

We need to guard against os.getenv returning None (as when USER is not set):

File "/builds/tegra-049/test/build/tests/mochitest/devicemanagerADB.py", line 15, in __init__     packageName = 'org.mozilla.fennec_' + os.getenv('USER'); TypeError: cannot concatenate 'str' and 'NoneType' objects
Attachment #552536 - Flags: review?(doug.turner) → review+
what about the non-adb version?
(In reply to Doug Turner (:dougt) from comment #4)
> what about the non-adb version?

it doesn't use the package name to my knowledge
OS: Mac OS X → Android
Hardware: x86 → All
(In reply to Geoff Brown [:gbrown] from comment #3)
> Pushed to try, along with my remote xpcshell patches, but many "Android opt"
> tests failed:

To clarify my previous comment: When I pushed this patch to try, many tests failed because the USER environment variable was not set, causing os.getenv to return None, which could not be concatenated with a string.
I have been using this version successfully in my xpcshell testing.
Attachment #553873 - Flags: feedback?(blassey.bugs)
Attachment #553873 - Flags: feedback?(blassey.bugs) → feedback+
Comment on attachment 553873 [details] [diff] [review]
slight change to blassey's patch that allows for os.getenv to return None

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

::: build/mobile/devicemanagerADB.py
@@ +11,5 @@
>      self.retrylimit = retrylimit
>      self.retries = 0
>      self._sock = None
> +    if packageName == None and os.getenv('USER'):
> +      packageName = 'org.mozilla.fennec_' + os.getenv('USER');

actually, I take that back. If USER isn't set, then packageName will be set to None, right? That can't be what we want. this should be
if packageName == None:
   if os.getenv('USER'):
      packageName = 'org.mozilla.fennec_' + os.getenv('USER');
   else:
      'org.mozilla.fennec_';
Attachment #553873 - Flags: feedback+ → feedback-
My thinking was that if USER is not set, it's best to proceed as though we do not know the package name. Setting packageName to None avoids use of the run-as trick (which is okay) and may also affect getAppRoot (depending on where Fennec is installed on the device). Your way may be less likely to cause problems for getAppRoot...I'll try it out.
updated as suggested
Attachment #553873 - Attachment is obsolete: true
Attachment #554301 - Flags: feedback?(blassey.bugs)
Comment on attachment 554301 [details] [diff] [review]
slight change to blassey's patch that allows for os.getenv to return None

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

looks good, get review and land it

::: build/mobile/devicemanagerADB.py
@@ +391,2 @@
>  
>      # Failure (either not installed or not a recognized platform)

please add some logging to indicate we hit this case
Attachment #554301 - Flags: feedback?(blassey.bugs) → feedback+
Identical to patch with feedback+, with requested warning added.
Attachment #552536 - Attachment is obsolete: true
Attachment #554301 - Attachment is obsolete: true
Attachment #554510 - Flags: review?(jmaher)
Assignee: blassey.bugs → gbrown
Whiteboard: [needs-review (dougt)]
Keywords: checkin-needed
Whiteboard: see also bug 668351 for check in instructions
http://hg.mozilla.org/integration/mozilla-inbound/rev/5378c0b9525a
Status: NEW → ASSIGNED
Keywords: checkin-needed
Whiteboard: see also bug 668351 for check in instructions
Target Milestone: --- → mozilla9
Version: unspecified → Trunk
http://hg.mozilla.org/mozilla-central/rev/5378c0b9525a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.