[Piglit] [PATCH 10/10] Generate .tests files at build time
Paul Berry
stereotype441 at gmail.com
Wed Sep 11 11:06:40 PDT 2013
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.
> ---
>
> (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".
>
>
>
> ##############################################################################
> 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.
> 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'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
.
> +
> +# 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?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130911/73c54845/attachment-0001.html>
More information about the Piglit
mailing list