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 <[email protected]> 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 <[email protected]> > --- > 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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/piglit _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
