These old tests are one of the longest polls in the mochitest-1 tent, mostly due to being organized around very long timeouts (which can also lead to random oranges).
-------- Forwarded Message --------
> From: Boris Zbarsky <firstname.lastname@example.org>
> To: email@example.com
> Subject: Re: Notes on ways to improve the build/automation turn around
> Date: Fri, 27 May 2011 15:33:11 -0400
> Newsgroups: mozilla.dev.planning
> On 5/27/11 3:21 PM, jmaher wrote:
> > 35404 INFO TEST-END | /tests/content/base/test/test_websocket.html |
> > finished in 128442ms
> > 36529 INFO TEST-END | /tests/content/base/test/
> > test_ws_basic_tests.html | finished in 116905ms
> As I recall those are just buggy: they use large setTimeouts all over
> and stuff. Can we just fix them?
so I inherited these tests, but I run them all the time - so I know how
annoying they are :)
Improving them is on my list - but I'm actually battling with one of
them right now (that fails intermittently in conjunction with a feature
that is not enabled on a anything) so I certainly won't touch them until
that is figured out.
Created attachment 549942 [details] [diff] [review]
I took a pass at this -
* parallelized a bunch of tests with timeouts in them
* lowered some timeouts that could be controlled by pref
* made the test completion be a little more event driven in the "green" case instead of checking on everything after a 60 second timeout.
note that websockets has a rule about starting more than 1 connection at a time to the same host (the restriction is on the connection phase, not the established phase) so I used a variety of hostnames in the tests :)
it's not great but it seems to bring execution time down from 120-180 sec to 10-15 sec without reducing the testing surface.
I will put it through a fuller set of try runs.
Created attachment 550255 [details] [diff] [review]
Why do we want the finishWSTest thing running off a timer? Could we not just call it from all the places where we set waitTest* to false and bail out from it if not all the waits are done or not all the tests have been started yet?
As written, this test looks like it will fail if it takes longer than 60s due to machine load, for example....
(In reply to comment #3)
> Why do we want the finishWSTest thing running off a timer? Could we not
> just call it from all the places where we set waitTest* to false and bail
> out from it if not all the waits are done or not all the tests have been
> started yet?
> As written, this test looks like it will fail if it takes longer than 60s
> due to machine load, for example....
it always ran off a timer to completion.. I set out to improve "firing too late", not "firing too early" :)
I can update that too, though.
Created attachment 550364 [details] [diff] [review]
this one is fully event driven
> I set out to improve "firing too late", not "firing too early" :)
The difference is that "firing too late" makes the test run slowly, while "firing too early" makes randomorange. ;)
Comment on attachment 550364 [details] [diff] [review]
Most of this looks great. The nits:
>+ if (!ranAllTests)
>+ return false;
Fix the indent? But also, the return value is ignored by all callers; why do we even need one?
>+ if (!(!waitTest2Part1 &&
if (waitTest2Part1 || waitTest2Part2 || ....
and also fix the indentation here.
Don't add that stray space, please.
r=me with those dealt with.
From OS X opt the changeset before this landed on inbound:
/tests/content/base/test/test_websocket.html | finished in 146648ms
With this changeset added:
/tests/content/base/test/test_websocket.html | finished in 9034ms
Mmmm.... the smell of 2+ minutes shaved off the test run time in the morning!