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: NCleber Rosa <crosa@redhat.com>

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
 

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)
 

avocado/plugins/xunit.py

@@ -83,68 +83,72 @@ class XmlResult(object):
             self.xml.append(tc)
         self.xml.append('</testsuite>')
 
-    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<testcase classname={class} name={name} time="{time}"/>'
-        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<testcase classname={class} name={name} time="{time}">
 \t\t<skipped />
 \t</testcase>'''
-        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<testcase classname={class} name={name} time="{time}">
 \t\t<failure type={type} message={reason}><![CDATA[{traceback}]]></failure>
 \t\t<system-out><![CDATA[{systemout}]]></system-out>
 \t</testcase>'''
-        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<testcase classname={class} name={name} time="{time}">
 \t\t<error type={type} message={reason}><![CDATA[{traceback}]]></error>
 \t\t<system-out><![CDATA[{systemout}]]></system-out>
 \t</testcase>'''
-        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):
         """

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'])

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):