[Piglit] replace glslparsertest standalone functionality

[Dylan Baker]

On Thursday, February 27, 2014 15:32:06 you wrote:
> On Thu, Feb 27, 2014 at 2:43 PM, Dylan Baker <baker.dylan.c at gmail.com> 
wrote:
> > This small series replaces glsl_parser_test.py's standalone
> > functionality with a small utility script for running a single
> > glslparsertest.
> > 
> > The output of this file is less than ideal, however, that is becasue
> > core.Test does some very silly things with the output of a test. That is
> > a patch for another day.
> > 
> > I'm not set on where to put the utility files, Fabian suggested that it go
> > in tests/ somewhere, I like the idea of keeping little utilites
> > together (I'd like to move generate-glean-test.py,
> > piglit-print-commands.py, and piglit-merge-results.py into the utils
> > folder as well), and putting them in tests feels odd to me, since that
> > is a folder for tests, not little utilities.
> 
> These ideas may be apparent to everyone, but thought I'd mention them
> anyways:
> 
> Piglit is composed of 2 largely unrelated bits -- a bunch of tests for
> various opengl/etc functionality, and a framework to run tests (which,
> perhaps not entirely coincidentally, are those very same
> opengl/opencl/etc tests). There is a funny overlap region, which sets
> up mechanisms to run individual tests which are fancier than just
> "execute this command". But the fact that opengl tests are executed is
> dictacted by passing in tests/all.py as the "seed the tests" file. I
> instead could pass in /path/to/other_file.py which would add tests to
> the framework that are of an entirely different nature, and as long as
> those tests abide by the API for returning results, the framework
> would be none-the-wiser.

We have had a lot of discussions about the 2 parts actually. There are some 
real advantages to splitting the two parts (and obviously some drawbacks as 
well). On the all.py test file, I'd like to move to a purely descriptive format 
for tests (I like XML, but I know that many people don't) 

> 
> IMHO GLSLParserTest does not belong in framework at all, and should
> instead be in tests (perhaps tests/util or tests/bin). Similarly for
> ShaderTest -- the reason it exists at all seems to be to determine
> whether a particular shader test is GL/GLES2/GLES3 and execute the
> appropriate binary. And it's advantageous to have only one of those
> due to the regex compiling... but it's not exactly a surprise to the
> author of a shader_test which API it specifies (be it a human being or
> a python script), one could certainly imagine using diff extensions
> for the diff ones, much as diff binaries are used.

I have some patches out to clean up glsl_parser_test.py and shader_test.py (If 
you're bored and want to have a look at them :) ). While I was working on that 
I began to wonder if the "right" solution is to just move all of the parsing 
out of python into the c binary (a single shader_test binary and a single 
glslparsertest binary), that can figure out whether the test is OpenGL or 
OpenGLES version x and react appropriately. At that point a glslparsertest or 
shadertest would just be a PlainExecTest with default options. It moves the 
complexity around, but it seems ideal to be able to say "this is a 
shader_test, I'll pass it to ShaderTest and it'll work"

> 
> Looking through glsl_parser_test.py --
> import_glsl_parser_tests/add_glsl_parser_test seems like it'd belong
> in tests/all.py. Most of the rest is concerned with parsing the test
> file to provide appropriate options to the glslparsertest command.
> This seems like it'd belong in tests, and all.py would just add
> executors for the python wrapper script.
> 
> All of this would take a lot of refactoring/changing, so feel free to
> ignore. (And also, I'm just a newcomer with crazy ideas, so feel free
> to ignore for that reason as well :) ) Just providing my 2c...
> 
> Cheers,
> 
>   -ilia
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140227/cd858c65/attachment.pgp>