[e10s] Enable browser_webconsole_output_01.js in e10s

RESOLVED FIXED in Firefox 47

Status

()

Firefox
Developer Tools: Console
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: linclark, Assigned: linclark)

Tracking

(Blocks: 2 bugs)

Trunk
Firefox 47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox47 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Updated

2 years ago
Blocks: 1112599

Updated

2 years ago
Blocks: 984139
tracking-e10s: --- → +
(Assignee)

Comment 1

2 years ago
Created attachment 8718650 [details] [diff] [review]
Bug1243974.patch

The problem with this one seems to be that setting the values on DebuggerServer isn't actually changing the limits on the backend. I looked up the default values for the two length constants... LONG_STRING_LENGTH = 10000 and LONG_STRING_INITIAL_LENGTH = 1000. When I just used those values (instead of 100 and 50) it worked.

> DebuggerServer.LONG_STRING_LENGTH = 10000;
> DebuggerServer.LONG_STRING_INITIAL_LENGTH = 1000;

I'm thinking that what we're really trying to test here is that the ellipses are shown. In order to properly test this functionality, do we need to change the limits? In case the answer is no, I've uploaded a patch that should pass.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=363f24f7493d
Assignee: nobody → lclark
Status: NEW → ASSIGNED
Attachment #8718650 - Flags: review?(bgrinstead)
Comment on attachment 8718650 [details] [diff] [review]
Bug1243974.patch

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

Good find.  By the way, looks like devtools/client/webconsole/test/browser_webconsole_promise.js has the same boilerplate but it isn't used.  Can you please remove it from that test as well?

::: devtools/client/webconsole/test/browser_webconsole_output_01.js
@@ -16,5 @@
>  const TEST_URI = "data:text/html;charset=utf8,test for console output - 01";
>  
>  var {DebuggerServer} = require("devtools/server/main");
>  
> -var LONG_STRING_LENGTH = DebuggerServer.LONG_STRING_LENGTH;

Maybe this was to prevent spamming test logs with 10000 character strings.  But if we want to fix that we can update the checkOutputForInputs helper function.  It looks like that function calls info multiple times with the full entry.input which seems unnecessary - it's already logging the full input once in the runner function, don't need to do it also in checkConsoleLog / checkPrintOutput / checkJSEval / checkObjectClick / onVariablesViewOpen.  Can you update that helper function to limit the amount of logging that happens?
Attachment #8718650 - Flags: review?(bgrinstead) → feedback+
(Assignee)

Comment 3

2 years ago
Created attachment 8720019 [details] [diff] [review]
Bug1243974.patch

Was this what you were thinking with limiting the logging? If so, I think this is ready.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=102828af8d7a
Attachment #8718650 - Attachment is obsolete: true
Attachment #8720019 - Flags: review?(bgrinstead)
Attachment #8720019 - Flags: review?(bgrinstead) → review+
(Assignee)

Comment 4

2 years ago
Refreshed try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=24d9a85f1a36
Keywords: checkin-needed

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b9b6dc96f46d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox47: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
You need to log in before you can comment on or make changes to this bug.