[v2] framework: Fix FastSkipMixin version comparisons

Submitted by Dylan Baker on Sept. 15, 2016, 7:08 p.m.

Details

Message ID 20160915190804.26777-1-dylan@pnwbakers.com
State New
Headers show
Series "framework: Fix FastSkipMixin version comparisons" ( rev: 2 ) in Piglit

Not browsing as part of any series.

Commit Message

Dylan Baker Sept. 15, 2016, 7:08 p.m.
Currently the FastSkipMixin assumes that versions will always be a
minimum required version, and nothing else. This is mostly true of
piglit tests, but isn't always true. Sometimes tests specify that they
need a version greater-or-equal, and a few have less-than or
less-than-equal.

This patch fixes that, and also generalizes the fast-skip mechanism a
bit. It does this by allowing the caller to set an operation as well as
a value.

This also updates glslparsertest and shadertest to use the newly updated
interfaces.

Signed-off-by: Dylan Baker <dylanx.c.baker@intel.com>
---

v2: - fix >= and <=

 framework/test/glsl_parser_test.py                |  23 +--
 framework/test/opengl.py                          | 209 +++++++++++++++++-----
 framework/test/shader_test.py                     |  28 ++-
 unittests/framework/test/test_glsl_parser_test.py |  12 +-
 unittests/framework/test/test_opengl.py           | 195 +++++++-------------
 unittests/framework/test/test_shader_test.py      |  21 ++-
 6 files changed, 282 insertions(+), 206 deletions(-)

Patch hide | download patch | download mbox

diff --git a/framework/test/glsl_parser_test.py b/framework/test/glsl_parser_test.py
index b1acb18..9cfbc65 100644
--- a/framework/test/glsl_parser_test.py
+++ b/framework/test/glsl_parser_test.py
@@ -24,6 +24,7 @@ 
 from __future__ import (
     absolute_import, division, print_function, unicode_literals
 )
+import operator
 import os
 import re
 import io
@@ -31,7 +32,7 @@  import six
 
 from framework import exceptions
 from .base import TestIsSkip
-from .opengl import FastSkipMixin
+from .opengl import FastSkipMixin, VersionSkip, ExtensionsSkip
 from .piglit_test import PiglitBaseTest, TEST_BIN_DIR
 
 __all__ = [
@@ -115,29 +116,31 @@  class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
         glsl = config.get('glsl_version')
         if glsl:
             if _is_gles_version(glsl):
-                self.glsl_es_version = float(glsl[:3])
+                self.glsl_es_version = VersionSkip(
+                    operator.ge, float(glsl[:3]), '>=')
             else:
-                self.glsl_version = float(glsl)
+                self.glsl_version = VersionSkip(
+                    operator.ge, float(glsl), '>=')
 
         req = config.get('require_extensions')
         if req:
-            self.gl_required = set(
-                r for r in req.split() if not r.startswith('!'))
+            self.gl_required = ExtensionsSkip(set(
+                r for r in req.split() if not r.startswith('!')))
 
         # If GLES is requested, but piglit was not built with a gles version,
         # then ARB_ES3<ver>_compatibility is required. Add it to
         # self.gl_required
         if self.glsl_es_version and not _HAS_GLES_BIN:
-            if self.glsl_es_version == 1.0:
+            if self.glsl_es_version.version == 1.0:
                 ver = '2'
-            elif self.glsl_es_version == 3.0:
+            elif self.glsl_es_version.version == 3.0:
                 ver = '3'
-            elif self.glsl_es_version == 3.1:
+            elif self.glsl_es_version.version == 3.1:
                 ver = '3_1'
-            elif self.glsl_es_version == 3.2:
+            elif self.glsl_es_version.version == 3.2:
                 ver = '3_2'
             ext = 'ARB_ES{}_compatibility'.format(ver)
-            self.gl_required.add(ext)
+            self.gl_required.extensions.add(ext)
             self._command.append(ext)
 
     @staticmethod
diff --git a/framework/test/opengl.py b/framework/test/opengl.py
index 6fe8670..5146051 100644
--- a/framework/test/opengl.py
+++ b/framework/test/opengl.py
@@ -31,6 +31,7 @@  import warnings
 import six
 
 from framework import exceptions, core
+from framework import compat
 from framework.options import OPTIONS
 from .base import TestIsSkip
 
@@ -45,6 +46,21 @@  __all__ = [
 _DISABLED = bool(os.environ.get('PIGLIT_NO_FAST_SKIP', False))
 
 
+@compat.python_2_unicode_compatible
+class FastSkip(exceptions.PiglitInternalError):
+    """Exception class raised when a test should fast skip.
+
+    This is used internally by the FastSkipMixin and is reraised as a
+    TestIsSkip exception.
+    """
+    def __init__(self, msg=''):
+        super(FastSkip, self).__init__()
+        self.msg = msg
+
+    def __str__(self):
+        return self.msg
+
+
 class StopWflinfo(exceptions.PiglitException):
     """Exception called when wlfinfo getter should stop."""
     def __init__(self, reason):
@@ -287,6 +303,96 @@  class WflInfo(object):
         return ret
 
 
+class VersionSkip(object):
+    """Class that implements skip conditions based on versions.
+
+    This class adds support for checking one version against another.
+
+    >>> import operator
+    >>> test = VersionSkip(operator.lt, 3.0, rep='<')
+    >>> test.test(4.0)
+    Traceback (most recent call last):
+        ...
+    framework.test.opengl.FastSkip: ...
+
+    >>> import operator
+    >>> test = VersionSkip(operator.lt, 3.0, rep='<')
+    >>> test.test(1.0)
+
+    Arguments:
+    version -- A numeric type version that is to be compared to.
+    op      -- A callable with the signature that takes two numeric types and
+               returns a bool. Good choices are operator.{lt,gt,etc}
+
+    Keyword Arguments:
+    rep     -- A text representation of the operator. If this is not provided
+               the value of six.text_type will be used.
+    """
+
+    __slots__ = ['version', 'op', 'rep']
+
+    def __init__(self, op, version, rep=None):
+        self.version = version
+        self.op = op
+        self.rep = rep or six.text_type(op)
+
+    def test(self, version):
+        """Test whether the Test should skip.
+
+        Arguments
+        version    -- The maximum supported version.
+
+        Raises:
+        FastSkip   -- If self.op(self.version, version) is truthy.
+        """
+        if (None not in [version, self.version] and
+                not self.op(version, self.version)):
+            raise FastSkip
+
+
+@compat.python_2_bool_compatible
+class ExtensionsSkip(object):
+    """Skip based on supported extensions.
+
+    This class has support for checking extensions.
+
+    >>> test = ExtensionSkip({'foo'})
+    >>> test.test(['bar', 'oof'])
+    Traceback (most recent call last):
+        ...
+    framework.test.opengl.FastSkip: ...
+
+    >>> test = VersionSkip({'foo'})
+    >>> test.test(['foo', 'bar, 'oof'])
+
+    Keyword Arguments:
+    extensions -- An iterable of extensions that are required by the test.
+    """
+
+    __slots__ = ['extensions']
+
+    def __init__(self, extensions=None):
+        self.extensions = extensions or set()
+
+    def test(self, extensions):
+        """Test whether the Test should skip.
+
+        Arguments
+        extensions -- The list of available extensions.
+
+        Raises:
+        FastSkip   -- If self.op(self.version, version) is truthy.
+        """
+        # If there are no requirements it should run, obviously.
+        if extensions:
+            for extension in self.extensions:
+                if extension not in extensions:
+                    raise FastSkip(msg=extension)
+
+    def __bool__(self):
+        return bool(self.extensions)
+
+
 class FastSkipMixin(object):
     """Fast test skipping for OpenGL based suites.
 
@@ -322,7 +428,7 @@  class FastSkipMixin(object):
                  gles_version=None, glsl_version=None, glsl_es_version=None,
                  **kwargs):  # pylint: disable=too-many-arguments
         super(FastSkipMixin, self).__init__(command, **kwargs)
-        self.gl_required = gl_required or set()
+        self.gl_required = gl_required or ExtensionsSkip()
         self.gl_version = gl_version
         self.gles_version = gles_version
         self.glsl_version = glsl_version
@@ -333,50 +439,63 @@  class FastSkipMixin(object):
 
         If no extensions were calculated (if wflinfo isn't installed) then run
         all tests.
-
         """
-        if self.__info.gl_extensions:
-            for extension in self.gl_required:
-                if extension not in self.__info.gl_extensions:
-                    raise TestIsSkip(
-                        'Test requires extension {} '
-                        'which is not available'.format(extension))
-
-        # TODO: Be able to handle any operator
-        if (self.__info.gl_version is not None
-                and self.gl_version is not None
-                and self.gl_version > self.__info.gl_version):
-            raise TestIsSkip(
-                'Test requires OpenGL version {}, '
-                'but only {} is available'.format(
-                    self.gl_version, self.__info.gl_version))
-
-        # TODO: Be able to handle any operator
-        if (self.__info.gles_version is not None
-                and self.gles_version is not None
-                and self.gles_version > self.__info.gles_version):
-            raise TestIsSkip(
-                'Test requires OpenGL ES version {}, '
-                'but only {} is available'.format(
-                    self.gles_version, self.__info.gles_version))
-
-        # TODO: Be able to handle any operator
-        if (self.__info.glsl_version is not None
-                and self.glsl_version is not None
-                and self.glsl_version > self.__info.glsl_version):
-            raise TestIsSkip(
-                'Test requires OpenGL Shader Language version {}, '
-                'but only {} is available'.format(
-                    self.glsl_version, self.__info.glsl_version))
-
-        # TODO: Be able to handle any operator
-        if (self.__info.glsl_es_version is not None
-                and self.glsl_es_version is not None
-                and self.glsl_es_version > self.__info.glsl_es_version):
-            raise TestIsSkip(
-                'Test requires OpenGL ES Shader Language version {}, '
-                'but only {} is available'.format(
-                    self.glsl_es_version, self.__info.glsl_es_version))
+        # Handle extension requirements.
+        if self.gl_required:
+            try:
+                self.gl_required.test(self.__info.gl_extensions)
+            except FastSkip as e:
+                raise TestIsSkip(
+                    'Test requires extension {} '
+                    'which is not available'.format(e.msg))
+
+        # Handle OpenGL version.
+        if self.gl_version is not None:
+            try:
+                self.gl_version.test(self.__info.gl_version)
+            except FastSkip:
+                raise TestIsSkip(
+                    'Test requires OpenGL version {} {}, '
+                    'but {} is available'.format(
+                        self.gl_version.op,
+                        self.gl_version.version,
+                        self.__info.gl_version))
+
+        # Handle OpenGL ES version.
+        if self.gles_version is not None:
+            try:
+                self.gles_version.test(self.__info.gles_version)
+            except FastSkip:
+                raise TestIsSkip(
+                    'Test requires OpenGL ES version {} {}, '
+                    'but {} is available'.format(
+                        self.gles_version.op,
+                        self.gles_version.version,
+                        self.__info.gles_version))
+
+        # Handle OpenGL SL version.
+        if self.glsl_version is not None:
+            try:
+                self.glsl_version.test(self.__info.glsl_version)
+            except FastSkip:
+                raise TestIsSkip(
+                    'Test requires OpenGL SL version {} {}, '
+                    'but {} is available'.format(
+                        self.glsl_version.op,
+                        self.glsl_version.version,
+                        self.__info.glsl_version))
+
+        # Handle OpenGL ES SL version.
+        if self.glsl_es_version is not None:
+            try:
+                self.glsl_es_version.test(self.__info.glsl_es_version)
+            except FastSkip:
+                raise TestIsSkip(
+                    'Test requires OpenGL ES SL version {} {}, '
+                    'but only {} is available'.format(
+                        self.glsl_es_version.op,
+                        self.glsl_es_version.version,
+                        self.__info.glsl_es_version))
 
         super(FastSkipMixin, self).is_skip()
 
@@ -387,7 +506,7 @@  class FastSkipMixinDisabled(object):
                  **kwargs):  # pylint: disable=too-many-arguments
         # Tests that implement the FastSkipMixin expect to have these values
         # set, so just fill them in with the default values.
-        self.gl_required = set()
+        self.gl_required = None
         self.gl_version = None
         self.gles_version = None
         self.glsl_version = None
diff --git a/framework/test/shader_test.py b/framework/test/shader_test.py
index e67c5ed..77c1938 100644
--- a/framework/test/shader_test.py
+++ b/framework/test/shader_test.py
@@ -27,10 +27,11 @@  from __future__ import (
     absolute_import, division, print_function, unicode_literals
 )
 import io
+import operator
 import re
 
 from framework import exceptions
-from .opengl import FastSkipMixin
+from .opengl import FastSkipMixin, VersionSkip, ExtensionsSkip
 from .piglit_test import PiglitBaseTest
 
 __all__ = [
@@ -38,6 +39,18 @@  __all__ = [
 ]
 
 
+_OPS = {
+    '>': operator.gt,
+    '>=': operator.ge,
+    '==': operator.eq,
+    '=': operator.eq,
+    '!=': operator.ne,
+    '<=': operator.le,
+    '<': operator.lt,
+    None: lambda x, y: False,
+}
+
+
 class ShaderTest(FastSkipMixin, PiglitBaseTest):
     """ Parse a shader test file and return a PiglitTest instance
 
@@ -126,13 +139,12 @@  class ShaderTest(FastSkipMixin, PiglitBaseTest):
         super(ShaderTest, self).__init__(
             [prog, filename],
             run_concurrent=True,
-            gl_required=gl_required,
-            # FIXME: the if here is related to an incomplete feature in the
-            # FastSkipMixin
-            gl_version=gl_version if op not in ['<', '<='] else None,
-            gles_version=gles_version if op not in ['<', '<='] else None,
-            glsl_version=glsl_version if sl_op not in ['<', '<='] else None,
-            glsl_es_version=glsl_es_version if sl_op not in ['<', '<='] else None)
+            gl_required=ExtensionsSkip(gl_required),
+            gl_version=VersionSkip(_OPS[op], gl_version, rep=op),
+            gles_version=VersionSkip(_OPS[op], gles_version, rep=op),
+            glsl_version=VersionSkip(_OPS[sl_op], glsl_version, rep=sl_op),
+            glsl_es_version=VersionSkip(_OPS[sl_op], glsl_es_version,
+                                        rep=sl_op))
 
     @PiglitBaseTest.command.getter
     def command(self):
diff --git a/unittests/framework/test/test_glsl_parser_test.py b/unittests/framework/test/test_glsl_parser_test.py
index 124f3e9..7ba8fa3 100644
--- a/unittests/framework/test/test_glsl_parser_test.py
+++ b/unittests/framework/test/test_glsl_parser_test.py
@@ -383,24 +383,24 @@  class TestGLSLParserTestSkipRequirements(object):
 
     def test_glsl_version(self, tmpdir):
         p = tmpdir.join('test.frag')
-        self.write_config(p)
-        assert glsl.GLSLParserTest(six.text_type(p)).glsl_version == 4.3
+        self.write_config(p, version='4.3')
+        assert glsl.GLSLParserTest(six.text_type(p)).glsl_version.version == 4.3
 
     def test_glsl_es_version(self, tmpdir):
         p = tmpdir.join('test.frag')
         self.write_config(p, version='3.0')
-        assert glsl.GLSLParserTest(six.text_type(p)).glsl_es_version == 3.0
+        assert glsl.GLSLParserTest(six.text_type(p)).glsl_es_version.version == 3.0
 
     def test_gl_required(self, tmpdir):
         p = tmpdir.join('test.frag')
         self.write_config(p, extra="require_extensions: GL_ARB_foo GL_ARB_bar")
-        assert glsl.GLSLParserTest(six.text_type(p)).gl_required == \
+        assert glsl.GLSLParserTest(six.text_type(p)).gl_required.extensions == \
             {'GL_ARB_foo', 'GL_ARB_bar'}
 
     def test_exclude_not_added_to_gl_required(self, tmpdir):
         p = tmpdir.join('test.frag')
         self.write_config(p, extra="require_extensions: GL_ARB_foo !GL_ARB_bar")
-        assert glsl.GLSLParserTest(six.text_type(p)).gl_required == \
+        assert glsl.GLSLParserTest(six.text_type(p)).gl_required.extensions == \
             {'GL_ARB_foo'}
 
 
@@ -449,7 +449,7 @@  def test_add_compatibility_requirement_fastskip(version, extension, tmpdir,
     test = glsl.GLSLParserTest(six.text_type(p))
 
     # The arb_compat extension was added to the fast skipping arguments
-    assert extension in test.gl_required
+    assert extension in test.gl_required.extensions
 
 
 
diff --git a/unittests/framework/test/test_opengl.py b/unittests/framework/test/test_opengl.py
index 6381373..3a03648 100644
--- a/unittests/framework/test/test_opengl.py
+++ b/unittests/framework/test/test_opengl.py
@@ -23,6 +23,7 @@ 
 from __future__ import (
     absolute_import, division, print_function, unicode_literals
 )
+import operator
 import subprocess
 import textwrap
 try:
@@ -399,7 +400,11 @@  class TestFastSkipMixin(object):  # pylint: disable=too-many-public-methods
     class _Test(opengl.FastSkipMixin, utils.Test):
         pass
 
-    @pytest.yield_fixture(autouse=True, scope='class')
+    @pytest.fixture
+    def inst(self):
+        return self._Test(['foo'])
+
+    @pytest.yield_fixture(autouse=True, scope='module')
     def patch(self):
         """Create a Class with FastSkipMixin, but patch various bits."""
         _mock_wflinfo = mock.Mock(spec=opengl.WflInfo)
@@ -413,10 +418,6 @@  class TestFastSkipMixin(object):  # pylint: disable=too-many-public-methods
                                _mock_wflinfo):
             yield
 
-    @pytest.fixture
-    def inst(self):
-        return self._Test(['foo'])
-
     def test_api(self):
         """Tests that the api works.
 
@@ -431,7 +432,7 @@  class TestFastSkipMixin(object):  # pylint: disable=too-many-public-methods
         """test.opengl.FastSkipMixin.is_skip: Skips when requires is missing
         from extensions.
         """
-        inst.gl_required.add('foobar')
+        inst.gl_required.extensions.add('foobar')
         with pytest.raises(_TestIsSkip):
             inst.is_skip()
 
@@ -439,131 +440,69 @@  class TestFastSkipMixin(object):  # pylint: disable=too-many-public-methods
         """test.opengl.FastSkipMixin.is_skip: runs when requires is in
         extensions.
         """
-        inst.gl_required.add('bar')
-        inst.is_skip()
-
-    def test_extension_empty(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: if extensions are empty test
-        runs.
-        """
-        inst.gl_required.add('foobar')
-        with mock.patch.object(inst._FastSkipMixin__info, 'gl_extensions',  # pylint: disable=no-member
-                               None):
-            inst.is_skip()
-
-    def test_requires_empty(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: if gl_requires is empty test
-        runs.
-        """
+        inst.gl_required.extensions.add('bar')
         inst.is_skip()
 
-    def test_max_gl_version_lt(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: skips if gl_version >
-        __max_gl_version.
-        """
-        inst.gl_version = 4.0
-        with pytest.raises(_TestIsSkip):
-            inst.is_skip()
-
-    def test_max_gl_version_gt(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: runs if gl_version <
-        __max_gl_version.
-        """
-        inst.gl_version = 1.0
-
-    def test_max_gl_version_unset(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: runs if __max_gl_version is
-        None.
-        """
-        inst.gl_version = 1.0
-        with mock.patch.object(inst._FastSkipMixin__info, 'gl_version',  # pylint: disable=no-member
-                               None):
-            inst.is_skip()
-
-    def test_max_gl_version_set(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: runs if gl_version is None"""
-        inst.is_skip()
-
-    def test_max_gles_version_lt(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: skips if gles_version >
-        __max_gles_version.
-        """
-        inst.gles_version = 4.0
-        with pytest.raises(_TestIsSkip):
-            inst.is_skip()
 
-    def test_max_gles_version_gt(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: runs if gles_version <
-        __max_gles_version.
-        """
-        inst.gles_version = 1.0
-
-    def test_max_gles_version_unset(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: runs if __max_gles_version is
-        None.
-        """
-        inst.gles_version = 1.0
-        with mock.patch.object(inst._FastSkipMixin__info, 'gles_version',  # pylint: disable=no-member
-                               None):
-            inst.is_skip()
-
-    def test_max_gles_version_set(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: runs if gles_version is None"""
-        inst.is_skip()
-
-    def test_max_glsl_version_lt(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: skips if glsl_version >
-        __max_glsl_version.
-        """
-        inst.glsl_version = 4.0
-        with pytest.raises(_TestIsSkip):
-            inst.is_skip()
-
-    def test_max_glsl_version_gt(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: runs if glsl_version <
-        __max_glsl_version.
-        """
-        inst.glsl_version = 1.0
-
-    def test_max_glsl_version_unset(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: runs if __max_glsl_version is
-        None.
-        """
-        inst.glsl_version = 1.0
-        with mock.patch.object(inst._FastSkipMixin__info, 'glsl_version',  # pylint: disable=no-member
-                               None):
-            inst.is_skip()
-
-    def test_max_glsl_version_set(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: runs if glsl_version is None"""
-        inst.is_skip()
-
-    def test_max_glsl_es_version_lt(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: skips if glsl_es_version >
-        __max_glsl_es_version.
-        """
-        inst.glsl_es_version = 4.0
-        with pytest.raises(_TestIsSkip):
-            inst.is_skip()
-
-    def test_max_glsl_es_version_gt(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: runs if glsl_es_version <
-        __max_glsl_es_version.
-        """
-        inst.glsl_es_version = 1.0
-
-    def test_max_glsl_es_version_unset(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: runs if __max_glsl_es_version is
-        None.
-        """
-        inst.glsl_es_version = 1.0
-        with mock.patch.object(inst._FastSkipMixin__info, 'glsl_es_version',  # pylint: disable=no-member
-                               None):
-            inst.is_skip()
-
-    def test_max_glsl_es_version_set(self, inst):
-        """test.opengl.FastSkipMixin.is_skip: runs if glsl_es_version is None"""
-        inst.is_skip()
+class TestExtensionSkip(object):
+    """Tests for the ExtensionSkip class."""
+
+    def test_extension_empty(self):
+        """If extensions pased to .test() are empty don't raise."""
+        inst = opengl.ExtensionsSkip()
+        inst.test(set())
+
+    def test_requires_empty(self):
+        inst = opengl.ExtensionsSkip()
+        inst.test(['bar'])
+
+    def test_in(self):
+        inst = opengl.ExtensionsSkip(['bar'])
+        inst.test(['bar'])
+
+    def test_not_in(self):
+        inst = opengl.ExtensionsSkip(['foo'])
+        with pytest.raises(opengl.FastSkip):
+            inst.test(['bar'])
+
+
+def _version_name(val):
+    if isinstance(val, (float, bool)):
+        return val
+    return val.__name__
+
+
+class TestVersionSkip(object):
+    """Tests for the version skip class."""
+
+    @pytest.mark.parametrize('op, required, available, skip', [
+        (operator.ge, 3.0, 2.0, True),
+        (operator.ge, 3.0, 3.0, False),
+        (operator.ge, 3.0, 4.0, False),
+        (operator.gt, 3.0, 2.0, True),
+        (operator.gt, 3.0, 3.0, True),
+        (operator.gt, 3.0, 4.0, False),
+        (operator.eq, 3.0, 2.0, True),
+        (operator.eq, 3.0, 3.0, False),
+        (operator.eq, 3.0, 4.0, True),
+        (operator.lt, 3.0, 2.0, False),
+        (operator.lt, 3.0, 3.0, True),
+        (operator.lt, 3.0, 4.0, True),
+        (operator.le, 3.0, 2.0, False),
+        (operator.le, 3.0, 3.0, False),
+        (operator.le, 3.0, 4.0, True),
+    ], ids=_version_name)
+    def test_version(self, op, required, available, skip):
+        inst = opengl.VersionSkip(op, required)
+        if skip:
+            with pytest.raises(opengl.FastSkip):
+                inst.test(available)
+        else:
+            inst.test(available)
+
+    def test_version_unset(self):
+        inst = opengl.VersionSkip(operator.lt, 3.0)
+        inst.test(None)
 
 
 class TestFastSkipMixinDisabled(object):
diff --git a/unittests/framework/test/test_shader_test.py b/unittests/framework/test/test_shader_test.py
index 9cf8b60..311c44d 100644
--- a/unittests/framework/test/test_shader_test.py
+++ b/unittests/framework/test/test_shader_test.py
@@ -23,6 +23,7 @@ 
 from __future__ import (
     absolute_import, division, print_function, unicode_literals
 )
+import operator
 import os
 import textwrap
 try:
@@ -120,7 +121,7 @@  class TestConfigParsing(object):
             """))
         test = shader_test.ShaderTest(six.text_type(p))
 
-        assert test.gl_required == {'GL_ARB_ham_sandwhich'}
+        assert test.gl_required.extensions == {'GL_ARB_ham_sandwhich'}
 
     def test_skip_gl_version(self, tmpdir):
         """test.shader_test.ShaderTest: finds gl_version."""
@@ -132,7 +133,7 @@  class TestConfigParsing(object):
             """))
         test = shader_test.ShaderTest(six.text_type(p))
 
-        assert test.gl_version == 2.0
+        assert test.gl_version.version == 2.0
 
     def test_skip_gles_version(self, tmpdir):
         """test.shader_test.ShaderTest: finds gles_version."""
@@ -144,7 +145,7 @@  class TestConfigParsing(object):
             """))
         test = shader_test.ShaderTest(six.text_type(p))
 
-        assert test.gles_version == 2.0
+        assert test.gles_version.version == 2.0
 
     def test_skip_glsl_version(self, tmpdir):
         """test.shader_test.ShaderTest: finds glsl_version."""
@@ -156,7 +157,7 @@  class TestConfigParsing(object):
             """))
         test = shader_test.ShaderTest(six.text_type(p))
 
-        assert test.glsl_version == 1.2
+        assert test.glsl_version.version == 1.2
 
     def test_skip_glsl_es_version(self, tmpdir):
         """test.shader_test.ShaderTest: finds glsl_es_version."""
@@ -168,7 +169,7 @@  class TestConfigParsing(object):
             """))
         test = shader_test.ShaderTest(six.text_type(p))
 
-        assert test.glsl_es_version == 1.0
+        assert test.glsl_es_version.version == 1.0
 
     def test_ignore_directives(self, tmpdir):
         """There are some directives for shader_runner that are not interpreted
@@ -179,7 +180,7 @@  class TestConfigParsing(object):
         p.write(textwrap.dedent("""\
             [require]
             GL >= 3.3
-            GLSL >= 1.50
+            GLSL <= 1.50
             GL_MAX_VERTEX_OUTPUT_COMPONENTS
             GL_MAX_FRAGMENT_UNIFORM_COMPONENTS
             GL_MAX_VERTEX_UNIFORM_COMPONENTS
@@ -188,9 +189,11 @@  class TestConfigParsing(object):
             """))
         test = shader_test.ShaderTest(six.text_type(p))
 
-        assert test.gl_version == 3.3
-        assert test.glsl_version == 1.50
-        assert test.gl_required == {'GL_ARB_foobar'}
+        assert test.gl_version.version == 3.3
+        assert test.gl_version.op == operator.ge
+        assert test.glsl_version.version == 1.50
+        assert test.glsl_version.op == operator.le
+        assert test.gl_required.extensions == {'GL_ARB_foobar'}
 
 
 def test_command_add_auto(tmpdir):