[Piglit] [PATCH 06/12] glsl_parser_test.py: Replace GLSLParserTest class with function
Dylan Baker
baker.dylan.c at gmail.com
Tue Feb 11 18:11:11 PST 2014
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
More information about the Piglit
mailing list