[Piglit] [PATCH 06/12] glsl_parser_test.py: Replace GLSLParserTest class with function
Ilia Mirkin
imirkin at alum.mit.edu
Sat Mar 1 13:12:52 PST 2014
Perhaps I missed it somewhere in the series, but where do you take
care of the tests/gpu.py bit which does:
profile.filter_tests(lambda p, t: not isinstance(t, GLSLParserTest))
On Tue, Feb 11, 2014 at 9:11 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> This is a rough 'port' of the class to a function. It deviates only to
> make idiomatic changes and to remove references to self, with a one
> notable exception: the class always returned a result when something went
> wrong, this function raises an exception. Some of these cases can
> probably be better handled, but returning a result seems like the wrong
> decision in every case; since getting back 'failed' is not likely to
> lead a developer to say "bad python code, bad!", but to go digging
> through the test or driver. Raising an exception will cause piglit to
> record the exception in the json, much more useful for realizing
> something when wrong at the python level.
>
> Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
> ---
> framework/glsl_parser_test.py | 351 ++++++++++-------------------------------
> framework/tests/dmesg_tests.py | 5 +-
> tests/all.py | 2 +-
> 3 files changed, 81 insertions(+), 277 deletions(-)
>
> diff --git a/framework/glsl_parser_test.py b/framework/glsl_parser_test.py
> index 35efb02..0c3c900 100644
> --- a/framework/glsl_parser_test.py
> +++ b/framework/glsl_parser_test.py
> @@ -27,13 +27,13 @@ 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
>
>
> def add_glsl_parser_test(group, filepath, test_name):
> """Add an instance of GLSLParserTest to the given group."""
> - group[test_name] = GLSLParserTest(filepath)
> + group[test_name] = glsl_parser_test(filepath)
>
>
> def import_glsl_parser_tests(group, basepath, subdirectories):
> @@ -68,164 +68,43 @@ def import_glsl_parser_tests(group, basepath, subdirectories):
> add_glsl_parser_test(group, filepath, testname)
>
>
> -class GLSLParserTest(PlainExecTest):
> - """Test for the GLSL parser (and more) on a GLSL source file.
> +def glsl_parser_test(filepath):
> + """ Read the option in a glsl parser test and return a PlainExecTest
>
> - 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. The reason to use a
> + function wrapper like this rather than a subclass is that there are no
> + method overrides, just helpers.
>
> - 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__
> -
> - # Text of config section.
> - text_io = StringIO()
> -
> - # Parsing state.
> - PARSE_FIND_START = 0
> - PARSE_IN_CONFIG = 1
> - PARSE_DONE = 2
> - PARSE_ERROR = 3
> - parse_state = PARSE_FIND_START
> -
> - # 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.
> -
> - 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
> + # Text of config section.
> + text_io = StringIO()
> +
> + # Parsing state.
> + PARSE_FIND_START = 0
> + PARSE_IN_CONFIG = 1
> + PARSE_DONE = 2
> + PARSE_ERROR = 3
> + parse_state = PARSE_FIND_START
> +
> + # 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 f:
> for line in f:
> if parse_state == PARSE_FIND_START:
> m = start.match(line)
> - if m is not None:
> + if m:
> parse_state = PARSE_IN_CONFIG
> text_io.write(m.group('content'))
> indent = '.' * len(m.group('indent'))
> @@ -235,135 +114,63 @@ class GLSLParserTest(PlainExecTest):
> 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:
> + if start.match(line):
> parse_state = PARSE_ERROR
> break
> - if end.match(line) is not None:
> + if end.match(line):
> parse_state = PARSE_DONE
> break
> m = internal.match(line)
> - if m is not None:
> + if m:
> text_io.write(m.group('content'))
> continue
> m = empty.match(line)
> - if m is not None:
> + if m:
> text_io.write('\n')
> continue
> parse_state = PARSE_ERROR
> break
> - else:
> - assert(False)
> -
> - 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(cls.__config_defaults)
> - try:
> - 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``.
> -
> - :return: [str] or None
> - """
> -
> - if self.result is not None:
> - return None
> -
> - 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()
> + if parse_state == PARSE_DONE:
> + pass
> + elif parse_state == PARSE_FIND_START:
> + raise GLSLParserException("Failed to find start of config section")
> + elif parse_state == PARSE_IN_CONFIG:
> + raise GLSLParserException("Failed to find end of config section")
> + elif parse_state == PARSE_ERROR:
> + raise GLSLParserException("Failed to parse config section. This is "
> + "most likely due to whitespace")
> + else:
> + raise GLSLParserException("How did you hit this? This shouldnt happen")
> +
> + config = ConfigParser.SafeConfigParser(defaults={'require_extensions': '',
> + 'check_link': 'false'})
> +
> + # Verify that the config was valid
> + try:
> + text = text_io.getvalue()
> + text_io.close()
> + config.readfp(StringIO(text))
> + except ConfigParser.Error:
> + raise GLSLParserException("Failed to parse config section. This is "
> + "most likely due to whitespace")
> +
> + for opt in ['expect_result', 'glsl_version']:
> + if not config.has_option('config', opt):
> + raise GLSLParserException("Missing required section {} from "
> + "config".format(opt))
> +
> + # 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 += config.get('config', 'require_extensions').split()
> +
> + return PlainExecTest(command)
> +
> +
> +class GLSLParserException(Exception):
> + pass
> diff --git a/framework/tests/dmesg_tests.py b/framework/tests/dmesg_tests.py
> index b96c067..e90729f 100644
> --- a/framework/tests/dmesg_tests.py
> +++ b/framework/tests/dmesg_tests.py
> @@ -29,7 +29,6 @@ from framework.dmesg import DummyDmesg, LinuxDmesg, get_dmesg, DmesgError
> from framework.core import TestResult, PiglitJSONEncoder, Environment
> from framework.exectest import PlainExecTest
> from framework.gleantest import GleanTest
> -from framework.glsl_parser_test import GLSLParserTest
>
>
> def _get_dmesg():
> @@ -219,9 +218,7 @@ def test_testclasses_dmesg():
> env = Environment()
>
> lists = [(PlainExecTest, ['attribs', '-auto', '-fbo'], 'PlainExecTest'),
> - (GleanTest, 'basic', "GleanTest"),
> - (GLSLParserTest, 'tests/glslparsertest/shaders/main1.vert',
> - 'GLSLParserTest')]
> + (GleanTest, 'basic', "GleanTest")]
>
> for tclass, tfile, desc in lists:
> yieldable = check_classes_dmesg
> diff --git a/tests/all.py b/tests/all.py
> index ba34543..0bd5dd4 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -12,7 +12,7 @@ import shlex
> from framework.core import Group, TestProfile
> from framework.exectest import PlainExecTest
> from framework.gleantest import GleanTest
> -from framework.glsl_parser_test import GLSLParserTest, add_glsl_parser_test, import_glsl_parser_tests
> +from framework.glsl_parser_test import add_glsl_parser_test, import_glsl_parser_tests
> from framework.shader_test import add_shader_test_dir
>
> # Path to tests dir, correct even when not running from the top directory.
> --
> 1.8.5.4
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
More information about the Piglit
mailing list