694716 - Android mochitest-2 is broken, because it foolishly thought that xpconnect would forever be in js/src/xpconnect

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Phil Ringnalda (:philor)]

Let me think how to put this...

Hard-coding paths into the source tree from within a separate tree is a suboptimal solution to any problem, whatever that problem may be.

Bug 691411 moved js/src/xpconnect to js/xpconnect.

Android M2 has a hunk which runs --test-path js/src/xpconnect/tests/mochitest.

Android M2 is hidden on mozilla-central, mozilla-inbound, and try, because those are the only trees I'll have enough time to hide it on.

Comment 1

[Mike Taylor [:bear]]

Attachment: change mochitest-2 test path

Comment 2

[Justin Wood (:Callek)]

Comment on attachment 567247 [details] [diff] [review]
change mochitest-2 test path

You didn't set a real reviewer here, but this does look right to me.

Comment 3

[Phil Ringnalda (:philor)]

How will that work with m-a and m-b? Trunkish branches that haven't merged from m-c yet we can just break until they merge, that's no big deal, but m-a isn't going to have xpconnect move until November 8, and m-b isn't going to have it move until December 20th.

Comment 4

[Mike Taylor [:bear]]

(In reply to Phil Ringnalda (:philor) from comment #3)
> How will that work with m-a and m-b? Trunkish branches that haven't merged
> from m-c yet we can just break until they merge, that's no big deal, but m-a
> isn't going to have xpconnect move until November 8, and m-b isn't going to
> have it move until December 20th.

*sigh* good point - i may have to hack in something that adjusts the path for them and then remove it post-merge

Comment 5

[Justin Wood (:Callek)]

Comment on attachment 567247 [details] [diff] [review]
change mochitest-2 test path

clearing my review+ per philor's excellent point :/

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

just change:
                             'dom/src/json/test', 'dom/src/jsurl/test',
                             'dom/tests/mochitest/dom-level0', 'js/jsd/test',
                             'js/src/xpconnect/tests/mochitest'
                             'js/xpconnect/tests/mochitest'

to:
                             'dom/src/json/test', 'dom/src/jsurl/test',
                             'dom/tests/mochitest/dom-level0', 'js'


originally when we were doing this, we specified the lowest level directory.  In this case we already run all the js/ tests.

and wtf, why did we allow a checkin that breaks android?

Comment 7

[Chris Cooper [:coop] (he/him)]

bear: you can assume r+ from me for a patch that implements joel's condensed list.

Comment 8

[Mike Taylor [:bear]]

Attachment: implement joel's cleaner fix for broken mochitests

given a r+ by coop in the comments
committed changeset 4904:db05ab9197c9

Comment 9

[Mike Taylor [:bear]]

patch landed manually on test-master01 until I can do a full reconfig on monday

Comment 10

[Mike Taylor [:bear]]

full reconfig done monday, closing