Closed Bug 1319247 Opened 3 years ago Closed 3 years ago

Intermittent devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_batching.js | This test exceeded the timeout threshold. It should be rewritten or split up.

Categories

(DevTools :: Console, defect, P3)

defect

Tracking

(firefox51 unaffected, firefox52 fixed, firefox53 fixed)

RESOLVED FIXED
Firefox 53
Tracking Status
firefox51 --- unaffected
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: nchevobbe)

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Component: Developer Tools → Developer Tools: Console
Priority: -- → P3
Assignee: nobody → chevobbe.nicolas
TRY doesn't show the intermittent with approx 30 repetitions (https://treeherder.mozilla.org/#/jobs?repo=try&revision=34cfa4facb3c5075beca2c2de0bfb17e1750f059)
I think it's worth committing this (if r+), and see if the orange shows up
Just found out that a huge slice of test duration is spent on logging the state change : http://searchfox.org/mozilla-central/source/devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_batching.js#30 .

We introduced this logging some times ago in order to have some data to check if the test fails (there was one failure one day, and we didn't knew what was causing it). Although this logging could be useful, it never helped us (the failure didn't occur a second time), and now it contributes making the test times out.
Given all that, I'm inclined to remove the logging in order to get the test run faster, and if we see some failure at some point, reintroduce it in a smarter way.
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #3)
> Just found out that a huge slice of test duration is spent on logging the
> state change :
> http://searchfox.org/mozilla-central/source/devtools/client/webconsole/new-
> console-output/test/mochitest/browser_webconsole_batching.js#30 .
> 
> We introduced this logging some times ago in order to have some data to
> check if the test fails (there was one failure one day, and we didn't knew
> what was causing it). Although this logging could be useful, it never helped
> us (the failure didn't occur a second time), and now it contributes making
> the test times out.
> Given all that, I'm inclined to remove the logging in order to get the test
> run faster, and if we see some failure at some point, reintroduce it in a
> smarter way.

Interesting - yes I'd be OK with removing the logging and associated logic there
I pushed to TRY a patch without logging and with the changes in head.js https://treeherder.mozilla.org/#/jobs?repo=try&revision=d17bc06b6fce647ab5c78659d7b6dfb901ee7fb7
Comment on attachment 8814216 [details]
Bug 1319247 - Fix devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_batching.js intermittent;

https://reviewboard.mozilla.org/r/95472/#review96186

Thanks!

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_batching.js:46
(Diff revision 1)
>        content.wrappedJSObject.batchLog(numMessages);
>      }
>    );
>  
>    for (let i = 0; i < messageNumber; i++) {
> -    let node = yield waitFor(() => findMessageAtIndex(hud, i, i));
> +    let node = yield waitFor(() => findMessageAtIndex(hud, i, i), null, 5);

As discussed, can we try changing the helper function to do something like 10ms and 500 retries to make the best case better for all tests using this function (so we won't need to change defaults here)?
Attachment #8814216 - Flags: review?(bgrinstead) → review+
Comment on attachment 8814216 [details]
Bug 1319247 - Fix devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_batching.js intermittent;

https://reviewboard.mozilla.org/r/95472/#review96186

> As discussed, can we try changing the helper function to do something like 10ms and 500 retries to make the best case better for all tests using this function (so we won't need to change defaults here)?

I did changed it already (you commented on Diff r1, see r2 https://reviewboard.mozilla.org/r/95472/diff/2/ )
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #9)
> Comment on attachment 8814216 [details]
> Bug 1319247 - Fix
> devtools/client/webconsole/new-console-output/test/mochitest/
> browser_webconsole_batching.js intermittent;
> 
> https://reviewboard.mozilla.org/r/95472/#review96186
> 
> > As discussed, can we try changing the helper function to do something like 10ms and 500 retries to make the best case better for all tests using this function (so we won't need to change defaults here)?
> 
> I did changed it already (you commented on Diff r1, see r2
> https://reviewboard.mozilla.org/r/95472/diff/2/ )

Sorry, that was an old comment that got included in the review -- this is good to go
Pushed by chevobbe.nicolas@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5e3fb7348212
Fix devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_batching.js intermittent; r=bgrins
https://hg.mozilla.org/mozilla-central/rev/5e3fb7348212
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.