Skip to content

Commit 0b091f6

Browse files
authored
Run @is_slow_test test first in parallel_testsuite.py (#25263)
Previously we were relying special naming of tests (e.g. `test_xxx_foo`) and using reverse alphabetic sort to run them first, but we dropped the `test_xxx` convention in favor of the `@is_slow_test` decorator. Ultimately it would be nice to merge this default sort with the `--failing-fast-and-slow` flag but for now they are still separate.
1 parent e46579e commit 0b091f6

File tree

2 files changed

+16
-13
lines changed

2 files changed

+16
-13
lines changed

test/common.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ def decorated(self, *args, **kwargs):
228228
return self.skipTest('skipping slow tests')
229229
return func(self, *args, **kwargs)
230230

231+
decorated.is_slow = True
231232
return decorated
232233

233234

test/parallel_testsuite.py

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,7 @@ def run(self, result):
114114
# Remove any old stale list of flaky tests before starting the run
115115
utils.delete_file(common.flaky_tests_log_filename)
116116

117-
# If we are running with --failing-and-slow-first, then the test list has been
118-
# pre-sorted based on previous test run results. Otherwise run the tests in
119-
# reverse alphabetical order.
120-
tests = list(self if self.failing_and_slow_first else self.reversed_tests())
117+
tests = self.get_sorted_tests()
121118
contains_browser_test = any(test.is_browser_test() for test in tests)
122119
use_cores = cap_max_workers_in_pool(min(self.max_cores, len(tests), num_cores()), contains_browser_test)
123120
errlog(f'Using {use_cores} parallel test processes')
@@ -179,17 +176,22 @@ def update_test_results_to(test_name):
179176

180177
return self.combine_results(result, results)
181178

182-
def reversed_tests(self):
183-
"""A list of this suite's tests, sorted reverse alphabetical order.
179+
def get_sorted_tests(self):
180+
"""A list of this suite's tests, sorted with the @is_slow_test tests first.
184181
185-
Many of the tests in test_core are intentionally named so that long tests
186-
fall toward the end of the alphabet (e.g. test_the_bullet). Tests are
187-
loaded in alphabetical order, so here we reverse that in order to start
188-
running longer tasks earlier, which should lead to better core utilization.
189-
190-
Future work: measure slowness of tests and sort accordingly.
182+
Future work: measure and store the speed of tests each test sort more accurately.
191183
"""
192-
return sorted(self, key=str, reverse=True)
184+
if self.failing_and_slow_first:
185+
# If we are running with --failing-and-slow-first, then the test list has been
186+
# pre-sorted based on previous test run results (see `runner.py`)
187+
return list(self)
188+
189+
def test_key(test):
190+
testMethod = getattr(test, test._testMethodName)
191+
is_slow = getattr(testMethod, 'is_slow', False)
192+
return (is_slow, str(test))
193+
194+
return sorted(self, key=test_key, reverse=True)
193195

194196
def combine_results(self, result, buffered_results):
195197
errlog('')

0 commit comments

Comments
 (0)