[e10s] Enable browser_webconsole_output_01.js in e10s

RESOLVED FIXED in Firefox 47

Status

defect
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: linclark, Assigned: linclark)

Tracking

(Blocks 1 bug)

Trunk
Firefox 47
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox47 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

3 years ago
No description provided.
Assignee

Updated

3 years ago
Blocks: 1112599
Blocks: e10s-tests
tracking-e10s: --- → +
Assignee

Comment 1

3 years ago
Posted patch Bug1243974.patch (obsolete) — Splinter Review
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

3 years ago
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+

Comment 6

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b9b6dc96f46d
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47

Updated

Last year
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.