Closed Bug 816630 Opened 7 years ago Closed 7 years ago

B2G XPCShell tests don't use the proper binary

Categories

(Testing :: XPCShell Harness, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(firefox18 fixed, firefox19 fixed)

RESOLVED FIXED
mozilla20
Tracking Status
firefox18 --- fixed
firefox19 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(2 files)

From investigation in bug 816086 I found a stupid mistake I made where the desktop version of xpcshell is getting copied to the device.
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+
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!
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
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
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
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.