On Thu, Feb 27, 2014 at 4:21 PM, Dylan Baker <[email protected]> wrote: > On Thursday, February 27, 2014 15:32:06 you wrote: >> On Thu, Feb 27, 2014 at 2:43 PM, Dylan Baker <[email protected]> > 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)
That sounds nice in theory. In practice there are things like generated tests/etc. I suppose you could create a format that encompasses all of the current custom logic... and then have to extend it any time you want more. I'm actually a big fan of having python files for that -- the majority of the time they do something very simple, but are able to do more complicated things. But usually I do restrict what they can do, e.g. no importing, and give them a few built-in functions like glob() and so on that help them do what they need to do. That way people don't get too creative. (e.g. see https://github.com/yext/icbm/blob/master/icbm/data.py#L607) > >> >> 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 That's no different than declaring shader_runner.py as being the thing that runs the test instead of shader_runner. (Which would be fine, IMO, although we'd have to check for slowdown -- I think that just using different extensions for the different ones would be quite easy and the most efficient.) > 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" The question I think is appropriate is whether a different set of tests using the test-running framework would make use of ShaderTest and GLSLParserTest. My take is that "no" :) So then they don't really belong in framework. -ilia _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
