From 7a56123e6feab0b88bc4989438bffad9ec783d71 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Tue, 29 Jul 2014 15:24:45 -0300 Subject: [PATCH] Separate test loading and introduce test state This commit implements two very different things, but still very much connected: 1) Separate test loading to the process that will be actually running the test. This leads to a better separation of duties of the process interacting with the user and the process that is actually doing the (usually) heavy work. 2) Remove the pickling of the test instance and sending it back and forth. As an added bonus, it was noted during development, that both processes load all necessary Python modules (including the test module) when unmarshalling the pickled instance, resulting in unecessary work. Actual data on performance improvements were not collected though. It has one change in behaviour (regression) though. Test timeout now comes only from the test parameters. The reasoning is that since there's now a better separation of roles and the fact that the first process is not loading the test instance, it can not look at the timeout defined there. IMHO this is a sign of good design, since other types of test (think of shell scripts) can not communicate their intended timeout. Maybe a 'test.cfg' or something like that should be the solution to that. Signed-off-by: Cleber Rosa --- avocado/job.py | 30 ++++--- avocado/plugins/jsonresult.py | 17 ++-- avocado/plugins/xunit.py | 85 ++++++++++---------- avocado/result.py | 143 ++++++++++++++++++---------------- avocado/test.py | 9 ++- 5 files changed, 152 insertions(+), 132 deletions(-) diff --git a/avocado/job.py b/avocado/job.py index d4d82f74..d93ff0d2 100644 --- a/avocado/job.py +++ b/avocado/job.py @@ -65,6 +65,8 @@ class TestRunner(object): """ Resolve and load the test url from the the test shortname. + This method should now be called by the test runner process. + :param params: Dictionary with test params. :type params: dict :return: an instance of :class:`avocado.test.Test`. @@ -109,7 +111,7 @@ class TestRunner(object): return test_instance - def run_test(self, instance, queue): + def run_test(self, params, queue): """ Run a test instance in a subprocess. @@ -123,10 +125,13 @@ class TestRunner(object): raise exceptions.TestTimeoutError(e_msg) signal.signal(signal.SIGUSR1, timeout_handler) + + instance = self.load_test(params) + self.result.start_test(instance.get_state()) try: instance.run_avocado() finally: - queue.put(instance) + queue.put(instance.get_state()) def run(self, params_list): """ @@ -145,32 +150,25 @@ class TestRunner(object): self.result.start_tests() q = multiprocessing.Queue() for params in params_list: - test_instance = self.load_test(params) - self.result.start_test(test_instance) p = multiprocessing.Process(target=self.run_test, - args=(test_instance, q,)) + args=(params, q,)) p.start() - # The test timeout can come from: - # 1) Test params dict (params) - # 2) Test default params dict (test_instance.params.timeout) + # Change in behaviour: timeout now comes *only* from test params timeout = params.get('timeout') - if timeout is None: - if hasattr(test_instance.params, 'timeout'): - timeout = test_instance.params.timeout if timeout is not None: timeout = float(timeout) # Wait for the test to end for [timeout] s try: - test_instance = q.get(timeout=timeout) + test_state = q.get(timeout=timeout) except Exception: # If there's nothing inside the queue after timeout, the process # must be terminated. send_signal(p, signal.SIGUSR1) - test_instance = q.get() + test_state = q.get() - self.result.check_test(test_instance) - if not status.mapping[test_instance.status]: - failures.append(test_instance.name) + self.result.check_test(test_state) + if not status.mapping[test_state['status']]: + failures.append(test_state['name']) self.result.end_tests() return failures diff --git a/avocado/plugins/jsonresult.py b/avocado/plugins/jsonresult.py index d0b1d8f0..a002cbc6 100644 --- a/avocado/plugins/jsonresult.py +++ b/avocado/plugins/jsonresult.py @@ -42,18 +42,19 @@ class JSONTestResult(TestResult): self.json = {'debuglog': self.stream.logfile, 'tests': []} - def end_test(self, test): + def end_test(self, state): """ Called when the given test has been run. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ - TestResult.end_test(self, test) - t = {'test': test.tagged_name, - 'url': test.name, - 'time': test.time_elapsed, - 'status': test.status, - 'whiteboard': test.whiteboard, + TestResult.end_test(self, state) + t = {'test': state['tagged_name'], + 'url': state['name'], + 'time': state['time_elapsed'], + 'status': state['status'], + 'whiteboard': state['whiteboard'], } self.json['tests'].append(t) diff --git a/avocado/plugins/xunit.py b/avocado/plugins/xunit.py index 34d20d4c..68ce45e8 100644 --- a/avocado/plugins/xunit.py +++ b/avocado/plugins/xunit.py @@ -83,68 +83,72 @@ class XmlResult(object): self.xml.append(tc) self.xml.append('') - def add_success(self, test): + def add_success(self, state): """ Add a testcase node of kind succeed. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ tc = '\t' - values = {'class': self._escape_attr(test.__class__.__name__), - 'name': self._escape_attr(test.tagged_name), - 'time': test.time_elapsed} + values = {'class': self._escape_attr(state['class_name']), + 'name': self._escape_attr(state['tagged_name']), + 'time': state['time_elapsed']} self.testcases.append(tc.format(**values)) - def add_skip(self, test): + def add_skip(self, state): """ Add a testcase node of kind skipped. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ tc = '''\t \t\t \t''' - values = {'class': self._escape_attr(test.__class__.__name__), - 'name': self._escape_attr(test.tagged_name), - 'time': test.time_elapsed} + values = {'class': self._escape_attr(state['class_name']), + 'name': self._escape_attr(state['tagged_name']), + 'time': state['time_elapsed']} self.testcases.append(tc.format(**values)) - def add_failure(self, test): + def add_failure(self, state): """ Add a testcase node of kind failed. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ tc = '''\t \t\t \t\t \t''' - values = {'class': self._escape_attr(test.__class__.__name__), - 'name': self._escape_attr(test.tagged_name), - 'time': test.time_elapsed, - 'type': self._escape_attr(test.fail_class), - 'traceback': self._escape_cdata(test.traceback), - 'systemout': self._escape_cdata(test.text_output), - 'reason': self._escape_attr(str(test.fail_reason))} + values = {'class': self._escape_attr(state['class_name']), + 'name': self._escape_attr(state['tagged_name']), + 'time': state['time_elapsed'], + 'type': self._escape_attr(state['fail_class']), + 'traceback': self._escape_cdata(state['traceback']), + 'systemout': self._escape_cdata(state['text_output']), + 'reason': self._escape_attr(str(state['fail_reason']))} self.testcases.append(tc.format(**values)) - def add_error(self, test): + def add_error(self, state): """ Add a testcase node of kind error. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ tc = '''\t \t\t \t\t \t''' - values = {'class': self._escape_attr(test.__class__.__name__), - 'name': self._escape_attr(test.tagged_name), - 'time': test.time_elapsed, - 'type': self._escape_attr(test.fail_class), - 'traceback': self._escape_cdata(test.traceback), - 'systemout': self._escape_cdata(test.text_output), - 'reason': self._escape_attr(str(test.fail_reason))} + values = {'class': self._escape_attr(state['class_name']), + 'name': self._escape_attr(state['tagged_name']), + 'time': state['time_elapsed'], + 'type': self._escape_attr(state['fail_class']), + 'traceback': self._escape_cdata(state['traceback']), + 'systemout': self._escape_cdata(state['text_output']), + 'reason': self._escape_attr(str(state['fail_reason']))} self.testcases.append(tc.format(**values)) @@ -183,19 +187,22 @@ class xUnitTestResult(TestResult): """ TestResult.start_test(self, test) - def end_test(self, test): + def end_test(self, state): """ Record an end test event, accord to the given test status. - """ - TestResult.end_test(self, test) - if test.status == 'PASS': - self.xml.add_success(test) - if test.status == 'TEST_NA': - self.xml.add_skip(test) - if test.status == 'FAIL': - self.xml.add_failure(test) - if test.status == 'ERROR': - self.xml.add_error(test) + + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict + """ + TestResult.end_test(self, state) + if state['status'] == 'PASS': + self.xml.add_success(state) + elif state['status'] == 'TEST_NA': + self.xml.add_skip(state) + elif state['status'] == 'FAIL': + self.xml.add_failure(state) + elif state['status'] == 'ERROR': + self.xml.add_error(state) def end_tests(self): """ diff --git a/avocado/result.py b/avocado/result.py index 4b2828ed..385d0ef3 100644 --- a/avocado/result.py +++ b/avocado/result.py @@ -52,37 +52,37 @@ class TestResultProxy(object): for output_plugin in self.output_plugins: output_plugin.end_tests() - def start_test(self, test): + def start_test(self, state): for output_plugin in self.output_plugins: - output_plugin.start_test(test) + output_plugin.start_test(state) - def end_test(self, test): + def end_test(self, state): for output_plugin in self.output_plugins: - output_plugin.end_test(test) + output_plugin.end_test(state) - def add_pass(self, test): + def add_pass(self, state): for output_plugin in self.output_plugins: - output_plugin.add_pass(test) + output_plugin.add_pass(state) - def add_error(self, test): + def add_error(self, state): for output_plugin in self.output_plugins: - output_plugin.add_error(test) + output_plugin.add_error(state) - def add_fail(self, test): + def add_fail(self, state): for output_plugin in self.output_plugins: - output_plugin.add_fail(test) + output_plugin.add_fail(state) - def add_skip(self, test): + def add_skip(self, state): for output_plugin in self.output_plugins: - output_plugin.add_skip(test) + output_plugin.add_skip(state) - def add_warn(self, test): + def add_warn(self, state): for output_plugin in self.output_plugins: - output_plugin.add_warn(test) + output_plugin.add_warn(state) - def check_test(self, test): + def check_test(self, state): for output_plugin in self.output_plugins: - output_plugin.check_test(test) + output_plugin.check_test(state) class TestResult(object): @@ -149,64 +149,70 @@ class TestResult(object): """ pass - def start_test(self, test): + def start_test(self, state): """ Called when the given test is about to run. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ pass - def end_test(self, test): + def end_test(self, state): """ Called when the given test has been run. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ self.tests_run += 1 - self.total_time += test.time_elapsed + self.total_time += state['time_elapsed'] - def add_pass(self, test): + def add_pass(self, state): """ Called when a test succeeded. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ - self.passed.append(test) + self.passed.append(state) - def add_error(self, test): + def add_error(self, state): """ Called when a test had a setup error. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ - self.errors.append(test) + self.errors.append(state) - def add_fail(self, test): + def add_fail(self, state): """ Called when a test fails. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ - self.failed.append(test) + self.failed.append(state) - def add_skip(self, test): + def add_skip(self, state): """ Called when a test is skipped. :param test: an instance of :class:`avocado.test.Test`. """ - self.skipped.append(test) + self.skipped.append(state) - def add_warn(self, test): + def add_warn(self, state): """ Called when a test had a warning. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ - self.warned.append(test) + self.warned.append(state) - def check_test(self, test): + def check_test(self, state): """ Called once for a test to check status and report. @@ -217,9 +223,9 @@ class TestResult(object): 'FAIL': self.add_fail, 'TEST_NA': self.add_skip, 'WARN': self.add_warn} - add = status_map[test.status] - add(test) - self.end_test(test) + add = status_map[state['status']] + add(state) + self.end_test(state) class HumanTestResult(TestResult): @@ -247,66 +253,73 @@ class HumanTestResult(TestResult): self.stream.log_header("TOTAL WARNED: %d" % len(self.warned)) self.stream.log_header("ELAPSED TIME: %.2f s" % self.total_time) - def start_test(self, test): + def start_test(self, state): """ Called when the given test is about to run. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ self.test_label = '(%s/%s) %s: ' % (self.tests_run, self.tests_total, - test.tagged_name) + state['tagged_name']) self.stream.info(msg=self.test_label, skip_newline=True) - def end_test(self, test): + def end_test(self, state): """ Called when the given test has been run. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ - TestResult.end_test(self, test) + TestResult.end_test(self, state) - def add_pass(self, test): + def add_pass(self, state): """ Called when a test succeeded. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ - TestResult.add_pass(self, test) - self.stream.log_pass(test.time_elapsed) + TestResult.add_pass(self, state) + self.stream.log_pass(state['time_elapsed']) - def add_error(self, test): + def add_error(self, state): """ Called when a test had a setup error. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ - TestResult.add_error(self, test) - self.stream.log_error(test.time_elapsed) + TestResult.add_error(self, state) + self.stream.log_error(state['time_elapsed']) - def add_fail(self, test): + def add_fail(self, state): """ Called when a test fails. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ - TestResult.add_fail(self, test) - self.stream.log_fail(test.time_elapsed) + TestResult.add_fail(self, state) + self.stream.log_fail(state['time_elapsed']) - def add_skip(self, test): + def add_skip(self, state): """ Called when a test is skipped. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ - TestResult.add_skip(self, test) - self.stream.log_skip(test.time_elapsed) + TestResult.add_skip(self, state) + self.stream.log_skip(state['time_elapsed']) - def add_warn(self, test): + def add_warn(self, state): """ Called when a test had a warning. - :param test: an instance of :class:`avocado.test.Test`. + :param state: result of :class:`avocado.test.Test.get_state`. + :type state: dict """ - TestResult.add_warn(self, test) - self.stream.log_warn(test.time_elapsed) + TestResult.add_warn(self, state) + self.stream.log_warn(state['time_elapsed']) diff --git a/avocado/test.py b/avocado/test.py index a3c797c6..14b728f2 100644 --- a/avocado/test.py +++ b/avocado/test.py @@ -187,12 +187,12 @@ class Test(unittest.TestCase): def __repr__(self): return "Test(%r)" % self.tagged_name - def __getstate__(self): + def get_state(self): """ - Pickle only selected attributes of the class for serialization. + Serialize selected attributes representing the test state - The fact we serialize the class means you'll have to modify this - class if you intend to make significant changes to its structure. + :returns: a dictionary containing relevant test state data + :rtype: dict """ orig = dict(self.__dict__) d = {} @@ -205,6 +205,7 @@ class Test(unittest.TestCase): if key in preserve_attr: d[key] = orig[key] d['params'] = orig['_raw_params'] + d['class_name'] = self.__class__.__name__ return d def _set_default(self, key, default): -- GitLab