1365139 - Intermittent TEST-UNEXPECTED-TIMEOUT | toolkit/components/places/tests/history/test_removeMany.js | Test timed out

Status: RESOLVED FIXED

(Toolkit :: Places, defect, P2)

Description

[Treeherder Bug Filer]

Filed by: philringnalda [at] gmail.com

https://treeherder.mozilla.org/logviewer.html#?job_id=99014249&repo=autoland

https://queue.taskcluster.net/v1/task/Qo7vaZ5pRkqPUwjtMcYaGA/runs/0/artifacts/public/logs/live_backing.log

Comment 1

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

wow, the test is going a little bit over the top, it starts at 03:13:32 and it's still running sub tests at 03:13:56.
It's probably because of const SIZE = 1000;
I guess we may want to make the chunklen somehow settable and reduce this to 100... OR make the test not print something on every loop (output slows down tests quite a bit).
Mark, could you have a look at this?

Comment 2

[Mark Banner (:standard8)]

Yes sure.

Comment 3

[Mark Banner (:standard8)]

Dropping the amount from 1000 -> 400 changes the test time from 16s > 5s on my machine, but will still trigger REMOVE_PAGES_CHUNKLEN. I've commented out one bit of debug as well.

That seems a bit more reasonable to me, shall we try it?

Comment 4

[Mark Banner (:standard8)]

Attachment: Bug 1365139 - Reduce the amount of iterations for test_removeMany.js so that it takes less time overall to avoid timeouts.

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

Comment 5

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 8869432 [details]
Bug 1365139 - Reduce the amount of iterations for test_removeMany.js so that it takes less time overall to avoid timeouts.

https://reviewboard.mozilla.org/r/141068/#review145062

::: toolkit/components/places/tests/history/test_removeMany.js:12
(Diff revision 1)
>  "use strict";
>  
>  // Test removing a list of pages
>  add_task(async function test_remove_many() {
> -  // This is set so that we are guarenteed to trigger REMOVE_PAGES_CHUNKLEN.
> -  const SIZE = 1000;
> +  // This is set so that we are guaranteed to trigger REMOVE_PAGES_CHUNKLEN.
> +  const SIZE = 400;

we could go down to anything above 300 right? so why not 310?

::: toolkit/components/places/tests/history/test_removeMany.js:135
(Diff revision 1)
>    do_print("Checking out results");
>    // By now the observers should have been called.
>    for (let i = 0; i < pages.length; ++i) {
>      let page = pages[i];
> -    do_print("Page: " + i);
> +    // do_print("Page: " + i);
>      Assert.ok(page.onResultCalled, "We have reached the page from the callback");

what about completely removing the do_print and replacing the first Assert message with
`We have reached the page #${i} from the callback`

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 8869432 [details]
Bug 1365139 - Reduce the amount of iterations for test_removeMany.js so that it takes less time overall to avoid timeouts.

https://reviewboard.mozilla.org/r/141068/#review145062

> we could go down to anything above 300 right? so why not 310?

Ok, sure.

> what about completely removing the do_print and replacing the first Assert message with
> `We have reached the page #${i} from the callback`

Yeah, nice suggestion, not sure why I didn't think of that.

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 8869432 [details]
Bug 1365139 - Reduce the amount of iterations for test_removeMany.js so that it takes less time overall to avoid timeouts.

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/48965785dc8b
Reduce the amount of iterations for test_removeMany.js so that it takes less time overall to avoid timeouts. r=mak

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/48965785dc8b