816630 - B2G XPCShell tests don't use the proper binary

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Andrew Halberstadt [:ahal]]

From investigation in bug 816086 I found a stupid mistake I made where the desktop version of xpcshell is getting copied to the device.

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: 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.

Comment 2

[Jonathan Griffin (:jgriffin)]

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

Comment 3

[Andrew Halberstadt [:ahal]]

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

Comment 4

[Geoff Brown [: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!

Comment 5

[Geoff Brown [:gbrown]]

Attachment: simple follow-up fix: ensure localAPK is set earlier

This fixes the problem I am seeing for Android.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[Andrew Halberstadt [:ahal]]

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)

Comment 8

[Geoff Brown [:gbrown]]

Follow-up pushed with nit addressed (thanks -- I've never used that before!)

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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