B2G XPCShell tests don't use the proper binary

RESOLVED FIXED in Firefox 18

Status

Testing
XPCShell Harness
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ahal, Assigned: ahal)

Tracking

unspecified
mozilla20
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(firefox18 fixed, firefox19 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
From investigation in bug 816086 I found a stupid mistake I made where the desktop version of xpcshell is getting copied to the device.
(Assignee)

Comment 1

5 years ago
Created attachment 686775 [details] [diff] [review]
Patch 1.0 - copy proper xpcshell, refactor logic to find localLib and localBin

I added --local-lib-dir and --local-bin-dir arguments to the Fennec options, this way fennec doesn't have to depend on running from an objdir either. The logic to find those two things is quite terrible, but I didn't want to break any backwards compatibility with how things currently work for Fennec. At least this way all the logic is in once place.

I tested all the codepaths (e.g specifying objdir, apk, or not). Flagging gbrown for feedback mainly for awareness.
Attachment #686775 - Flags: review?(jgriffin)
Attachment #686775 - Flags: feedback?(gbrown)
Comment on attachment 686775 [details] [diff] [review]
Patch 1.0 - copy proper xpcshell, refactor logic to find localLib and localBin

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

lgtm
Attachment #686775 - Flags: review?(jgriffin) → review+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d743a639162d
https://hg.mozilla.org/releases/mozilla-aurora/rev/55872da25531
https://hg.mozilla.org/releases/mozilla-beta/rev/608c3308c5e1
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
status-firefox18: --- → fixed
status-firefox19: --- → fixed
(Assignee)

Updated

5 years ago
Attachment #686775 - Flags: feedback?(gbrown)
Sorry, I should have looked at this sooner! It causes a problem for Android xpcshell tests. 

verifyRemoteOptions() is called before options.localAPK is set to its default here:
https://hg.mozilla.org/mozilla-central/file/5c8ee6600533/testing/xpcshell/remotexpcshelltests.py#l420

so then this code isn't executed:

https://hg.mozilla.org/mozilla-central/file/5c8ee6600533/testing/xpcshell/remotexpcshelltests.py#l133

and we end up using <objdir>/dist/bin instead of <objdir>/dist/fennec for localLib. As a result, we try to push unstripped libraries, which pushes the limits of time-outs and device space!
Created attachment 687292 [details] [diff] [review]
simple follow-up fix: ensure localAPK is set earlier

This fixes the problem I am seeing for Android.
Attachment #687292 - Flags: review?(ahalberstadt)
https://hg.mozilla.org/mozilla-central/rev/d743a639162d
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
(Assignee)

Comment 7

5 years ago
Comment on attachment 687292 [details] [diff] [review]
simple follow-up fix: ensure localAPK is set earlier

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

Crap, I'm sorry. I guess next time I need to make changes to remotexpcshelltests.py I'll just bite the bullet and figure out how to get Fennec built/running locally.

::: testing/xpcshell/remotexpcshelltests.py
@@ +413,5 @@
>  
>      parser = RemoteXPCShellOptions()
>      options, args = parser.parse_args()
> +    if not options.localAPK:
> +      for file in os.listdir(os.path.join(options.objdir, "dist")):

nit: we can use an else clause on this for statement to avoid the second "if not options.localAPK" below (see http://docs.python.org/2/tutorial/controlflow.html#break-and-continue-statements-and-else-clauses-on-loops)
Attachment #687292 - Flags: review?(ahalberstadt) → review+
Follow-up pushed with nit addressed (thanks -- I've never used that before!)

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa8437087bad
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/fa8437087bad
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.