From 92577ea5d84973c2b479f9966b9653de29269d85 Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Thu, 23 Jun 2016 17:48:47 +0200 Subject: [PATCH] Bug 1275269 - Refactor & test BaseMarionetteTestRunner.run_tests; r=maja_zf Refactor BaseMarionetteTestRunner.run_tests into smaller sub-methods. Refactor _print_summary, moving functionality not related to summary-printing into run_tests. In test_marionette_runner.py, test run_tests and submethod: - Add mock_runner fixture to create a runner instance with some mocked properties - Test reset_test_stats - Test _initialize_test_run MozReview-Commit-ID: 7k1GJ0dyLCe --HG-- extra : rebase_source : fa347627c8dd049055f564f3cfb0ef0413deef2f --- .../harness/marionette/runner/base.py | 83 ++++++++++--------- .../harness_unit/test_marionette_runner.py | 48 ++++++++++- 2 files changed, 89 insertions(+), 42 deletions(-) diff --git a/testing/marionette/harness/marionette/runner/base.py b/testing/marionette/harness/marionette/runner/base.py index 0d3d08b2249e..3b295cfc52c2 100644 --- a/testing/marionette/harness/marionette/runner/base.py +++ b/testing/marionette/harness/marionette/runner/base.py @@ -696,9 +696,6 @@ class BaseMarionetteTestRunner(object): kwargs['workspace'] = self.workspace_path return kwargs - def start_marionette(self): - self.marionette = self.driverclass(**self._build_kwargs()) - def launch_test_container(self): if self.marionette.session is None: self.marionette.start_session() @@ -755,22 +752,24 @@ setReq.onerror = function() { traceback.print_exc() return crash - def run_tests(self, tests): + def _initialize_test_run(self, tests): assert len(tests) > 0 assert len(self.test_handlers) > 0 self.reset_test_stats() - self.start_time = time.time() + def _start_marionette(self): need_external_ip = True if not self.marionette: - self.start_marionette() + self.marionette = self.driverclass(**self._build_kwargs()) # if we're working against a desktop version, we usually don't need # an external ip if self.capabilities['device'] == "desktop": need_external_ip = False self.logger.info('Initial Profile Destination is ' '"{}"'.format(self.marionette.profile_path)) + return need_external_ip + def _set_baseurl(self, need_external_ip): # Gaia sets server_root and that means we shouldn't spin up our own httpd if not self.httpd: if self.server_root is None or os.path.isdir(self.server_root): @@ -782,29 +781,20 @@ setReq.onerror = function() { self.marionette.baseurl = self.server_root self.logger.info("using remote content from %s" % self.marionette.baseurl) - device_info = None + def _add_tests(self, tests): for test in tests: self.add_test(test) - # ensure we have only tests files with names starting with 'test_' invalid_tests = \ [t['filepath'] for t in self.tests if not os.path.basename(t['filepath']).startswith('test_')] if invalid_tests: - raise Exception("Tests file names must starts with 'test_'." + raise Exception("Tests file names must start with 'test_'." " Invalid test names:\n %s" % '\n '.join(invalid_tests)) - self.logger.info("running with e10s: {}".format(self.e10s)) - version_info = mozversion.get_version(binary=self.bin, - sources=self.sources, - dm_type=os.environ.get('DM_TRANS', 'adb') ) - - self.logger.suite_start(self.tests, - version_info=version_info, - device_info=device_info) - + def _log_skipped_tests(self): for test in self.manifest_skipped_tests: name = os.path.basename(test['path']) self.logger.test_start(name) @@ -813,13 +803,31 @@ setReq.onerror = function() { message=test['disabled']) self.todo += 1 + def run_tests(self, tests): + start_time = time.time() + self._initialize_test_run(tests) + + need_external_ip = self._start_marionette() + self._set_baseurl(need_external_ip) + + self._add_tests(tests) + + self.logger.info("running with e10s: {}".format(self.e10s)) + version_info = mozversion.get_version(binary=self.bin, + sources=self.sources, + dm_type=os.environ.get('DM_TRANS', 'adb') ) + + self.logger.suite_start(self.tests, version_info=version_info) + + self._log_skipped_tests() + interrupted = None try: counter = self.repeat while counter >=0: - round = self.repeat - counter - if round > 0: - self.logger.info('\nREPEAT %d\n-------' % round) + round_num = self.repeat - counter + if round_num > 0: + self.logger.info('\nREPEAT %d\n-------' % round_num) self.run_test_sets() counter -= 1 except KeyboardInterrupt: @@ -829,6 +837,21 @@ setReq.onerror = function() { interrupted = sys.exc_info() try: self._print_summary(tests) + self.record_crash() + self.elapsedtime = time.time() - start_time + + if self.marionette.instance: + self.marionette.instance.close() + self.marionette.instance = None + self.marionette.cleanup() + + for run_tests in self.mixin_run_tests: + run_tests(tests) + if self.shuffle: + self.logger.info("Using seed where seed is:%d" % self.shuffle_seed) + + self.logger.info('mode: {}'.format('e10s' if self.e10s else 'non-e10s')) + self.logger.suite_end() except: # raise only the exception if we were not interrupted if not interrupted: @@ -855,24 +878,6 @@ setReq.onerror = function() { for failed_test in self.failures: self.logger.info('%s' % failed_test[0]) - self.record_crash() - self.end_time = time.time() - self.elapsedtime = self.end_time - self.start_time - - if self.marionette.instance: - self.marionette.instance.close() - self.marionette.instance = None - - self.marionette.cleanup() - - for run_tests in self.mixin_run_tests: - run_tests(tests) - if self.shuffle: - self.logger.info("Using seed where seed is:%d" % self.shuffle_seed) - - self.logger.info('mode: {}'.format('e10s' if self.e10s else 'non-e10s')) - self.logger.suite_end() - def start_httpd(self, need_external_ip): warnings.warn("start_httpd has been deprecated in favour of create_httpd", DeprecationWarning) diff --git a/testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py b/testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py index c4c26bc79cbb..40fcc6ae04ba 100644 --- a/testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py +++ b/testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py @@ -2,7 +2,7 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. import pytest -from mock import patch, Mock, DEFAULT, mock_open +from mock import patch, Mock, DEFAULT, mock_open, MagicMock from marionette.runtests import ( MarionetteTestRunner, @@ -10,6 +10,7 @@ from marionette.runtests import ( cli ) from marionette.runner import MarionetteTestResult +from marionette_driver.marionette import Marionette # avoid importing MarionetteJSTestCase to prevent pytest from # collecting and running it as part of this test suite @@ -27,8 +28,7 @@ def _check_crash_counts(has_crashed, runner, mock_marionette): @pytest.fixture() def mock_marionette(request): """ Mock marionette instance """ - import marionette_driver - marionette = Mock(spec=marionette_driver.marionette.Marionette) + marionette = MagicMock(spec=Marionette) if 'has_crashed' in request.funcargnames: marionette.check_for_crash.return_value = request.getfuncargvalue( 'has_crashed' @@ -129,6 +129,18 @@ def runner(mach_parsed_kwargs): return MarionetteTestRunner(**mach_parsed_kwargs) +@pytest.fixture() +def mock_runner(runner, mock_marionette): + """ + MarionetteTestRunner instance with mocked-out + self.marionette and other properties. + """ + runner.driverclass = mock_marionette + runner._set_baseurl = Mock() + runner.run_test_set = Mock() + return runner + + @pytest.fixture def harness_class(request): """ @@ -358,6 +370,36 @@ def test_add_test_manifest(runner): assert test['expected'] == 'pass' +def test_reset_test_stats(runner): + def reset_successful(runner): + stats = ['passed', 'failed', 'unexpected_successes', 'todo', 'skipped', 'failures'] + return all([((s in vars(runner)) and (not vars(runner)[s])) for s in stats]) + assert reset_successful(runner) + runner.passed = 1 + runner.failed = 1 + runner.failures.append(['TEST-UNEXPECTED-FAIL']) + assert not reset_successful(runner) + with pytest.raises(Exception): + runner.run_tests([u'test_fake_thing.py']) + assert reset_successful(runner) + + +def test_initialize_test_run(mock_runner): + tests = [u'test_fake_thing.py'] + mock_runner.reset_test_stats = Mock() + with patch('marionette.runner.base.mozversion.get_version'): + mock_runner.run_tests(tests) + assert mock_runner.reset_test_stats.called + with pytest.raises(AssertionError) as test_exc: + mock_runner.run_tests([]) + assert "len(tests)" in str(test_exc.traceback[-1].statement) + with pytest.raises(AssertionError) as hndl_exc: + mock_runner.test_handlers = [] + mock_runner.run_tests(tests) + assert "test_handlers" in str(hndl_exc.traceback[-1].statement) + assert mock_runner.reset_test_stats.call_count == 1 + + if __name__ == '__main__': import sys sys.exit(pytest.main(['--verbose', __file__]))