Closed Bug 1372814 Opened 7 years ago Closed 7 years ago

Add a test case to test-perf.html to measure streaming time with a store containing {logLimit} messages

Categories

(DevTools :: Console, enhancement, P1)

enhancement

Tracking

(firefox56 fixed)

RESOLVED FIXED
Firefox 56
Iteration:
56.1 - Jun 26
Tracking Status
firefox56 --- fixed

People

(Reporter: nchevobbe, Assigned: nchevobbe)

Details

(Whiteboard: [console-html])

Attachments

(1 file)

This will help us see if there are any differences from the simpler stream test.
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Flags: qe-verify-
Priority: -- → P1
Whiteboard: [console-html]
Whiteboard: [console-html] → [console-html] [triage]
Iteration: --- → 56.1 - Jun 26
Whiteboard: [console-html] [triage] → [console-html]
Comment on attachment 8878459 [details]
Bug 1372814 - Add test case and performance measurement in test-perf.html .

https://reviewboard.mozilla.org/r/149816/#review155206

Those user timing are **very** useful in the profile, thanks!!

::: devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:155
(Diff revision 1)
>  
> +  const measureLabel = "stream - add single message";
> +  for (let i = 0; i < NUM_STREAMING; i++) {
> +    const measure = startMeasure(measureLabel);
> +    await addMessage(wrapper, testPackets[i]);
> +    measure.stop(false);

In other places the 'false' is implied

::: devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:179
(Diff revision 1)
>      {hud: EventEmitter.decorate({proxy: {}})},
>      {},
>      null,
>      document,
>    );
>    wrapper.init();

While we are at it, maybe an `await wait(500)` here before we start the profiler might give the test some extra startup time to reduce variability (haven't tested this myself, feel free to not add)
Attachment #8878459 - Flags: review?(bgrinstead) → review+
Comment on attachment 8878459 [details]
Bug 1372814 - Add test case and performance measurement in test-perf.html .

https://reviewboard.mozilla.org/r/149816/#review155206

> In other places the 'false' is implied

The stop function takes a boolean whose defulat is true : 
```
    stop: (clear = true) => {
      performance.measure(label, startLabel);
      const entries = performance.getEntriesByName(label);
      if (clear){
        performance.clearMeasures(label);
      }
      return entries[entries.length - 1];
    }
```
This is the only case where we don't want the performance entries to be clear right away because we want to use them after the streaming iterations.
Ah, misread that. Carry on!
Comment on attachment 8878459 [details]
Bug 1372814 - Add test case and performance measurement in test-perf.html .

https://reviewboard.mozilla.org/r/149816/#review155206

> While we are at it, maybe an `await wait(500)` here before we start the profiler might give the test some extra startup time to reduce variability (haven't tested this myself, feel free to not add)

I tried it and launched the test a few time, I think this might help, thanks !
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ee03c4d8ee6c
Add test case and performance measurement in test-perf.html . r=bgrins
Backed out for eslint failures in test_render_perf.html:

https://hg.mozilla.org/integration/autoland/rev/ebd15598fd772fe8c1053b72a1c52adb65559453

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=ee03c4d8ee6c25aeedcebcba266cfcb3d6c560ad&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=108264299&repo=autoland

 TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:65:17 | Missing space before opening brace. (space-before-blocks)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:70:4 | Missing semicolon. (semi)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:116:30 | Missing space before opening brace. (space-before-blocks)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:133:1 | Line 119 exceeds the maximum line length of 90. (max-len)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:133:7 | 'filterToggleTimeOff' is assigned a value but never used. (no-unused-vars)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:138:7 | 'filterToggleTimeOn' is assigned a value but never used. (no-unused-vars)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:159:1 | Line 145 exceeds the maximum line length of 90. (max-len)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:162:52 | Infix operators must be spaced. (space-infix-ops)
Flags: needinfo?(nchevobbe)
Sorry about that. Let me fix those errors
Flags: needinfo?(nchevobbe)
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/54ba22bded5e
Add test case and performance measurement in test-perf.html . r=bgrins
https://hg.mozilla.org/mozilla-central/rev/54ba22bded5e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: