1319247 - 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.

Status: RESOLVED FIXED

(DevTools :: Console, defect, P3)

Description

[Treeherder Bug Filer]

Filed by: philringnalda [at] gmail.com

https://treeherder.mozilla.org/logviewer.html#?job_id=39573521&repo=mozilla-inbound

https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-linux-debug/1479756120/mozilla-inbound_ubuntu32_vm-debug_test-mochitest-devtools-chrome-1-bm08-tests1-linux32-build94.txt.gz

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1319247 - Fix devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_batching.js intermittent;

Adjust `waitFor` interval to speed things up.

Review commit: https://reviewboard.mozilla.org/r/95472/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/95472/

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

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

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

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.

Comment 4

[Brian Grinstead [:bgrins]]

(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

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

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 6

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8814216 [details]
Bug 1319247 - Fix devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_batching.js intermittent;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/95472/diff/1-2/

Comment 7

[Brian Grinstead [:bgrins]]

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)?

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

TRY https://treeherder.mozilla.org/#/jobs?repo=try&revision=d17bc06b6fce647ab5c78659d7b6dfb901ee7fb7 looks good.
Test went from ~40s to ~25s in Linux Debug

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

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/ )

Comment 10

[Brian Grinstead [:bgrins]]

(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

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e3fb7348212
Fix devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_batching.js intermittent; r=bgrins

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/5e3fb7348212

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e55b04dfc2b5