Issue 67198: Add concept of slow tests.

Issue 67198: Add concept of slow tests. (Closed)

Created:
11 years, 8 months ago by ojan

Modified:
9 years, 7 months ago

Reviewers:
tony

CC:
chromium-reviews_googlegroups.com

Visibility:
Public.

Description

Add concept of slow tests. Only mark one test as slow for now. After checkin, I'll add a ton of slow tests and reduce the timeout for non-slow tests to something like 1 second. To aid in that, I've added 90th percentile and 99th percentile times to the statistics we report. Also made it so that tests marked as slow and timeout/crash tests are listed separately from the slowest 50 tests so that list can be used to know which tests to mark slow. Tests cannot be marked as both SLOW and TIMEOUT. If the test times out, then we want it to timeout early to avoid waiting a long timeout for a test that we know is just going to timeout. TestShell can now do per-test timeouts. We could easily make run-webkit-tests do per-test timeouts, but I think just having the one modifier of SLOW is easier to grok. Makes the test lists simpler and allows for having longer timeouts on debug/purify builders. BUG=9324

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+198 lines, -100 lines)			Patch
M	webkit/tools/layout_tests/layout_package/test_expectations.py	View	1	5 chunks	+17 lines, -3 lines	0 comments	Download
M	webkit/tools/layout_tests/layout_package/test_shell_thread.py	View	1	11 chunks	+46 lines, -45 lines	0 comments	Download
M	webkit/tools/layout_tests/run_webkit_tests.py	View	1	17 chunks	+118 lines, -43 lines	0 comments	Download
M	webkit/tools/layout_tests/test_expectations.txt	View	1 2 3	4 chunks	+7 lines, -7 lines	0 comments	Download
M	webkit/tools/test_shell/test_shell_main.cc	View		1 chunk	+10 lines, -2 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

tony

Mostly nits. 10x seems like too big a factor for slow tests. I was thinking ...

11 years, 8 months ago (2009-04-16 17:13:22 UTC) #2

ojan

On 2009/04/16 17:13:22, tony wrote: > Mostly nits. 10x seems like too big a factor ...

11 years, 8 months ago (2009-04-16 18:47:43 UTC) #3

On 2009/04/16 17:13:22, tony wrote:
> Mostly nits.  10x seems like too big a factor for slow tests.  I was thinking
> more like 2x.  I guess we can tweak later.  I would think that 3-5 sec for
fast
> tests and 20-30 sec for slow tests, so it doesn't really matter until we get
the
> fast number down.

I think we should eventually have a 1 sec timeout for fast test and 60-100 sec
for slow tests. As you said though, we can tweak these numbers as we get more
data. I'm pretty sure the 99th percentile of test runs is below 1 second though.

I think it's fair to say that any test that takes more than 1 second should be
fixed or be an exceptional case (e.g. acid3.html is probably fine to leave as is
instead of breaking up into 60 small tests).

http://codereview.chromium.org/67198/diff/1/2
File webkit/tools/layout_tests/layout_package/test_expectations.py (right):

http://codereview.chromium.org/67198/diff/1/2#newcode311
Line 311: self._AddError(lineno, 'A test cannot be both slow and timeout. If the
test '
On 2009/04/16 17:13:22, tony wrote:
> Nit: 80 col

Done.

http://codereview.chromium.org/67198/diff/1/4
File webkit/tools/layout_tests/run_webkit_tests.py (right):

http://codereview.chromium.org/67198/diff/1/4#newcode342
Line 342: The Queue of lists of (test file, test uri) tuples.
On 2009/04/16 17:13:22, tony wrote:
> Nit: This is now a 3-tuple.  Maybe it's time to make it a class/struct?

Done.

http://codereview.chromium.org/67198/diff/1/4#newcode550
Line 550: def _PrintAggregateTestStatistics(self, individual_test_timings):
On 2009/04/16 17:13:22, tony wrote:
> Nit: Missing method docstring.

Done.

http://codereview.chromium.org/67198/diff/1/4#newcode567
Line 567: self._PrintStatisticsForTestTimings("PER TEST DIFF PROCESSING TIMES
(seconds):",
On 2009/04/16 17:13:22, tony wrote:
> Nit: 80 cols

Done.

http://codereview.chromium.org/67198/diff/1/4#newcode574
Line 574: def _PrintIndividualTestTimes(self, individual_test_timings,
failures):
On 2009/04/16 17:13:22, tony wrote:
> Nit: Missing method docstring.

Done.

http://codereview.chromium.org/67198/diff/1/4#newcode604
Line 604: self._PrintTestListTiming("%s slowest tests that are not marked as
SLOW and did "
On 2009/04/16 17:13:22, tony wrote:
> Nit: 80 cols

Done.

http://codereview.chromium.org/67198/diff/1/4#newcode659
Line 659: logging.debug(("Median: %s, Mean: %s, 90th percentile: %s, 99th
percentile: %s, "
On 2009/04/16 17:13:22, tony wrote:
> Nit: 80 cols

Done.

http://codereview.chromium.org/67198/diff/1/4#newcode1040
Line 1040: option_parser.add_option("", "--slow-test-time-out-ms", default=None,
On 2009/04/16 17:13:22, tony wrote:
> Do we really want a flag for this?  It seems like having it be a constant
factor
> relative to time-out-ms will cover all use cases.  I think we already have too
> many command line flags :(

I'm buy that. Done.

http://codereview.chromium.org/67198/diff/1/5
File webkit/tools/layout_tests/test_expectations.txt (right):

http://codereview.chromium.org/67198/diff/1/5#newcode1259
Line 1259: BUG10353 DEFER WIN LINUX :
LayoutTests/http/tests/misc/dns-prefetch-control.html = TIMEOUT PASS
On 2009/04/16 17:13:22, tony wrote:
> Can we change this test to SLOW too?

Dunno. It's currently taking >50 seconds on the windows bot (although, I'm not
sure how the bot is letting it run for more than the 20 second timeout!) and
it's taking >20 seconds on the linux bot. I guess I'll try and and see.

tony

11 years, 8 months ago (2009-04-16 19:00:26 UTC) #4

LGTM.

I'm skeptical that 99% is good enough (99% of 8000 is still 80 tests to mark as
slow).  Anyway, we can tweak it easily.

Expand Messages | Collapse Messages