Closed Bug 820463 Opened 11 years ago Closed 11 years ago

Revert the early loading code

Categories

(Remote Protocol :: Marionette, defect)

Product:
Remote Protocol
Component:
Marionette
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox21 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla21
Tracking Flags:
Tracking Status
firefox21 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: jgriffin, Assigned: jgriffin)

Details

Attachments

(4 files, 3 obsolete files)

Revert early-loading changes,
3.46 KB, patch
jgriffin
: review+
Details | Diff | Splinter Review
Remove loadearly code from test runners,
6.63 KB, patch
ahal
: review+
Details | Diff | Splinter Review
Bugggg - Remove --load-earlyargument from Marionette script,
929 bytes, patch
ahal
: review+
Details | Diff | Splinter Review
Remove the --load-early CLI,
1.05 KB, patch
ahal
: review+
Details | Diff | Splinter Review 
We should back out the early loading code landed as part of the work on bug 800138, since it turns out to be unnecessary.
Attached patch Revert early-loading changes, (obsolete) — — Splinter Review 
This patch works on the panda, need to test on unagi and through try still.
Try run for 8be1cb2f5420 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=8be1cb2f5420
Results (out of 19 total builds):
    success: 18
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jgriffin@mozilla.com-8be1cb2f5420
Try run for 8be1cb2f5420 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=8be1cb2f5420
Results (out of 20 total builds):
    success: 19
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jgriffin@mozilla.com-8be1cb2f5420
Attached patch Revert early-loading changes, (obsolete) — — Splinter Review 
Patch passes try and works fine locally on unagi and panda
Attachment #693175 - Flags: review?(mdas)
Attachment #692527 - Attachment is obsolete: true
Comment on attachment 693175 [details] [diff] [review]
Revert early-loading changes,

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

lgtm, are there any profiles in gaia or b2g repos where they reference this preference and if so are they removed as well?
Attachment #693175 - Flags: review?(mdas) → review+
(In reply to Malini Das [:mdas] from comment #6)
> Comment on attachment 693175 [details] [diff] [review]
> Revert early-loading changes,
> 
> Review of attachment 693175 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> lgtm, are there any profiles in gaia or b2g repos where they reference this
> preference and if so are they removed as well?

There aren't AFAIK. 

After this lands, there will still be two things left to do:

- update the mozharness scripts not to pass --load-early any longer
- remove the --load-early CLI from Marionette
https://hg.mozilla.org/integration/mozilla-inbound/rev/22f9cf3cc575
Whiteboard: [automation-needed-in-aurora][automation-needed-in-b2g18][leave open]
Fixed problem with desktop Marionette; pushed to try for desktop (https://tbpl.mozilla.org/?tree=Try&rev=7b72639a2ee3) and B2G (https://tbpl.mozilla.org/?tree=Try&rev=6ac23e581f0e)
Attachment #697186 - Attachment is obsolete: true
Attachment #693175 - Attachment is obsolete: true
Comment on attachment 697188 [details] [diff] [review]
Revert early-loading changes,

Carry r+ forward.
Attachment #697188 - Flags: review+
Another try push for B2G, the last one only got the empty try commit...

https://tbpl.mozilla.org/?tree=Try&rev=850ecef2f239
Try run for 7b72639a2ee3 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=7b72639a2ee3
Results (out of 4 total builds):
    success: 4
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jgriffin@mozilla.com-7b72639a2ee3
https://hg.mozilla.org/releases/mozilla-b2g18/rev/b2f781164204
Whiteboard: [automation-needed-in-aurora][automation-needed-in-b2g18][leave open] → [leave open]
The previous patch made the pref marionette.loadearly do nothing; this patch removes all traces of loadearly from the testrunners, except for the Marionette CLI which is being used by mozharness.  After this lands, we can patch the mozharness scripts, then remove the CLI as well.
Attachment #703500 - Flags: review?(ahalberstadt)
(In reply to Jonathan Griffin (:jgriffin) from comment #18)
> Created attachment 703500 [details] [diff] [review]
> Remove loadearly code from test runners,
> 
> The previous patch made the pref marionette.loadearly do nothing; this patch
> removes all traces of loadearly from the testrunners, except for the
> Marionette CLI which is being used by mozharness.  After this lands, we can
> patch the mozharness scripts, then remove the CLI as well.

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=07b6f57c7076
Comment on attachment 703500 [details] [diff] [review]
Remove loadearly code from test runners,

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

Nuke it!
Attachment #703500 - Flags: review?(ahalberstadt) → review+
Why did this land on b2g18 without tef+?   This breaks us it seems :(
Opps.  My bad, wrong bug. This doesn't break us!

But my question about why this landed on b2g18 without tef+ stands.
(In reply to Michael Vines [:m1] from comment #24)
> Opps.  My bad, wrong bug. This doesn't break us!
> 
> But my question about why this landed on b2g18 without tef+ stands.

Sorry, it wasn't totally clear that the tef+ requirement applied to test-only bugs.  Should we refrain from landing any automation changes on b2g18 without tef+?
That would be my preference at this point yes.  Any automation changes, even though test-only, could affect our automation as well and unexpected automation bustage generally causes unnecessary hardship during crunch time.
The next step is to remove the --load-early command-line arg from the mozharness Marionette script, since it currently does nothing.  I'll test this on ash.
Attachment #705649 - Flags: review?(ahalberstadt)
Attachment #705649 - Flags: review?(ahalberstadt) → review+
Finally landed the mozharness change:  http://hg.mozilla.org/build/mozharness/rev/24c361d3fc70

I'll watch the trees for bustage.   After this sticks, there's one more trivial gecko change that needs to land.
This just removes the now-unused --load-early command-line arg from Marionette.
Attachment #709988 - Flags: review?(ahalberstadt)
Attachment #709988 - Flags: review?(ahalberstadt) → review+
The last piece of this!

https://hg.mozilla.org/integration/mozilla-inbound/rev/5b7bf70c3be2
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/5b7bf70c3be2
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.