Commit 932b3fc6 authored by Erik Chen's avatar Erik Chen Committed by Commit Bot

Restart content shell rather than renderers between layout tests.

Restarting content shell between layout tests increases run time by 2-5x but
produces more consistent results. Previously, this was implemented as a flag to
content shell that would restart the renderer between tests. This CL changes the
test runner to instead restart content shell between tests. This has comparable
run time, with less complexity and even less probability of state carrying over
between tests.

The content shell flag --reset-shell-between-tests is now unused, and will be
removed in a future CL.

Bug: 894527
Change-Id: Idce9a74006000ded0d308affaff0568226a2cf7f
Reviewed-on: https://chromium-review.googlesource.com/c/1296673
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: default avatarDirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602388}
parent 2f074de1
......@@ -74,13 +74,14 @@ class LayoutTestRunner(object):
self._current_run_results = None
def run_tests(self, expectations, test_inputs, tests_to_skip, num_workers, retry_attempt):
batch_size = self._options.derived_batch_size
# If we're retrying a test, then it's because we think it might be flaky
# and rerunning it might provide a different result. We must restart
# content shell to get a valid result, as otherwise state can leak
# from previous tests. To do so, we set a batch size of 1, as that
# prevents content shell reuse.
batch_size = self._options.batch_size or 0
if retry_attempt >= 1:
if not self._options.must_use_derived_batch_size and retry_attempt >= 1:
batch_size = 1
self._expectations = expectations
......
......@@ -297,7 +297,7 @@ class Port(object):
# Relaunching the driver periodically helps keep it under control.
return 40
# The default is infinite batch size.
return None
return 0
def default_child_processes(self):
"""Returns the number of child processes to use for this port."""
......
......@@ -336,13 +336,24 @@ def parse_args(args):
action='store',
help='Output per-test profile information, using the specified profiler.'),
optparse.make_option(
'--reset-shell-between-tests',
'--restart-shell-between-tests',
action='store_true',
default=False,
help='Resetting the shell between tests causes the tests to '
help='Restarting the shell between tests causes the tests to '
'take twice as long to run on average, but provides more '
'consistent results. This is automatically enabled if '
'--repeat-each or --gtest_repeat is specified'),
'--repeat-each or --gtest_repeat is specified with '
'iterations > 1. This is equivalent to setting '
'--batch-size=1'),
optparse.make_option(
'--reuse-shell-between-tests',
action='store_true',
default=False,
help='Reusing the shell between tests causes tests to run more '
'quickly but has less consistent results. This is '
'primarily useful for debugging flakiness that only '
'occurs when content shell is reused. This is equivalent '
'to setting --batch-size=0.'),
optparse.make_option(
'--repeat-each',
type='int',
......@@ -513,8 +524,36 @@ def parse_args(args):
def _set_up_derived_options(port, options, args):
"""Sets the options values that depend on other options values."""
if options.batch_size is None:
options.batch_size = port.default_batch_size()
if options.restart_shell_between_tests:
# --restart-shell-between-tests is identical to setting --batch-size=1.
assert not options.reuse_shell_between_tests, (
'--restart-shell-between-tests is not compatible with '
'--reuse-shell-between-tests.')
assert options.batch_size is None, (
'--restart-shell-between-tests is not compatible with --batch-size')
options.derived_batch_size = 1
options.must_use_derived_batch_size = True
elif options.reuse_shell_between_tests:
# --reuse-shell-between-tests is identical to setting --batch-size=0
assert options.batch_size is None, (
'--reuse-shell-between-tests is not compatible with --batch-size')
options.derived_batch_size = 0
options.must_use_derived_batch_size = True
elif options.batch_size is not None:
options.derived_batch_size = options.batch_size
options.must_use_derived_batch_size = True
else:
# No flag has explicitly set the batch size.
# If 'repeat_each' or 'iterations' has been set, then implicitly set the
# batch size to 1. If we're already repeating the tests more than once,
# then we're not particularly concerned with speed. Restarting content
# shell provides more consistent results.
if options.repeat_each > 1 or options.iterations > 1:
options.derived_batch_size = 1
options.must_use_derived_batch_size = True
else:
options.derived_batch_size = port.default_batch_size()
options.must_use_derived_batch_size = False
if not options.child_processes:
options.child_processes = port.host.environ.get(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment