[2/2] framework/test: split Valgrind code into a Mixin

Submitted by Dylan Baker on Aug. 3, 2015, 10:06 p.m.

Details

Message ID 1438639581-30521-2-git-send-email-baker.dylan.c@gmail.com
State New, archived
Headers show

Not browsing as part of any series.

Commit Message

Dylan Baker Aug. 3, 2015, 10:06 p.m.
This code is pretty self contained and specific, by making a mixin we
allow it to be added to additional Test classes, and provides isolation
of the logic.

Signed-off-by: Dylan Baker <dylanx.c.baker@intel.com>
---
 framework/test/base.py        | 64 +++++++++++++++++++++++---------
 framework/test/piglit_test.py | 12 +-----
 framework/tests/base_tests.py | 85 ++++++++++++++++++++++++++++++++++++++++++-
 framework/tests/utils.py      |  2 +
 4 files changed, 135 insertions(+), 28 deletions(-)

Patch hide | download patch | download mbox

diff --git a/framework/test/base.py b/framework/test/base.py
index 4030a82..d3a4e2e 100644
--- a/framework/test/base.py
+++ b/framework/test/base.py
@@ -43,6 +43,9 @@  from framework.results import TestResult
 
 __all__ = [
     'Test',
+    'TestIsSkip',
+    'TestRunError',
+    'ValgrindMixin',
     'WindowResizeMixin',
 ]
 
@@ -250,18 +253,6 @@  class Test(object):
 
         self.interpret_result()
 
-        if self.OPTS.valgrind:
-            # If the underlying test failed, simply report
-            # 'skip' for this valgrind test.
-            if self.result['result'] != 'pass':
-                self.result['result'] = 'skip'
-            elif self.result['returncode'] == 0:
-                # Test passes and is valgrind clean.
-                self.result['result'] = 'pass'
-            else:
-                # Test passed but has valgrind errors.
-                self.result['result'] = 'fail'
-
     def is_skip(self):
         """ Application specific check for skip
 
@@ -326,11 +317,9 @@  class Test(object):
             out, err = proc.communicate()
             returncode = proc.returncode
         except OSError as e:
-            # Different sets of tests get built under
-            # different build configurations.  If
-            # a developer chooses to not build a test,
-            # Piglit should not report that test as having
-            # failed.
+            # Different sets of tests get built under different build
+            # configurations.  If a developer chooses to not build a test,
+            # Piglit should not report that test as having failed.
             if e.errno == errno.ENOENT:
                 raise TestRunError("Test executable not found.\n", 'skip')
             else:
@@ -389,3 +378,44 @@  class WindowResizeMixin(object):
         # If we reach this point then there has been no error, but spurious
         # resize was detected more than 5 times. Set the result to fail
         raise TestRunError('Got spurious resize more than 5 times', 'fail')
+
+
+class ValgrindMixin(object):
+    """Mixin class that adds support for running tests through valgrind.
+
+    This mixin allows a class to run with the --valgrind option.
+
+    This mixin doesn't do the obvious thing and override the interpret_result()
+    value. This is on purpose. the result interpretation that the ValgrindMixin
+    does *must* be done last, since it matters to valgrind whether the test
+    passed or not (valgrind will only apply it's statuses to tests that pass).
+    To ensure that this happens run() is overwritten, and the valgrind status
+    is set after that (which is after interpret_result() is called in run()
+
+    """
+    @Test.command.getter
+    def command(self):
+        command = super(ValgrindMixin, self).command
+        if self.OPTS.valgrind:
+            return ['valgrind', '--quiet', '--error-exitcode=1',
+                    '--tool=memcheck'] + command
+        else:
+            return command
+
+    def run(self):
+        super(ValgrindMixin, self).run()
+        self.__valgrind_result()
+
+    def __valgrind_result(self):
+        """Set valgrind status."""
+        if self.OPTS.valgrind:
+            # If the underlying test failed, simply report
+            # 'skip' for this valgrind test.
+            if self.result['result'] != 'pass':
+                self.result['result'] = 'skip'
+            elif self.result['returncode'] == 0:
+                # Test passes and is valgrind clean.
+                self.result['result'] = 'pass'
+            else:
+                # Test passed but has valgrind errors.
+                self.result['result'] = 'fail'
diff --git a/framework/test/piglit_test.py b/framework/test/piglit_test.py
index cf74f30..fcd613c 100644
--- a/framework/test/piglit_test.py
+++ b/framework/test/piglit_test.py
@@ -31,7 +31,7 @@  try:
 except ImportError:
     import json
 
-from .base import Test, WindowResizeMixin, TestIsSkip
+from .base import Test, WindowResizeMixin, ValgrindMixin, TestIsSkip
 import framework.core as core
 
 
@@ -52,7 +52,7 @@  CL_CONCURRENT = (not sys.platform.startswith('linux') or
                  glob.glob('/dev/dri/render*'))
 
 
-class PiglitBaseTest(Test):
+class PiglitBaseTest(ValgrindMixin, Test):
     """
     PiglitTest: Run a "native" piglit test executable
 
@@ -65,14 +65,6 @@  class PiglitBaseTest(Test):
         # Prepend TEST_BIN_DIR to the path.
         self._command[0] = os.path.join(TEST_BIN_DIR, self._command[0])
 
-    @Test.command.getter
-    def command(self):
-        command = super(PiglitBaseTest, self).command
-        if self.OPTS.valgrind:
-            return ['valgrind', '--quiet', '--error-exitcode=1',
-                    '--tool=memcheck'] + command
-        return command
-
     def interpret_result(self):
         outlines = self.result['out'].split('\n')
         outpiglit = (s[7:] for s in outlines if s.startswith('PIGLIT:'))
diff --git a/framework/tests/base_tests.py b/framework/tests/base_tests.py
index e5b9f29..a305658 100644
--- a/framework/tests/base_tests.py
+++ b/framework/tests/base_tests.py
@@ -25,8 +25,12 @@  from __future__ import print_function, absolute_import
 import nose.tools as nt
 
 import framework.tests.utils as utils
-from framework.test.base import Test, WindowResizeMixin, TestRunError
+from framework.test.base import (
+    Test, WindowResizeMixin, ValgrindMixin, TestRunError
+)
+from framework.tests.status_tests import PROBLEMS, STATUSES
 
+# pylint: disable=invalid-name
 
 # Helpers
 class TestTest(Test):
@@ -159,3 +163,82 @@  def test_mutation():
     _Test(args)
 
     nt.assert_list_equal(args, ['a', 'b'])
+
+
+def test_ValgrindMixin_command():
+    """test.base.ValgrindMixin.command: overrides self.command"""
+    class _Test(ValgrindMixin, utils.Test):
+        pass
+
+    test = _Test(['foo'])
+    test.OPTS.valgrind = True
+    nt.eq_(test.command, ['valgrind', '--quiet', '--error-exitcode=1',
+                          '--tool=memcheck', 'foo'])
+    test.OPTS.valgrind = False
+
+
+class TestValgrindMixinRun(object):
+    @classmethod
+    def setup_class(cls):
+        class _NoRunTest(utils.Test):
+            def run(self):
+                self.interpret_result()
+
+        class _Test(ValgrindMixin, _NoRunTest):
+            pass
+
+        cls.test = _Test
+
+    @utils.nose_generator
+    def test_bad_valgrind_true(self):
+        """Test non-pass status when OPTS.valgrind is True."""
+        def test(status, expected):
+            test = self.test(['foo'])
+            test.OPTS.valgrind = True
+            test.result['result'] = status
+            test.run()
+            nt.eq_(test.result['result'], expected)
+
+        desc = ('test.base.ValgrindMixin.run: '
+                'when status is "{}" it is changed to "{}"')
+
+        for status in PROBLEMS:
+            test.description = desc.format(status, 'skip')
+            yield test, status, 'skip'
+
+    @utils.nose_generator
+    def test_valgrind_false(self):
+        """Test non-pass status when OPTS.valgrind is False."""
+        def test(status):
+            test = self.test(['foo'])
+            test.OPTS.valgrind = False
+            test.result['result'] = status
+            test.run()
+            nt.eq_(test.result['result'], status)
+
+        desc = ('test.base.ValgrindMixin.run: when status is "{}" '
+                'it is not changed when not running valgrind.')
+
+        for status in STATUSES:
+            test.description = desc.format(status)
+            yield test, status
+
+    def test_pass(self):
+        """test.base.ValgrindMixin.run: when test is 'pass' and returncode is '0' result is pass
+        """
+        test = self.test(['foo'])
+        test.OPTS.valgrind = True
+        test.result['result'] = 'pass'
+        test.result['returncode'] = 0
+        test.run()
+        nt.eq_(test.result['result'], 'pass')
+
+    def test_fallthrough(self):
+        """test.base.ValgrindMixin.run: when a test is 'pass' but returncode is not 0 it's 'fail'
+        """
+        test = self.test(['foo'])
+        test.OPTS.valgrind = True
+        test.result['result'] = 'pass'
+        test.result['returncode'] = 1
+        test.run()
+        nt.eq_(test.result['result'], 'fail')
diff --git a/framework/tests/utils.py b/framework/tests/utils.py
index 55d2b6e..8214331 100644
--- a/framework/tests/utils.py
+++ b/framework/tests/utils.py
@@ -255,6 +255,8 @@  def nose_generator(func):
     GeneratedTestWrapper reverse-proxy object
 
     """
+
+    @functools.wraps(func)
     def test_wrapper(*args, **kwargs):
         for x in func(*args, **kwargs):
             x = list(x)