Closed Bug 1220011 Opened 4 years ago Closed 4 years ago

Permaorange on beta after merge of Gecko 43: browser_cmd_commands.js | html output for console close - Got Error: Invalid Command: ''., expected

Categories

(DevTools :: General, defect, blocker)

defect
Not set
blocker

Tracking

(firefox45 fixed, b2g-v2.5 fixed)

RESOLVED FIXED
Firefox 45
Tracking Status
firefox45 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: philor, Assigned: bgrins)

References

Details

Attachments

(2 files, 1 obsolete file)

Temporarily skipped on release_build in https://hg.mozilla.org/releases/mozilla-beta/rev/dde0e0b495db to maybe see a little green.
This is also permafail on central with the change from bug 12242924 applied:
https://treeherder.mozilla.org/logviewer.html#?job_id=14115272&repo=try

For context, that bug is removing the XUL overlay in the mochitest extension and instead using marionette to bootstrap mochitest. I don't understand how my change could be affecting this test, but given that this failure also happens on beta, maybe the test is racy?

Bug 1224294 is a hard requirement for addons signing, so we'll either need to figure out what's going on or disable this test across the board. Brian, do you know of anyone familiar with this test who could help me take a look? I tried to debug it myself, but this is beyond my JS chops.
Blocks: 1224294
Flags: needinfo?(bgrinstead)
Attached patch commandline-beta-fix.patch (obsolete) — Splinter Review
Joe, I truly don't know why this fixes the issue but it does for me locally on a beta build.  Maybe something with Promise something or another in the audit method..

Andrew, can you confirm this fixes things locally for you?
Flags: needinfo?(bgrinstead)
Attachment #8695007 - Flags: review?(jwalker)
Attachment #8695007 - Flags: review?(ahalberstadt)
Attachment #8695007 - Flags: review?(jwalker) → review+
If jsterm.clearOutput() was async then this would be obviously racy, but I can't see how it is. The audit functions could have a promise hole, but this is the first I've heard of it - something has changed for this to be a permaorange, but I don't get what.
Comment on attachment 8695007 [details] [diff] [review]
commandline-beta-fix.patch

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

Yes this fixes things, thanks for looking into it!

This patch doesn't apply to central (due to the devtools directory split) but I tested it out by applying it to the relevant files manually.
Attachment #8695007 - Flags: review?(ahalberstadt) → review+
(In reply to Andrew Halberstadt [:ahal] from comment #6)
> Comment on attachment 8695007 [details] [diff] [review]
> commandline-beta-fix.patch
> 
> Review of attachment 8695007 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Yes this fixes things, thanks for looking into it!
> 
> This patch doesn't apply to central (due to the devtools directory split)
> but I tested it out by applying it to the relevant files manually.

Can you upload the patch that applies to central?  We should land on central / aurora with that patch (assuming it's green on try for those) so that it doesn't permaorange again after the next merge.
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
ni? for comment 7
Flags: needinfo?(ahalberstadt)
Here you go.

Also, you probably want to remove the skip-if added in comment 1 from the beta version of the patch.
Flags: needinfo?(ahalberstadt)
Removed skip-if
Attachment #8695007 - Attachment is obsolete: true
Attachment #8695304 - Flags: review+
Phil, should I push the beta / aurora fixes?  I guess it's been skipped in Comment 1 but I'd like to get it re-enabled.
Flags: needinfo?(philringnalda)
Do eet, a=testonly.
Flags: needinfo?(philringnalda)
Well, I guess it isn't quite testonly, is it?
(In reply to Phil Ringnalda (:philor) from comment #14)
> Well, I guess it isn't quite testonly, is it?

Too late :)

https://hg.mozilla.org/releases/mozilla-beta/rev/bf8e89308c04
https://hg.mozilla.org/mozilla-central/rev/e09649f19381
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.