[Piglit] [PATCH 10/10] Generate .tests files at build time
Dylan Baker
baker.dylan.c at gmail.com
Wed Sep 11 19:15:47 PDT 2013
On Wednesday 11 September 2013 11:06:40 you wrote:
> On 11 September 2013 10:41, Paul Berry <stereotype441 at gmail.com> wrote:
> > From: Dylan Baker <baker.dylan.c at gmail.com>
> >
> > This patch allows testlists to be generated at build time by cmake,
> > these generated testlist are python TestProfile objects that have been
> > pickled, a process that converts a python object to a textual
> > representation of itself. Since this is done using cPickle it is backed
> > by native code it is very fast.
> >
> > This is a big win in two ways: first it elimnates any need to parse
> > through the python test files at the start of every run, since there is
> > already a testprofile object which is loaded and unpickled, again using
> > native code, it's very fast.
> >
> > Second, it creates freedom to change the way the tests are generated,
> > since all piglit-run.py needs is a valid pickled TestProfile object.
> > The biggest usecase for this is to replace ``execfile'', since it has
> > been dropped from python 3.x.
> >
> > This moves all of the tests/*.tests files to
> > testlists/(gl|cl|external)/*.tests files.
>
> Can you comment more on why it was necessary to move all the *.tests
> files? This is going to cause merge conflicts for anyone who has
> outstanding branches that affect all.tests, and it's not clear to me that
> it's necessary at all.
>
> If it does prove to be mecessary, it would be nice to move the *.tests
> files in a separate patch from the patch that does the pickling idea. That
> way if someone winds up running into difficulties, and they bisect, they'll
> have a lot easier time untangling the problem.
Okay, sure. I'll explain it here and add it to an appropriate commit message
(Since you're right I should split this up).
I felt that having the .py generator files, and the pickled .tests files in the
same directory cluttered things. I also felt like it was appropriate to move
content meant to generate other content out of the working tree (like
generated tests). But I'm not at all set on the idea, if there is a reason not
to I'll change it.
>
> > ---
> >
> > (Paul Berry): Re-sending on Dylan's behalf, since the previous patch
> > appears to have been too big for the mailing list. This patch was
> > generated with "--find-renames", so it's much smaller.
> >
> > .gitignore | 1 +
> > CMakeLists.txt | 1 +
> > __init__.py | 0
> > piglit-run.py | 4 +-
> > testslist/CMakeLists.txt | 43
> >
> > +++++++++++++++++++++
> >
> > testslist/__init__.py | 0
> > tests/all_cl.tests => testslist/cl/all.py | 2 +-
> > .../external/es3conform.py | 0
> > tests/gtf.tests => testslist/external/gtf.py | 0
> > tests/igt.tests => testslist/external/igt.py | 0
> > .../external/oglconform.py | 0
> > testslist/generate_lists.py | 44
> >
> > ++++++++++++++++++++++
> >
> > testslist/gl/__init__.py | 0
> > tests/all.tests => testslist/gl/all.py | 19 +++++-----
> > .../gl/glean-fragProg1.py | 0
> > .../gl/glean-glsl1.py | 0
> > .../gl/glean-vertProg1.py | 0
> > tests/gpu.tests => testslist/gl/gpu.py | 3 +-
> > .../gl/quick-driver.py | 4 +-
> > tests/quick.tests => testslist/gl/quick.py | 4 +-
> > tests/r300.tests => testslist/gl/r300.py | 4 +-
> > tests/r500.tests => testslist/gl/r500.py | 4 +-
> > tests/sanity.tests => testslist/gl/sanity.py | 0
> > 23 files changed, 107 insertions(+), 26 deletions(-)
> > create mode 100644 __init__.py
> > create mode 100644 testslist/CMakeLists.txt
> > create mode 100644 testslist/__init__.py
> > rename tests/all_cl.tests => testslist/cl/all.py (98%)
> > rename tests/es3conform.tests => testslist/external/es3conform.py (100%)
> > rename tests/gtf.tests => testslist/external/gtf.py (100%)
> > rename tests/igt.tests => testslist/external/igt.py (100%)
> > rename tests/oglconform.tests => testslist/external/oglconform.py (100%)
> > create mode 100644 testslist/generate_lists.py
> > create mode 100644 testslist/gl/__init__.py
> > rename tests/all.tests => testslist/gl/all.py (99%)
> > rename tests/glean-fragProg1.tests => testslist/gl/glean-fragProg1.py
> >
> > (100%)
> >
> > rename tests/glean-glsl1.tests => testslist/gl/glean-glsl1.py (100%)
> > rename tests/glean-vertProg1.tests => testslist/gl/glean-vertProg1.py
> >
> > (100%)
> >
> > rename tests/gpu.tests => testslist/gl/gpu.py (94%)
> > rename tests/quick-driver.tests => testslist/gl/quick-driver.py (83%)
> > rename tests/quick.tests => testslist/gl/quick.py (71%)
> > rename tests/r300.tests => testslist/gl/r300.py (76%)
> > rename tests/r500.tests => testslist/gl/r500.py (77%)
> > rename tests/sanity.tests => testslist/gl/sanity.py (100%)
> >
> > diff --git a/.gitignore b/.gitignore
> > index 635a107..3f63fd9 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -3,6 +3,7 @@
> >
> > *.tar.bz2
> > *.zip
> > *~
> >
> > +*.tests
> >
> > /.ninja_log
> > /build.ninja
> >
> > diff --git a/CMakeLists.txt b/CMakeLists.txt
> > index a39d9df..4a79f1a 100644
> > --- a/CMakeLists.txt
> > +++ b/CMakeLists.txt
> > @@ -280,6 +280,7 @@ include(cmake/piglit_dispatch.cmake)
> >
> > include_directories(src)
> > add_subdirectory(cmake/target_api)
> > add_subdirectory(generated_tests)
> >
> > +add_subdirectory(testslist)
>
> The commit message calls this subdirectory "testlists", but the patch
> consistently uses "testslist". I prefer "testlists"--my brain is having
> trouble parsing "testslist".
>
Fixed in v2
> >
#########################################################################
> > #####>
> > diff --git a/testslist/CMakeLists.txt b/testslist/CMakeLists.txt
> > new file mode 100644
> > index 0000000..3669a05
> > --- /dev/null
> > +++ b/testslist/CMakeLists.txt
> > @@ -0,0 +1,43 @@
> > +# Create a custom command to generate pickled python test lists using the
> > +# generate_python.py file. This is based on
> > ``piglit_make_generated_tests``
> > +# from generated_tests/CMakeLists.txt
> > +
> > +function(pickle_object generate_script tests_file)
> > + add_custom_command(
> > + OUTPUT ${tests_file}
> > + COMMAND ${python} generate_lists.py ${generate_script}
> > ${CMAKE_SOURCE_DIR}/tests/${tests_file}
> > + DEPENDS ${generate_script}
> > + VERBATIM)
> > +endfunction(pickle_object generate_script tests_file)
> > +
> > +# Piglit Gl tests
> > +pickle_object(gl/all.py all.tests)
> > +pickle_object(gl/gpu.py gpu.tests)
> > +pickle_object(gl/quick-driver.py quick-driver.tests)
> > +pickle_object(gl/quick.py quick.tests)
> > +pickle_object(gl/sanity.py sanity.tests)
> > +pickle_object(gl/r500.py r500.tests)
> > +pickle_object(gl/r300.py r300.tests)
> > +
> > +# Piglit CL tests
> > +pickle_object(cl/all.py all_cl.tests)
> > +
> > +# External tests
> > +pickle_object(external/igt.py igt.tests)
> > +pickle_object(external/gtf.py gtf.tests)
> > +pickle_object(external/es3conform.py es3conform.tests)
> > +pickle_object(external/oglconform.py oglconform.tests)
> > +
> > +add_custom_target(generate-lists ALL
> > + DEPENDS all.tests
> > + gpu.tests
> > + quick.tests
> > + quick-driver.tests
> > + sanity.tests
> > + r300.tests
> > + r500.tests
> > + all_cl.tests
> > + igt.tests
> > + gtf.tests
> > + es3conform.tests
> > + oglconform.tests)
>
> Inconsistent indentation here.
also fixed in v2
>
> > diff --git a/testslist/generate_lists.py b/testslist/generate_lists.py
> > new file mode 100644
> > index 0000000..fb5f7f3
> > --- /dev/null
> > +++ b/testslist/generate_lists.py
> > @@ -0,0 +1,44 @@
> > +# Copyright (c) 2013 Intel Corporation
> > +#
> > +# Permission is hereby granted, free of charge, to any person obtaining a
> > +# copy of this software and associated documentation files (the
> > "Software"),
> > +# to deal in the Software without restriction, including without
> > limitation
> > +# the rights to use, copy, modify, merge, publish, distribute,
> > sublicense,
> > +# and/or sell copies of the Software, and to permit persons to whom the
> > +# Software is furnished to do so, subject to the following conditions:
> > +#
> > +# The above copyright notice and this permission notice (including the
> > next
> > +# paragraph) shall be included in all copies or substantial portions of
> > the
> > +# Software.
> > +#
> > +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> > OR
> > +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY,
> > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> > SHALL
> > +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > OTHER
> > +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > DEALINGS
> > +# IN THE SOFTWARE.
> > +
> > +import sys
> > +import argparse
> > +import cPickle
> > +
> > +# Allow execfiles to look in the root for imports
> > +sys.path.append("../")
> > +
> > +
> > +parser = argparse.ArgumentParser()
> > +parser.add_argument("execfile",
> > + help="The name of the python file to execute")
> > +parser.add_argument("output",
> > + help="The location to write the file to")
> > +arguments = parser.parse_args()
> > +
> > +# Make the profile object a global object, so it is returned after the
> > file is
> > +# executed
> > +global profile
> > +execfile(arguments.execfile)
>
> I thought I understood from the commit message that one of the advantages
> of this approach is that it allows us to stop using "execfile", but the new
> scheme uses "execfile" too. I'm confused. Why does the new scheme make
> the "execfile" situation better than the old scheme?'
I have failed to word my commit message well. I will try again.
>
> I'm also a little unclear on why the execfile issue is significant, since
> it sounds like execfile() can be pretty easily simulated in Python 3--see
> http://stackoverflow.com/questions/436198/what-is-an-alternative-to-execfile
> -in-python-3-0 .
I'm trying to find it, but i found discussion suggesting tht execfile() had been
removed from python because there are issues with it and the dev's want to
discourage it's use.
>
> > +
> > +# Write the pickled object out to a file
> > +with open(arguments.output, 'w+') as file:
> > + cPickle.dump(profile, file)
>
> Finally, I tried your branch and got a build failure:
>
> FAILED: cd /home/pberry/.platform/piglit-mesa/piglit/build/testslist &&
> python2 generate_lists.py gl/all.py /home/pberry/piglit/tests/all.tests
> python2: can't open file 'generate_lists.py': [Errno 2] No such file or
> directory
>
> Maybe because I do out-of-tree builds?
That is possible, I didn't test out of tree builds
I also discovered that I broke igt integration (and probably es3conform and
gft also). So, I'll rework some of these things, split and reword commits, and
send out a second version of patch 10 later.
-------------- 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/938800a1/attachment.pgp>
More information about the Piglit
mailing list