[Piglit] [PATCH v2 02/16] framework/test: Use an exception for is_skip() rather than return

Dylan Baker baker.dylan.c at gmail.com
Mon May 18 10:57:36 PDT 2015


This makes working with skips a little bit cleaner for a couple of
reasons. First, there is no need to return the super() call. Second, it
provides us a way to pass information up the call chain to Test.run(),
so that we can provide a reason for the skip.

This also adds some tests for glean, since it doesn't have any tests
currently for it's 'is_skip()' behavior.

Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
---
 framework/test/base.py               | 14 +++++++++++---
 framework/test/gleantest.py          |  9 ++++++---
 framework/test/piglit_test.py        | 14 ++++++++++----
 framework/tests/gleantest_tests.py   | 26 ++++++++++++++++++++++++++
 framework/tests/piglit_test_tests.py | 13 +++++++++----
 framework/tests/utils.py             | 16 ++++++++++++++++
 6 files changed, 78 insertions(+), 14 deletions(-)

diff --git a/framework/test/base.py b/framework/test/base.py
index 8a4bf99..9a3a149 100644
--- a/framework/test/base.py
+++ b/framework/test/base.py
@@ -36,6 +36,7 @@ import itertools
 import abc
 import copy
 
+from framework import exceptions
 from framework.core import Options
 from framework.results import TestResult
 
@@ -46,6 +47,11 @@ __all__ = [
 ]
 
 
+class TestIsSkip(exceptions.PiglitException):
+    """Exception raised in is_skip() if the test is a skip."""
+    pass
+
+
 class ProcessTimeout(threading.Thread):
     """ Timeout class for test processes
 
@@ -198,9 +204,11 @@ class Test(object):
             '{0}="{1}"'.format(k, v) for k, v in itertools.chain(
                 self.OPTS.env.iteritems(), self.env.iteritems()))
 
-        if self.is_skip():
+        try:
+            self.is_skip()
+        except TestIsSkip as e:
             self.result['result'] = 'skip'
-            self.result['out'] = "skipped by self.is_skip()"
+            self.result['out'] = e.message
             self.result['err'] = ""
             self.result['returncode'] = None
             return
@@ -241,7 +249,7 @@ class Test(object):
         skipped. The base version will always return False
 
         """
-        return False
+        pass
 
     def __set_process_group(self):
         if hasattr(os, 'setpgrp'):
diff --git a/framework/test/gleantest.py b/framework/test/gleantest.py
index de43857..539212f 100644
--- a/framework/test/gleantest.py
+++ b/framework/test/gleantest.py
@@ -24,7 +24,7 @@
 
 from __future__ import print_function, absolute_import
 import os
-from .base import Test
+from .base import Test, TestIsSkip
 from .piglit_test import TEST_BIN_DIR
 
 __all__ = [
@@ -61,5 +61,8 @@ class GleanTest(Test):
     def is_skip(self):
         # Glean tests require glx
         if self.OPTS.env['PIGLIT_PLATFORM'] not in ['glx', 'mixed_glx_egl']:
-            return True
-        return super(GleanTest, self).is_skip()
+            raise TestIsSkip(
+                'Glean tests require platform to support glx, '
+                'but the platform is "{}"'.format(
+                    self.OPTS.env['PIGLIT_PLATFORM']))
+        super(GleanTest, self).is_skip()
diff --git a/framework/test/piglit_test.py b/framework/test/piglit_test.py
index 5dd2de6..86843a2 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
+from .base import Test, WindowResizeMixin, TestIsSkip
 import framework.core as core
 
 
@@ -128,10 +128,16 @@ class PiglitGLTest(WindowResizeMixin, PiglitBaseTest):
         """
         platform = self.OPTS.env['PIGLIT_PLATFORM']
         if self.__require_platforms and platform not in self.__require_platforms:
-            return True
+            raise TestIsSkip(
+                'Test requires one of the following platforms "{}" '
+                'but the platform is "{}"'.format(
+                    self.__require_platforms, platform))
         elif self.__exclude_platforms and platform in self.__exclude_platforms:
-            return True
-        return super(PiglitGLTest, self).is_skip()
+            raise TestIsSkip(
+                'Test cannot be run on any of the following platforms "{}" '
+                'and the platform is "{}"'.format(
+                    self.__exclude_platforms, platform))
+        super(PiglitGLTest, self).is_skip()
 
     @PiglitBaseTest.command.getter
     def command(self):
diff --git a/framework/tests/gleantest_tests.py b/framework/tests/gleantest_tests.py
index 7c13ca3..5cbf427 100644
--- a/framework/tests/gleantest_tests.py
+++ b/framework/tests/gleantest_tests.py
@@ -26,6 +26,7 @@ import nose.tools as nt
 
 from framework.test import GleanTest
 from framework.tests import utils
+from framework.test.base import TestIsSkip
 
 
 @utils.no_error
@@ -67,3 +68,28 @@ def test_bad_returncode():
     test.interpret_result()
     nt.assert_equal(test.result['result'], 'fail',
                     msg="Result should have been fail")
+
+
+ at nt.raises(TestIsSkip)
+def test_is_skip_not_glx():
+    """test.gleantest.GleanTest.is_skip: Skips when platform isn't glx"""
+    GleanTest.OPTS.env['PIGLIT_PLATFORM'] = 'gbm'
+    test = GleanTest('foo')
+    test.is_skip()
+
+
+ at utils.not_raises(TestIsSkip)
+def test_is_skip_glx():
+    """test.gleantest.GleanTest.is_skip: Does not skip when platform is glx"""
+    GleanTest.OPTS.env['PIGLIT_PLATFORM'] = 'glx'
+    test = GleanTest('foo')
+    test.is_skip()
+
+
+ at utils.not_raises(TestIsSkip)
+def test_is_skip_glx_egl():
+    """test.gleantest.GleanTest.is_skip: Does not skip when platform is mixed_glx_egl
+    """
+    GleanTest.OPTS.env['PIGLIT_PLATFORM'] = 'mixed_glx_egl'
+    test = GleanTest('foo')
+    test.is_skip()
diff --git a/framework/tests/piglit_test_tests.py b/framework/tests/piglit_test_tests.py
index b65945a..c21f1fe 100644
--- a/framework/tests/piglit_test_tests.py
+++ b/framework/tests/piglit_test_tests.py
@@ -25,6 +25,7 @@ from __future__ import print_function, absolute_import
 import nose.tools as nt
 
 from framework.tests import utils
+from framework.test.base import TestIsSkip
 from framework.test.piglit_test import (PiglitBaseTest, PiglitGLTest,
                                         PiglitCLTest)
 
@@ -93,29 +94,33 @@ def test_PiglitGLTest_include_and_exclude():
                      exclude_platforms=['gbm'])
 
 
+ at utils.not_raises(TestIsSkip)
 def test_PiglitGLTest_platform_in_require():
     """test.piglit_test.PiglitGLTest.is_skip(): does not skip if platform is in require_platforms"""
     PiglitGLTest.OPTS.env['PIGLIT_PLATFORM'] = 'glx'
     test = PiglitGLTest(['foo'], require_platforms=['glx'])
-    nt.assert_false(test.is_skip())
+    test.is_skip()
 
 
+ at nt.raises(TestIsSkip)
 def test_PiglitGLTest_platform_not_in_require():
     """test.piglit_test.PiglitGLTest.is_skip(): skips if platform is not in require_platforms"""
     PiglitGLTest.OPTS.env['PIGLIT_PLATFORM'] = 'gbm'
     test = PiglitGLTest(['foo'], require_platforms=['glx'])
-    nt.assert_true(test.is_skip())
+    test.is_skip()
 
 
+ at nt.raises(TestIsSkip)
 def test_PiglitGLTest_platform_in_exclude():
     """test.piglit_test.PiglitGLTest.is_skip(): skips if platform is in exclude_platforms"""
     PiglitGLTest.OPTS.env['PIGLIT_PLATFORM'] = 'glx'
     test = PiglitGLTest(['foo'], exclude_platforms=['glx'])
-    nt.assert_true(test.is_skip())
+    test.is_skip()
 
 
+ at utils.not_raises(TestIsSkip)
 def test_PiglitGLTest_platform_not_in_exclude():
     """test.piglit_test.PiglitGLTest.is_skip(): does not skip if platform is in exclude_platforms"""
     PiglitGLTest.OPTS.env['PIGLIT_PLATFORM'] = 'gbm'
     test = PiglitGLTest(['foo'], exclude_platforms=['glx'])
-    nt.assert_false(test.is_skip())
+    test.is_skip()
diff --git a/framework/tests/utils.py b/framework/tests/utils.py
index abc8ef5..9fd1cc8 100644
--- a/framework/tests/utils.py
+++ b/framework/tests/utils.py
@@ -360,3 +360,19 @@ def capture_stderr(func):
             sys.stderr = restore
 
     return _inner
+
+
+def not_raises(exception):
+    """Decorator for tests that should not raise one of the follow exceptions.
+    """
+    def _decorator(function):
+        @functools.wraps(function)
+        def _inner(*args, **kwargs):
+            try:
+                function(*args, **kwargs)
+            except exception as e:
+                raise TestFailure(e)
+
+        return _inner
+
+    return _decorator
-- 
2.4.0



More information about the Piglit mailing list