[Piglit] [PATCH v4 2/3] framework/test/glsl_parser_test.py: Handle gl versions correctly

baker.dylan.c at gmail.com baker.dylan.c at gmail.com
Fri Nov 20 17:37:12 PST 2015


From: Dylan Baker <baker.dylan.c at gmail.com>

This patch fixes the behavior of glsl_parser_test in cases other that
OpenGL and OpenGL ES are available. This means that if OpenGL ES isn't
available then OpenGL ES shaders will be passed to the regular version
of glslparsertest, which can run them with an ARB_ES<ver>_compatibility
extension. When a test is for desktop OpenGL, but only OpenGL ES is
available, then the test will skip in the python layer.

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

v3: - Rebase on top of now upstream fast-skipping patches
    - When using glslparsertest to run GLES tests add
      ARB_ES<ver>_compatibility to gl_required, to integrate with fast
      skipping.
    - Fix some tests. These tests are pretty fragile and probably just
      need to be reworked.
    - small refactors, make use of the _is_gles_version helper
    - Split skip conditions into a private method
v4: - Fix bug where ARB_ES<ver>_compatibility skip requirement would be
      overwritten if the test had other extension requirements (and
      update tests to catch this bug)
    - Rename some constants to be more clear
    - Remove use of flag for skipping.
---
 framework/test/glsl_parser_test.py        |  61 +++++++++++--
 framework/tests/glsl_parser_test_tests.py | 141 +++++++++++++++++++++---------
 2 files changed, 156 insertions(+), 46 deletions(-)

diff --git a/framework/test/glsl_parser_test.py b/framework/test/glsl_parser_test.py
index 6d91c22..7ab03af 100644
--- a/framework/test/glsl_parser_test.py
+++ b/framework/test/glsl_parser_test.py
@@ -26,14 +26,19 @@ import os
 import re
 
 from framework import exceptions
-from .piglit_test import PiglitBaseTest
+from .base import TestIsSkip
 from .opengl import FastSkipMixin
+from .piglit_test import PiglitBaseTest, TEST_BIN_DIR
 
 __all__ = [
     'GLSLParserTest',
     'GLSLParserNoConfigError',
 ]
 
+# In different configurations piglit may have one or both of these.
+_HAS_GL_BIN = os.path.exists(os.path.join(TEST_BIN_DIR, 'glslparsertest'))
+_HAS_GLES_BIN = os.path.exists(os.path.join(TEST_BIN_DIR, 'glslparsertest_gles2'))
+
 
 def _is_gles_version(version):
     """Return True if version is es, otherwsie false."""
@@ -42,6 +47,7 @@ def _is_gles_version(version):
         # glslparsertest doesn't honor them.
         if version.endswith('es'):
             return True
+
         version = float(version)
 
     return version in [1.0, 3.0, 3.1, 3.2]
@@ -89,12 +95,14 @@ class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
 
         super(GLSLParserTest, self).__init__(command, run_concurrent=True)
 
+        self.__set_skip_conditions(config)
+
+    def __set_skip_conditions(self, config):
+        """Set OpenGL and OpenGL ES fast skipping conditions."""
         glsl = config.get('glsl_version')
         if glsl:
-            if glsl in ['1.00', '3.00']:
-                self.glsl_es_version = float(glsl)
-            elif glsl.endswith('es'):
-                self.glsl_es_version = float(glsl.split()[0])
+            if _is_gles_version(glsl):
+                self.glsl_es_version = float(glsl[:3])
             else:
                 self.glsl_version = float(glsl)
 
@@ -102,6 +110,40 @@ class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
         if req:
             self.gl_required = set(req.split())
 
+        # 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:
+                ver = '2'
+            elif self.glsl_es_version == 3.0:
+                ver = '3'
+            elif self.glsl_es_version == 3.1:
+                ver = '3_1'
+            elif self.glsl_es_version == 3.2:
+                ver = '3_2'
+            self.gl_required.add('ARB_ES{}_compatibility'.format(ver))
+
+    @staticmethod
+    def __pick_binary(version):
+        """Pick the correct version of glslparsertest to use.
+
+        This will try to select glslparsertest_gles2 for OpenGL ES tests, and
+        glslparsertest for desktop OpenGL tests. However, sometimes this isn't
+        possible. In that case all tests will be assigned to the desktop
+        version.
+
+        If the test requires desktop OpenGL, but only OpenGL ES is available,
+        then the test will be skipped in the python layer.
+
+        """
+        if _is_gles_version(version) and _HAS_GLES_BIN:
+            return 'glslparsertest_gles2'
+        elif _HAS_GL_BIN:
+            return 'glslparsertest'
+        else:
+            return 'None'
+
     def __get_command(self, config, filepath):
         """ Create the command argument to pass to super()
 
@@ -120,7 +162,7 @@ class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
         # Create the command and pass it into a PiglitTest()
         glsl = config['glsl_version']
         command = [
-            'glslparsertest_gles2' if _is_gles_version(glsl) else 'glslparsertest',
+            self.__pick_binary(glsl),
             filepath,
             config['expect_result'],
             config['glsl_version']
@@ -208,3 +250,10 @@ class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
             raise GLSLParserInternalError("No [end config] section found!")
 
         return keys
+
+    def is_skip(self):
+        if os.path.basename(self.command[0]) == 'None':
+            raise TestIsSkip('Test is for desktop OpenGL, '
+                             'but only an OpenGL ES binary has been built')
+
+        super(GLSLParserTest, self).is_skip()
diff --git a/framework/tests/glsl_parser_test_tests.py b/framework/tests/glsl_parser_test_tests.py
index ba02947..b8e0e20 100644
--- a/framework/tests/glsl_parser_test_tests.py
+++ b/framework/tests/glsl_parser_test_tests.py
@@ -30,25 +30,32 @@ import nose.tools as nt
 from framework import exceptions
 import framework.test.glsl_parser_test as glsl
 import framework.tests.utils as utils
-from framework.test import TEST_BIN_DIR
+from framework.test import TEST_BIN_DIR, TestIsSkip
 
 # pylint: disable=line-too-long,invalid-name
 
 
 class _Setup(object):
+    """A class holding setup and teardown methods.
+
+    These methods need to share data, and a class is a nice way to encapsulate
+    that.
+
+    """
     def __init__(self):
-        self.__patchers = []
-        self.__patchers.append(mock.patch.dict(
-            'framework.test.opengl.OPTIONS.env',
-            {'PIGLIT_PLATFORM': 'foo'}))
+        self.patchers = [
+            mock.patch('framework.test.glsl_parser_test._HAS_GL_BIN', True),
+            mock.patch('framework.test.glsl_parser_test._HAS_GLES_BIN', True),
+            mock.patch.dict('framework.test.opengl.OPTIONS.env', {'PIGLIT_PLATFORM': 'foo'}),
+        ]
 
     def setup(self):
-        for patcher in self.__patchers:
-            patcher.start()
+        for p in self.patchers:
+            p.start()
 
     def teardown(self):
-        for patcher in self.__patchers:
-            patcher.stop()
+        for p in self.patchers:
+            p.stop()
 
 
 _setup = _Setup()
@@ -354,12 +361,10 @@ def test_good_extensions():
 @utils.nose_generator
 def test_get_glslparsertest_gles2():
     """GLSLParserTest: gets gles2 binary if glsl is 1.00 or 3.00"""
-
-    @utils.no_error
-    def test(content):
+    def test(content, expected):
         with utils.tempfile(content) as f:
             t = glsl.GLSLParserTest(f)
-            nt.eq_(os.path.basename(t.command[0]), 'glslparsertest_gles2')
+            nt.eq_(os.path.basename(t.command[0]), expected)
 
     content = textwrap.dedent("""\
         /*
@@ -369,16 +374,22 @@ def test_get_glslparsertest_gles2():
          * [end config]
          */
         """)
-
-    description = ("test.glsl_parser_test.GLSLParserTest: "
-                   "gets gles2 binary if glsl is {}")
-
     versions = ['1.00', '3.00', '3.10', '3.20', '3.00 es', '3.10 es',
                 '3.20 es']
+    description = ("test.glsl_parser_test.GLSLParserTest: "
+                   "gets gles2 binary if glsl is '{}' and gles2 binary exists")
 
     for version in versions:
         test.description = description.format(version)
-        yield test, content.format(version)
+        yield test, content.format(version), 'glslparsertest_gles2'
+
+    description = ("test.glsl_parser_test.GLSLParserTest: "
+                   "gets gl binary if glsl is '{}' and gles2 binary doesn't exist")
+
+    with mock.patch('framework.test.glsl_parser_test._HAS_GLES_BIN', False):
+        for version in versions:
+            test.description = description.format(version)
+            yield test, content.format(version), 'glslparsertest'
 
 
 def test_set_glsl_version():
@@ -387,14 +398,13 @@ def test_set_glsl_version():
     with mock.patch.object(glsl.GLSLParserTest, '_GLSLParserTest__parser',
                            mock.Mock(return_value=rt)):
         with mock.patch.object(glsl.GLSLParserTest,
-                               '_GLSLParserTest__get_command'):
-            with mock.patch('framework.test.glsl_parser_test.super',
-                            mock.Mock()):
-                with mock.patch('framework.test.glsl_parser_test.open',
+                               '_GLSLParserTest__get_command',
+                               return_value=['foo']):
+            with mock.patch('framework.test.glsl_parser_test.open',
+                            mock.mock_open()):
+                with mock.patch('framework.test.glsl_parser_test.os.stat',
                                 mock.mock_open()):
-                    with mock.patch('framework.test.glsl_parser_test.os.stat',
-                                    mock.mock_open()):
-                        test = glsl.GLSLParserTest('foo')
+                    test = glsl.GLSLParserTest('foo')
     nt.eq_(test.glsl_version, 4.3)
 
 
@@ -404,29 +414,80 @@ def test_set_glsl_es_version():
     with mock.patch.object(glsl.GLSLParserTest, '_GLSLParserTest__parser',
                            mock.Mock(return_value=rt)):
         with mock.patch.object(glsl.GLSLParserTest,
-                               '_GLSLParserTest__get_command'):
-            with mock.patch('framework.test.glsl_parser_test.super',
-                            mock.Mock()):
-                with mock.patch('framework.test.glsl_parser_test.open',
+                               '_GLSLParserTest__get_command',
+                               return_value=['foo']):
+            with mock.patch('framework.test.glsl_parser_test.open',
+                            mock.mock_open()):
+                with mock.patch('framework.test.glsl_parser_test.os.stat',
                                 mock.mock_open()):
-                    with mock.patch('framework.test.glsl_parser_test.os.stat',
-                                    mock.mock_open()):
-                        test = glsl.GLSLParserTest('foo')
+                    test = glsl.GLSLParserTest('foo')
     nt.eq_(test.glsl_es_version, 3.0)
 
 
 def test_set_gl_required():
-    """test.glsl_parser_test.GLSLParserTest: sets glsl_es_version"""
+    """test.glsl_parser_test.GLSLParserTest: sets gl_required"""
     rt = {'require_extensions': 'GL_ARB_foobar GL_EXT_foobar'}
     with mock.patch.object(glsl.GLSLParserTest, '_GLSLParserTest__parser',
                            mock.Mock(return_value=rt)):
         with mock.patch.object(glsl.GLSLParserTest,
-                               '_GLSLParserTest__get_command'):
-            with mock.patch('framework.test.glsl_parser_test.super',
-                            mock.Mock()):
-                with mock.patch('framework.test.glsl_parser_test.open',
+                               '_GLSLParserTest__get_command',
+                               return_value=['foo']):
+            with mock.patch('framework.test.glsl_parser_test.open',
+                            mock.mock_open()):
+                with mock.patch('framework.test.glsl_parser_test.os.stat',
                                 mock.mock_open()):
-                    with mock.patch('framework.test.glsl_parser_test.os.stat',
-                                    mock.mock_open()):
-                        test = glsl.GLSLParserTest('foo')
+                    test = glsl.GLSLParserTest('foo')
     nt.eq_(test.gl_required, set(['GL_ARB_foobar', 'GL_EXT_foobar']))
+
+
+ at mock.patch('framework.test.glsl_parser_test._HAS_GL_BIN', False)
+ at nt.raises(TestIsSkip)
+def test_binary_skip():
+    """test.glsl_parser_test.GLSLParserTest.is_skip: skips OpenGL tests when not built with desktop support"""
+    content = textwrap.dedent("""\
+        /*
+         * [config]
+         * expect_result: pass
+         * glsl_version: 1.10
+         * [end config]
+         */
+        """)
+
+    with utils.tempfile(content) as f:
+        test = glsl.GLSLParserTest(f)
+        test.is_skip()
+
+
+ at utils.nose_generator
+def test_add_compatability():
+    """test.glsl_parser_test.GLSLParserTest: Adds ARB_ES<ver>_COMPATIBILITY
+    when shader is gles but only gl is available"""
+    content = textwrap.dedent("""\
+        /*
+         * [config]
+         * expect_result: pass
+         * glsl_version: {}
+         * require_extensions: GL_ARB_ham_sandwhich
+         * [end config]
+         */
+        """)
+
+    @mock.patch('framework.test.glsl_parser_test._HAS_GLES_BIN', False)
+    def test(ver, expected):
+        with utils.tempfile(content.format(ver)) as f:
+            test = glsl.GLSLParserTest(f)
+        nt.assert_in(expected, test.gl_required)
+
+    desc = ('test.glsl_parser_test.GLSLParserTest: Add {} to gl_extensions '
+            'for GLES tests on OpenGL')
+
+    vers = [
+        ('1.00', 'ARB_ES2_compatibility'),
+        ('3.00', 'ARB_ES3_compatibility'),
+        ('3.10', 'ARB_ES3_1_compatibility'),
+        ('3.20', 'ARB_ES3_2_compatibility'),
+    ]
+
+    for ver, expected in vers:
+        test.description = desc.format(expected)
+        yield test, ver, expected
-- 
2.6.2



More information about the Piglit mailing list