[Piglit] Generate testlists at buildtime

Dylan Baker baker.dylan.c at gmail.com
Wed Sep 11 19:17:20 PDT 2013


On Wednesday 11 September 2013 10:36:41 Paul Berry wrote:
> On 9 September 2013 16:46, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> > On Monday 09 September 2013 16:34:08 you wrote:
> > > This series changes the way testlists are generated. This allows us to
> > > generate a TestProfile object only once, at build time. This is
> > > accomplished by using python's pickle module, which stores a python
> > > object representation as a plain text file.
> > > 
> > > The first 9 patches touch all of the testfiles the majority of these
> > > patches simply replace ``from <foo> import *'' with explicit imports.
> > > This is the recomended way of doing things from the python devs. r600 is
> > > removed, since it *is* quick-driver.tests, as is external-glparser.tests
> > > since it is deprecated.
> > > 
> > > The last patch does all of the heavy lifting, moving all of the tests
> > > files to a new folder, and adding the CMake magic to generate the
> > > testslists
> > > 
> > > There are a couple of reasons for doing this:
> > > 1) Since piglit-run is no longer responsible for producing
> > > 
> > >    the TestProfile objects, it is much simpler to change the format
> > >    they are stored it. This is to our advantage as we try to create a
> > >    python3 branch of piglit since ``execfile'' is deprecated in python3
> > > 
> > > 2) This takes a few seconds off the start of piglit-run, since all of
> > > 
> > >    the heavy liftiner of running the python *tests files are removed,
> > >    instead being done durring build time
> > 
> > This is also available at my github:
> > https://github.com/crymson/piglit.git generate-tests
> 
> I see one patch on the github branch that I don't see on the mailing list:
> 
> ae04a6b all.tests: Replace import * with only the necissary functions
> 
> In that patch, s/necissary/necessary/.
> 
> General comment on all the patches that replace "import *" with specific
> imports: rather than the variations on "this is bad" in the commit
> messages, I would just quote the relevant text from PEP 8, which says:
> 
>     "Wildcard imports (from <module> import *) should be avoided, as they
> make it unclear which names are present in the namespace, confusing both
> readers and many automated tools."
> 
> This will be helpful in case someone goes digging through git history later
> trying to understand why you did what you did.
> 
> With that change, all patches but the last are:
> 
> Reviewed-by: Paul Berry <stereotype441 at gmail.com>
> 
> Regarding the last patch, it looks like the mailing list ate it because all
> the renamed files made it too big.  I'm going to re-generate the patch
> using "--find-renames" and re-send it; then I'll reply to it with my
> comments.

I have made the appropriate changes to patches 1-9 and will send them 
upstream.

Thanks for the review.
-------------- 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/20130911/19c25fc7/attachment.pgp>


More information about the Piglit mailing list