[Piglit] [PATCH 2/2] framework/test: split Valgrind code into a Mixin
Dylan Baker
baker.dylan.c at gmail.com
Mon Aug 3 15:06:21 PDT 2015
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 at 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(-)
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)
--
2.5.0
More information about the Piglit
mailing list