Skip to content

Commit d61ddee

Browse files
agrievecopybara-github
authored andcommitted
Revert "build: Speed up JUnit test listing"
This reverts commit 5a395fa64508945192896b2a10cf97c730e51ac6. Reason for revert: Speculative fix for flake endorser failures Original change's description: > build: Speed up JUnit test listing > > Listing JUnit tests can be very slow, 17+ seconds slow. This CL > optimizes the process by parallelizing test class discovery and > pre-filtering classes against the gtest filter(s) before passing the > filtered classes to JUnit. This avoids expensive reflection on classes > that would have been filtered out. Now it takes just 1 second. > > Since the underlying test listing is now significantly faster, the > Python-side test list cache in local_machine_junit_test_run.py is no > longer necessary and has been removed. This avoids staleness issues and > reduces the number of things to examine when a test fails (i.e. no > longer need to wonder whether the right tests ran due to the cache being > stale). > > [email protected] > > Bug: 40878339, 448894165 > Change-Id: I7a59372fa2e940e60fbab2d36989333b5b422173 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7173486 > Reviewed-by: Andrew Grieve <[email protected]> > Auto-Submit: Peter Wen <[email protected]> > Commit-Queue: Peter Wen <[email protected]> > Reviewed-by: Henrique Nakashima <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1547984} Bug: 40878339, 448894165, 462739429 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 40878339, 448894165 Change-Id: Id90abc870041d707c5cf154cecf9bc37b0ed4eba Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7187501 Bot-Commit: Rubber Stamper <[email protected]> Commit-Queue: Andrew Grieve <[email protected]> Cr-Commit-Position: refs/heads/main@{#1548572} NOKEYCHECK=True GitOrigin-RevId: 6847a7b2d60715df51f813afee0689f7ffcccdd3
1 parent 05da05d commit d61ddee

File tree

1 file changed

+73
-6
lines changed

1 file changed

+73
-6
lines changed

android/pylib/local/machine/local_machine_junit_test_run.py

Lines changed: 73 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import logging
88
import multiprocessing
99
import os
10+
import pathlib
1011
import queue
1112
import re
1213
import subprocess
@@ -16,6 +17,7 @@
1617
import time
1718
import zipfile
1819
from concurrent.futures import ThreadPoolExecutor
20+
import ast
1921

2022
from devil.utils import cmd_helper
2123
from py_utils import tempfile_ext
@@ -25,6 +27,9 @@
2527
from pylib.constants import host_paths
2628
from pylib.results import json_results
2729

30+
sys.path.append(str(pathlib.Path(__file__).parents[3] / 'gyp'))
31+
from util import md5_check
32+
2833
# Chosen after timing test runs of chrome_junit_tests with 7,16,32,
2934
# and 64 workers in threadpool and different classes_per_job.
3035
_MAX_TESTS_PER_JOB = 128
@@ -210,15 +215,77 @@ def _MakeJob(self, shard_id, temp_dir, test_group, properties_jar_path,
210215
json_config=job_json_config,
211216
json_results_path=json_results_path)
212217

213-
def _GetJsonConfig(self):
214-
with tempfile_ext.NamedTemporaryDirectory() as temp_dir:
215-
return self._QueryTestJsonConfig(temp_dir,
216-
allow_debugging=False,
217-
enable_shadow_allowlist=True)
218+
def _GetJsonConfig(self, list_only: bool = False):
219+
# The test list is expensive to generate, so cache it. The cache is
220+
# invalidated when the classpath or this file changes.
221+
cache_dir = pathlib.Path(
222+
constants.GetOutDirectory()) / 'junit_test_list_cache'
223+
os.makedirs(cache_dir, exist_ok=True)
224+
225+
# Cache --list-tests separately so that changing test filters does not
226+
# invalidate its cache.
227+
if list_only:
228+
record_path = (cache_dir / f'{self._test_instance.suite}_list.stamp')
229+
cached_test_list_path = (cache_dir /
230+
f'{self._test_instance.suite}_list.json')
231+
else:
232+
record_path = cache_dir / f'{self._test_instance.suite}.stamp'
233+
cached_test_list_path = cache_dir / f'{self._test_instance.suite}.json'
234+
235+
# Extract the classpath from the wrapper script.
236+
wrapper_script_source = ''
237+
print(f'Wrapper path: {self._wrapper_path}')
238+
with open(self._wrapper_path) as f:
239+
wrapper_script_source = f.read()
240+
classpath_re = re.compile(r'classpath = (\[[^\]]+\])', re.MULTILINE)
241+
matches = classpath_re.search(wrapper_script_source)
242+
classpath_str = matches.group(1)
243+
classpath_list = ast.literal_eval(classpath_str)
244+
# The paths in the wrapper script are relative to the script's location. We
245+
# need to make them relative to the current working directory.
246+
wrapper_dir = pathlib.Path(self._wrapper_path).parent
247+
classpath = [str(wrapper_dir / p) for p in classpath_list]
248+
249+
# Filters affect the test list so changes in filter should invalidate the
250+
# cache.
251+
input_strings = self._GetFilterArgs()
252+
253+
# We don't want to rebuild the cache if only method implementations have
254+
# been changed and no new tests have been added. This would be common when
255+
# iterating on private methods or tests method content. We should only
256+
# invalidate the cache if the public interface changes, which is likely
257+
# when a new test is added or tests are removed. But avoid this
258+
# optimization when listing tests as for parameterized tests, turbine jars
259+
# do not change when parameters are updated (this does not affect modifying
260+
# parameters and then running the test as robolectric loads the correct
261+
# values at runtime).
262+
if not list_only:
263+
for i, p in enumerate(classpath):
264+
if 'javac' in p:
265+
turbine_p = p.replace('javac', 'turbine')
266+
if os.path.exists(turbine_p):
267+
classpath[i] = turbine_p
268+
269+
def do_query_test_json_config():
270+
with tempfile_ext.NamedTemporaryDirectory() as temp_dir:
271+
result = self._QueryTestJsonConfig(temp_dir,
272+
allow_debugging=False,
273+
enable_shadow_allowlist=True)
274+
with open(cached_test_list_path, 'w') as f:
275+
json.dump(result, f)
276+
277+
md5_check.CallAndRecordIfStale(do_query_test_json_config,
278+
record_path=str(record_path),
279+
input_paths=classpath + [__file__],
280+
input_strings=input_strings,
281+
output_paths=[str(cached_test_list_path)])
282+
283+
with open(cached_test_list_path) as f:
284+
return json.load(f)
218285

219286
#override
220287
def GetTestsForListing(self):
221-
json_config = self._GetJsonConfig()
288+
json_config = self._GetJsonConfig(list_only=True)
222289
ret = []
223290
for config in json_config['configs'].values():
224291
for class_name, methods in config.items():

0 commit comments

Comments
 (0)