 Chromium Code Reviews
 Chromium Code Reviews Issue 1415533007:
  [Android] Add sharding for AMP instrumentation tests.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master
    
  
    Issue 1415533007:
  [Android] Add sharding for AMP instrumentation tests.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: build/android/pylib/remote/device/remote_device_instrumentation_test_run.py | 
| diff --git a/build/android/pylib/remote/device/remote_device_instrumentation_test_run.py b/build/android/pylib/remote/device/remote_device_instrumentation_test_run.py | 
| index 8858b069ce6fdd4bed80cdbef419f7a7e3f32e06..f62e8565043a241dbbbfbbe03a18218bd4d2c152 100644 | 
| --- a/build/android/pylib/remote/device/remote_device_instrumentation_test_run.py | 
| +++ b/build/android/pylib/remote/device/remote_device_instrumentation_test_run.py | 
| @@ -8,6 +8,8 @@ import logging | 
| import os | 
| import tempfile | 
| +from devil.utils import reraiser_thread | 
| +from devil.utils import tempdir | 
| from pylib.base import base_test_result | 
| from pylib.remote.device import remote_device_test_run | 
| @@ -21,55 +23,90 @@ class RemoteDeviceInstrumentationTestRun( | 
| return self._test_instance.test_package | 
| #override | 
| - def _TriggerSetUp(self): | 
| - """Set up the triggering of a test run.""" | 
| - logging.info('Triggering test run.') | 
| - | 
| - # pylint: disable=protected-access | 
| - with tempfile.NamedTemporaryFile(suffix='.txt') as test_list_file: | 
| - tests = self._test_instance.GetTests() | 
| - logging.debug('preparing to run %d instrumentation tests remotely:', | 
| - len(tests)) | 
| - for t in tests: | 
| - test_name = '%s#%s' % (t['class'], t['method']) | 
| - logging.debug(' %s', test_name) | 
| - test_list_file.write('%s\n' % test_name) | 
| - test_list_file.flush() | 
| - self._test_instance._data_deps.append( | 
| - (os.path.abspath(test_list_file.name), None)) | 
| - | 
| - env_vars = self._test_instance.GetDriverEnvironmentVars( | 
| - test_list_file_path=test_list_file.name) | 
| - env_vars.update(self._test_instance.GetHttpServerEnvironmentVars()) | 
| - | 
| - logging.debug('extras:') | 
| - for k, v in env_vars.iteritems(): | 
| - logging.debug(' %s: %s', k, v) | 
| - | 
| - self._AmInstrumentTestSetup( | 
| - self._test_instance.apk_under_test, | 
| - self._test_instance.driver_apk, | 
| - self._test_instance.driver_name, | 
| - environment_variables=env_vars, | 
| - extra_apks=([self._test_instance.test_apk] + | 
| - self._test_instance.additional_apks)) | 
| + def _GetAppPath(self): | 
| + return self._test_instance.apk_under_test | 
| #override | 
| - def _ParseTestResults(self): | 
| + def _GetTestFramework(self): | 
| + if self._env.test_framework: | 
| + logging.warning('Ignoring configured test_framework "%s"', | 
| + self._env.test_framework) | 
| + return 'robotium' | 
| + | 
| + #override | 
| + def _ParseTestResults(self, test_output, results_zip): | 
| logging.info('Parsing results from stdout.') | 
| - r = base_test_result.TestRunResults() | 
| + results = base_test_result.TestRunResults() | 
| result_code, result_bundle, statuses = ( | 
| self._test_instance.ParseAmInstrumentRawOutput( | 
| - self._results['results']['output'].splitlines())) | 
| + test_output['results']['output'].splitlines())) | 
| result = self._test_instance.GenerateTestResults( | 
| result_code, result_bundle, statuses, 0, 0) | 
| if isinstance(result, base_test_result.BaseTestResult): | 
| - r.AddResult(result) | 
| + results.AddResult(result) | 
| elif isinstance(result, list): | 
| - r.AddResults(result) | 
| + results.AddResults(result) | 
| else: | 
| raise Exception('Unexpected result type: %s' % type(result).__name__) | 
| - self._DetectPlatformErrors(r) | 
| - return r | 
| + remote_device_test_run.DetectPlatformErrors( | 
| + results, test_output, results_zip) | 
| + return results | 
| + | 
| + #override | 
| + def _ShouldShard(self): | 
| + return True | 
| + | 
| + #override | 
| + def _SetupTestShards(self, num_shards): | 
| 
jbudorick
2015/12/01 04:07:13
This function is also doing too much. It shouldn't
 | 
| + test_ids = [] | 
| + | 
| + all_tests = self._test_instance.GetTests() | 
| + test_shards = [all_tests[i::num_shards] for i in range(num_shards)] | 
| + | 
| + test_list_files = [] | 
| + with tempdir.TempDir() as test_list_dir: | 
| + for i, tests in enumerate(test_shards): | 
| + test_list_file = tempfile.NamedTemporaryFile( | 
| + suffix='.txt', dir=test_list_dir, delete=False) | 
| + test_list_files.append(test_list_file) | 
| + for t in tests: | 
| + test_name = '%s#%s' % (t['class'], t['method']) | 
| + logging.debug('shard %s: %s', i, test_name) | 
| + test_list_file.write('%s\n' % test_name) | 
| + test_list_file.flush() | 
| + | 
| + with tempfile.NamedTemporaryFile(suffix='.zip') as test_package: | 
| + extra_apks = ([self._test_instance.test_apk] + | 
| + self._test_instance.additional_apks) | 
| + data_deps = (self._test_instance.GetDataDependencies() + | 
| + [(os.path.abspath(f.name), None) for f in test_list_files]) | 
| + package_configs = self._PackageTest( | 
| + test_package.name, self._test_instance.driver_apk, data_deps, | 
| + extra_apks=extra_apks) | 
| + | 
| + framework_configs = { | 
| + 'runner': self._test_instance.driver_name, | 
| + } | 
| + framework_configs.update(package_configs) | 
| + | 
| + def upload_test_shards(test_list_file): | 
| + shard_framework_config = dict(framework_configs) | 
| + env_vars = self._test_instance.GetDriverEnvironmentVars( | 
| + test_list_file_path=test_list_file.name) | 
| + env_vars.update(self._test_instance.GetHttpServerEnvironmentVars()) | 
| + shard_framework_config['environment_vars'] = ','.join( | 
| + '%s=%s' % (k, v) for k, v in env_vars.iteritems()) | 
| + | 
| + test_id = self._UploadTestToDevice(test_package.name) | 
| + self._UploadTestConfigToDevice( | 
| + test_id, shard_framework_config, self._appurify_configs) | 
| + test_ids.append(test_id) | 
| + | 
| + reraiser_thread.RunAsync( | 
| + funcs=[upload_test_shards]*num_shards, | 
| 
jbudorick
2015/12/01 04:07:13
I'd prefer you not add args, kwargs, or names to R
 | 
| + args=[[test_list_file] for test_list_file in test_list_files], | 
| + names=['upload test shard %d' % i for i in range(num_shards)]) | 
| + | 
| + return test_ids |