[Piglit] [Patch v2 07/11] glsl_parser_test.py: Simplify GLSLParserTest

Dylan Baker baker.dylan.c at gmail.com
Thu Mar 27 15:34:00 PDT 2014


This vastly simplifies the way that GLSLParserTest works. This
simplification has a couple of advantages. First, we call
PlainExecTest's __init__ rather than ExecTest's __init__, which is good
OO practice; second we reduce the number of regex's used, which yields
enhanced performance. In tests this reduced GLSLParserTest
initialization time by about 40%

Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
---
 framework/glsl_parser_test.py             | 347 ++++++------------------------
 framework/tests/glsl_parser_test_tests.py | 112 ++++++----
 2 files changed, 139 insertions(+), 320 deletions(-)

diff --git a/framework/glsl_parser_test.py b/framework/glsl_parser_test.py
index 35efb02..e750d59 100644
--- a/framework/glsl_parser_test.py
+++ b/framework/glsl_parser_test.py
@@ -27,7 +27,7 @@ import os.path as path
 import re
 from cStringIO import StringIO
 
-from .core import Test, testBinDir, TestResult
+from .core import testBinDir
 from .exectest import PlainExecTest
 
 
@@ -69,301 +69,94 @@ def import_glsl_parser_tests(group, basepath, subdirectories):
 
 
 class GLSLParserTest(PlainExecTest):
-    """Test for the GLSL parser (and more) on a GLSL source file.
+    """ Read the options in a glsl parser test and create a Test object
 
-    This test takes a GLSL source file and passes it to the executable
-    ``glslparsertest``. The GLSL source file being tested must have a GLSL
-    file extension: one of ``.vert``, ``.geom``, or ``.frag``. The test file
-    must have a properly formatted comment section containing configuration
-    data (see below).
+    Specifically it is necessary to parse a glsl_parser_test to get information
+    about it before actually creating a PlainExecTest. Even though this could
+    be done with a funciton wrapper, making it a distinct class makes it easier
+    to sort in the profile.
 
-    For example test files, see the directory
-    'piglit.repo/examples/glsl_parser_text`.
+    Arguments:
+    filepath -- the path to a glsl_parser_test which must end in .frag, .vert,
+                or .geom
 
-    Quirks
-    ------
-    It is not completely corect to state that this is a test for the GLSL
-    parser, because it also tests later compilation stages, such as AST
-    construction and static type checking. Specifically, this tests the
-    success of the executable ``glslparsertest``, which in turn tests the
-    success of the native function ``glCompileShader()``.
-
-    Config Section
-    --------------
-    The GLSL source file must contain a special config section in its
-    comments. This section can appear anywhere in the file: above,
-    within, or below the actual GLSL source code. The syntax of the config
-    section is essentially the syntax of
-    ``ConfigParser.SafeConfigParser``.
-
-    The beginning of the config section is marked by a comment line that
-    contains only '[config]'. The end of the config section is marked by
-    a comment line that contains only '[end config]'. All intervening
-    lines become the text of the config section.
-
-    Whitespace is significant, because ``ConfigParser`` treats it so.  The
-    config text of each non-empty line begins on the same column as the
-    ``[`` in the ``[config]`` line.  (A line is considered empty if it
-    contains whitespace and an optional C comment marker: ``//``, ``*``,
-    ``/*``). Therefore, option names must be aligned on this column. Text
-    that begins to the right of this column is considered to be a line
-    continuation.
-
-
-    Required Options
-    ----------------
-    * glsl_version: A valid GLSL version number, such as 1.10.
-    * expect_result: Either ``pass`` or ``fail``.
-
-    Nonrequired Options
-    -------------------
-    * require_extensions: List of GL extensions. If an extension is not
-          supported, the test is skipped. Each extension name must begin
-          with GL and elements are separated by whitespace.
-    * check_link: Either ``true`` or ``false``.  A true value passes the
-          --check-link option to be supplied to glslparsertest, which
-          causes it to detect link failures as well as compilation
-          failures.
-
-    Examples
-    --------
-    ::
-            // [config]
-            // glsl_version: 1.30
-            // expect_result: pass
-            // # Lists may be single-line.
-            // require_extensions: GL_ARB_fragment_coord_conventions GL_AMD_conservative_depth
-            // [end config]
-
-    ::
-            /* [config]
-             * glsl_version: 1.30
-             * expect_result: pass
-             * # Lists may be span multiple lines.
-             * required_extensions:
-             *     GL_ARB_fragment_coord_conventions
-             *     GL_AMD_conservative_depth
-             * [end config]
-             */
-
-    ::
-            /*
-            [config]
-            glsl_version: 1.30
-            expect_result: pass
-            check_link: true
-            [end config]
-            */
-
-    An incorrect example, where text is not properly aligned::
-            /* [config]
-            glsl_version: 1.30
-            expect_result: pass
-            [end config]
-            */
-
-    Another alignment problem::
-            // [config]
-            //   glsl_version: 1.30
-            //   expect_result: pass
-            // [end config]
     """
-
-    __required_opts = ['expect_result',
-                       'glsl_version']
-
-    __config_defaults = {'require_extensions': '',
-                         'check_link': 'false'}
-
-    def __init__(self, filepath, runConcurrent=True):
-        """
-        :filepath: Must end in one '.vert', '.geom', or '.frag'.
-        """
-        Test.__init__(self, runConcurrent)
-        self.__config = None
-        self.__command = None
-        self.__filepath = filepath
-        self.result = None
-
-    def __get_config(self):
-        """Extract the config section from the test file.
-
-        Set ``self.__cached_config``.  If the config section is missing
-        or invalid, or any other errors occur, then set ``self.result``
-        to failure.
-
-        :return: None
-        """
-
-        cls = self.__class__
-
+    def __init__(self, filepath):
         # Text of config section.
         text_io = StringIO()
+        text_io.write('[config]\n')
 
-        # Parsing state.
-        PARSE_FIND_START = 0
-        PARSE_IN_CONFIG = 1
-        PARSE_DONE = 2
-        PARSE_ERROR = 3
-        parse_state = PARSE_FIND_START
+        os.stat(filepath)
 
-        # Regexen that change parser state.
-        start = re.compile(r'\A(?P<indent>\s*(|//|/\*|\*)\s*)'
-                           '(?P<content>\[config\]\s*\n)\Z')
-        empty = None  # Empty line in config body.
-        internal = None  # Non-empty line in config body.
-        end = None  # Marks end of config body.
+        # Parse the config file and get the config section, then write this
+        # section to a StringIO and pass that to ConfigParser
+        with open(filepath, 'r') as testfile:
 
-        try:
-            f = open(self.__filepath, 'r')
-        except IOError:
-            self.result = TestResult()
-            self.result['result'] = 'fail'
-            self.result['errors'] = \
-                ["Failed to open test file '{0}'".format(self.__filepath)]
-            return
-        for line in f:
-            if parse_state == PARSE_FIND_START:
-                m = start.match(line)
-                if m is not None:
-                    parse_state = PARSE_IN_CONFIG
-                    text_io.write(m.group('content'))
-                    indent = '.' * len(m.group('indent'))
-                    empty = re.compile(r'\A\s*(|//|/\*|\*)\s*\n\Z')
-                    internal = re.compile(r'\A{indent}(?P<content>'
-                                          '.*\n)\Z'.format(indent=indent))
-                    end = re.compile(r'\A{indent}\[end( |_)'
-                                     'config\]\s*\n\Z'.format(indent=indent))
-            elif parse_state == PARSE_IN_CONFIG:
-                if start.match(line) is not None:
-                    parse_state = PARSE_ERROR
-                    break
-                if end.match(line) is not None:
-                    parse_state = PARSE_DONE
+            # Create a generator that iterates over the lines in the test file.
+            # This allows us to run the loop until we find the header, stop and
+            # then run again looking for the config sections. This reduces if
+            # checking substantially.
+            lines = (l for l in testfile)
+
+            is_header = re.compile(r'\s*(//|/\*|\*)\s*\[config\]')
+            for line in lines:
+                if is_header.match(line):
                     break
-                m = internal.match(line)
-                if m is not None:
-                    text_io.write(m.group('content'))
-                    continue
-                m = empty.match(line)
-                if m is not None:
-                    text_io.write('\n')
+            else:
+                raise GLSLParserException("No [config] section found!")
+
+            is_header = re.compile(r'\s*(//|/\*|\*)\s*\[end config\]')
+            for line in lines:
+                # Remove all leading whitespace
+                line = line.strip()
+
+                # If strip renendered '' that means we had a blank newline,
+                # just go on
+                if line == '':
                     continue
-                parse_state = PARSE_ERROR
-                break
+                # If we get to the end of the config break
+                elif is_header.match(line):
+                    break
+                # If the starting character is a two character comment
+                # remove that and any newly revealed whitespace, then write
+                # it into the StringIO
+                elif line[:2] in ['//', '/*', '*/']:
+                    text_io.write(line[2:].lstrip() + '\n')
+                # If we have just * then we're in the middle of a C style
+                # comment, do like above
+                elif line[:1] == '*':
+                    text_io.write(line[1:].lstrip() + '\n')
+                else:
+                    raise GLSLParserException(
+                        "The config section is malformed."
+                        "Check file {0}".format(filepath))
             else:
-                assert(False)
+                raise GLSLParserException("No [end config] section found!")
 
-        if parse_state == PARSE_DONE:
-            pass
-        elif parse_state == PARSE_FIND_START:
-            self.result = TestResult()
-            self.result['result'] = 'fail'
-            self.result['errors'] = ["Config section of test file '{0}' is "
-                                     "missing".format(self.__filepath)]
-            self.result['errors'] += ["Failed to find initial line of config "
-                                      "section '// [config]'"]
-            self.result['note'] = \
-                "See the docstring in file '{0}'".format(__file__)
-            return
-        elif parse_state == PARSE_IN_CONFIG:
-            self.result = TestResult()
-            self.result['result'] = 'fail'
-            self.result['errors'] = ["Config section of test file '{0}' does "
-                                     "not terminate".format(self.__filepath)]
-            self.result['errors'] += ["Failed to find terminal line of config "
-                                      "section '// [end config]'"]
-            self.result['note'] = \
-                "See the docstring in file '{0}'".format(__file__)
-            return
-        elif parse_state == PARSE_ERROR:
-            self.result = TestResult()
-            self.result['result'] = 'fail'
-            self.result['errors'] = ["Config section of test file '{0}' is "
-                                     "ill formed, most likely due to "
-                                     "whitespace".format(self.__filepath)]
-            self.result['note'] = \
-                "See the docstring in file '{0}'".format(__file__)
-            return
-        else:
-            assert(False)
+            config = ConfigParser.SafeConfigParser(
+                    defaults={'require_extensions': '', 'check_link': 'false'})
 
-        config = ConfigParser.SafeConfigParser(cls.__config_defaults)
-        try:
+            # Verify that the config was valid
             text = text_io.getvalue()
             text_io.close()
             config.readfp(StringIO(text))
-        except ConfigParser.Error as e:
-            self.result = TestResult()
-            self.result['result'] = 'fail'
-            self.result['errors'] = ['Errors exist in config section of test '
-                                     'file']
-            self.result['errors'] += [e.message]
-            self.result['note'] = \
-                "See the docstring in file '{0}'".format(__file__)
-            return
-
-        self.__config = config
-
-    def __validate_config(self):
-        """Validate config.
-
-        Check that that all required options are present. If
-        validation fails, set ``self.result`` to failure.
-
-        Currently, this function does not validate the options'
-        values.
-
-        :return: None
-        """
-        cls = self.__class__
-
-        if self.__config is None:
-            return
-        for o in cls.__required_opts:
-            if not self.__config.has_option('config', o):
-                self.result = TestResult()
-                self.result['result'] = 'fail'
-                self.result['errors'] = ['Errors exist in config section of '
-                                         'test file']
-                self.result['errors'] += ["Option '{0}' is required".format(o)]
-                self.result['note'] = \
-                    "See the docstring in file '{0}'".format(__file__)
-                return
-
-    @property
-    def config(self):
-        if self.__config is None:
-            self.__get_config()
-            self.__validate_config()
-        return self.__config
-
-    @property
-    def command(self):
-        """Command line arguments for 'glslparsertest'.
 
-        The command line arguments are constructed by parsing the
-        config section of the test file. If any errors are present in
-        the config section, then ``self.result`` is set to failure and
-        this returns ``None``.
+            for opt in ['expect_result', 'glsl_version']:
+                if not config.has_option('config', opt):
+                    raise GLSLParserException("Missing required section {} "
+                                              "from config".format(opt))
 
-        :return: [str] or None
-        """
+            # Create the command and pass it into a PlainExecTest()
+            command = [path.join(testBinDir, 'glslparsertest'),
+                       filepath,
+                       config.get('config', 'expect_result'),
+                       config.get('config', 'glsl_version')]
+            if config.get('config', 'check_link').lower() == 'true':
+                command.append('--check-link')
+            command.extend(config.get('config', 'require_extensions').split())
 
-        if self.result is not None:
-            return None
+            super(GLSLParserTest, self).__init__(command)
 
-        assert(self.config is not None)
-        command = [path.join(testBinDir, 'glslparsertest'),
-                   self.__filepath,
-                   self.config.get('config', 'expect_result'),
-                   self.config.get('config', 'glsl_version')
-                   ]
-        if self.config.get('config', 'check_link').lower() == 'true':
-            command.append('--check-link')
-        command += self.config.get('config', 'require_extensions').split()
-        return command
 
-    @property
-    def env(self):
-        return dict()
+class GLSLParserException(Exception):
+    pass
diff --git a/framework/tests/glsl_parser_test_tests.py b/framework/tests/glsl_parser_test_tests.py
index 203c50f..f1cea00 100644
--- a/framework/tests/glsl_parser_test_tests.py
+++ b/framework/tests/glsl_parser_test_tests.py
@@ -33,13 +33,72 @@ def _check_config(content):
         return glsl.GLSLParserTest(tfile), tfile
 
 
-def test_glslparser_initializer():
-    """ Test that GLSLParserTest initializes """
-    glsl.GLSLParserTest('spec/glsl-es-1.00/execution/sanity.shader_test')
+def test_no_config_start():
+    """ GLSLParserTest requires [config] """
+    content = ('// expect_result: pass\n'
+               '// glsl_version: 1.00\n'
+               '// [end config]\n')
+    with utils.with_tempfile(content) as tfile:
+        with nt.assert_raises(glsl.GLSLParserException) as exc:
+            glsl.GLSLParserTest(tfile)
+            nt.assert_equal(
+                exc.exception, 'No [config] section found!',
+                msg="No config section found, no exception raised")
+
+
+def test_find_config_start():
+    """ GLSLParserTest finds [config] """
+    content = ('// [config]\n'
+               '// glsl_version: 1.00\n'
+               '//\n')
+    with utils.with_tempfile(content) as tfile:
+        with nt.assert_raises(glsl.GLSLParserException) as exc:
+            glsl.GLSLParserTest(tfile)
+            nt.assert_not_equal(
+                exc.exception, 'No [config] section found!',
+                msg="Config section not parsed")
+
+
+def test_no_config_end():
+    """ GLSLParserTest requires [end config] """
+    with utils.with_tempfile('// [config]\n') as tfile:
+        with nt.assert_raises(glsl.GLSLParserException) as exc:
+            glsl.GLSLParserTest(tfile)
+            nt.assert_equal(
+                exc.exception, 'No [end config] section found!',
+                msg="config section not closed, no exception raised")
+
+
+def test_no_expect_result():
+    """ expect_result section is required """
+    content = ('// [config]\n'
+               '// glsl_version: 1.00\n'
+               '//\n')
+    with utils.with_tempfile(content) as tfile:
+        with nt.assert_raises(glsl.GLSLParserException) as exc:
+            glsl.GLSLParserTest(tfile)
+            nt.assert_equal(
+                exc.exception,
+                'Missing required section expect_result from config',
+                msg="config section not closed, no exception raised")
+
+
+def test_no_glsl_version():
+    """ glsl_version section is required """
+    content = ('//\n'
+               '// expect_result: pass\n'
+               '// [end config]\n')
+    with utils.with_tempfile(content) as tfile:
+        with nt.assert_raises(glsl.GLSLParserException) as exc:
+            glsl.GLSLParserTest(tfile)
+            nt.assert_equal(
+                exc.exception,
+                'Missing required section glsl_version from config',
+                msg="config section not closed, no exception raised")
 
 
 def test_cpp_comments():
-    """ Test C++ style comments """
+    """ Parses C++ style comments """
     content = ('// [config]\n'
                '// expect_result: pass\n'
                '// glsl_version: 1.00\n'
@@ -52,7 +111,7 @@ def test_cpp_comments():
 
 
 def test_c_comments():
-    """ Test C style comments """
+    """ Parses C style comments """
     content = ('/*\n'
                ' * [config]\n'
                ' * expect_result: pass\n'
@@ -66,44 +125,6 @@ def test_c_comments():
                     msg="C style comments were not properly parsed")
 
 
- at nt.raises(Exception)
-def test_no_config_end():
-    """ end_config section is required """
-    content = ('// [config]\n'
-               '// expect_result: pass\n'
-               '// glsl_version: 1.00\n'
-               '//\n')
-    _, _ = _check_config(content)
-
-
- at nt.raises(Exception)
-def test_no_expecte_result():
-    """ expect_result section is required """
-    content = ('// [config]\n'
-               '// glsl_version: 1.00\n'
-               '//\n')
-    _, _ = _check_config(content)
-
-
- at nt.raises(Exception)
-def test_no_required_glsl_version():
-    """ glsl_version section is required """
-    content = ('//\n'
-               '// expect_result: pass\n'
-               '// [end config]\n')
-    _, _ = _check_config(content)
-
-
- at nt.raises(Exception)
-def test_no_config_start():
-    """ Config section is required """
-    content = ('//\n'
-               '// expect_result: pass\n'
-               '// glsl_version: 1.00\n'
-               '// [end config]\n')
-    _, _ = _check_config(content)
-
-
 def test_blank_in_config():
     """ C++ style comments can have uncommented newlines """
     content = ('// [config]\n'
@@ -118,3 +139,8 @@ def test_blank_in_config():
                                    name, 'pass', '1.00'],
                     msg="A newline in a C++ style comment was not properly "
                         "parsed.")
+
+
+def test_glslparser_initializer():
+    """ GLSLParserTest initializes """
+    glsl.GLSLParserTest('tests/spec/glsl-es-1.00/compiler/version-macro.frag')
-- 
1.9.1



More information about the Piglit mailing list