[Piglit] [RFC 03/10] framework: add Test.name attribute
Dylan Baker
dylan at pnwbakers.com
Thu Oct 19 00:52:31 UTC 2017
Quoting Nicolai Hähnle (2017-10-11 03:26:52)
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> So that we can just pass Test objects around instead of (name, test)
> pairs.
> ---
> framework/backends/abstract.py | 8 ++++----
> framework/profile.py | 26 +++++++++++++++-----------
> framework/programs/run.py | 2 +-
> framework/test/base.py | 11 ++++++-----
> tests/all.py | 2 +-
> tests/cpu.py | 4 ++--
> tests/glslparser.py | 2 +-
> tests/gpu.py | 4 ++--
> tests/quick.py | 6 +++---
> tests/shader.py | 2 +-
> tests/xts-render.py | 4 +++-
> 11 files changed, 39 insertions(+), 32 deletions(-)
>
> diff --git a/framework/backends/abstract.py b/framework/backends/abstract.py
> index 27dec7e6b..cee7d7bdf 100644
> --- a/framework/backends/abstract.py
> +++ b/framework/backends/abstract.py
> @@ -184,40 +184,40 @@ class FileBackend(Backend):
>
> @abc.abstractmethod
> def _write(self, f, name, data):
> """Method that writes a TestResult into a result file."""
>
> @abc.abstractproperty
> def _file_extension(self):
> """The file extension of the backend."""
>
> @contextlib.contextmanager
> - def write_test(self, name):
> + def write_test(self, test):
> """Write a test.
>
> When this context manager is opened it will first write a placeholder
> file with the status incomplete.
>
> When it is called to write the final result it will create a temporary
> file, write to that file, then move that file over the original,
> incomplete status file. This helps to make the operation atomic, as
> long as the filesystem continues running and the result was valid in
> the original file it will be valid at the end
>
> """
> def finish(val):
> tfile = file_ + '.tmp'
> with open(tfile, 'w') as f:
> - self._write(f, name, val)
> + self._write(f, test.name.lower(), val)
Lowering shouldn't be necessary here, since the name is stored lowered, right?
If it's not stored lowered let's do that, I think the cost of calling .lower
throughout the codebase is well above zero.
> self.__fsync(f)
> shutil.move(tfile, file_)
>
> file_ = os.path.join(self._dest, 'tests', '{}.{}'.format(
> next(self._counter), self._file_extension))
>
> with open(file_, 'w') as f:
> incomplete = TestResult(result=INCOMPLETE)
> - incomplete.root = name
> - self._write(f, name, incomplete)
> + incomplete.root = test.name
> + self._write(f, test.name.lower(), incomplete)
> self.__fsync(f)
>
> yield finish
> diff --git a/framework/profile.py b/framework/profile.py
> index a19b4d2d2..c95ab9341 100644
> --- a/framework/profile.py
> +++ b/framework/profile.py
> @@ -71,33 +71,33 @@ class RegexFilter(object):
> filters -- a list of regex compiled objects.
>
> Keyword Arguments:
> inverse -- Inverse the sense of the match.
> """
>
> def __init__(self, filters, inverse=False):
> self.filters = [re.compile(f, flags=re.IGNORECASE) for f in filters]
> self.inverse = inverse
>
> - def __call__(self, name, _): # pylint: disable=invalid-name
> + def __call__(self, test):
> # This needs to match the signature (name, test), since it doesn't need
> # the test instance use _.
>
> # If self.filters is empty then return True, we don't want to remove
> # any tests from the run.
> if not self.filters:
> return True
>
> if not self.inverse:
> - return any(r.search(name) for r in self.filters)
> + return any(r.search(test.name) for r in self.filters)
> else:
> - return not any(r.search(name) for r in self.filters)
> + return not any(r.search(test.name) for r in self.filters)
>
>
> class TestDict(collections.MutableMapping):
> """A special kind of dict for tests.
>
> This mapping lowers the names of keys by default, and enforces that keys be
> strings (not bytes) and that values are Test derived objects. It is also a
> wrapper around collections.OrderedDict.
>
> This class doesn't accept keyword arguments, this is intentional. This is
> @@ -130,20 +130,23 @@ class TestDict(collections.MutableMapping):
> # Values should either be more Tests
> if not isinstance(value, Test):
> raise exceptions.PiglitFatalError(
> "TestDict values must be a Test, but was a {}".format(
> type(value)))
>
> # This must be lowered before the following test, or the test can pass
> # in error if the key has capitals in it.
> key = key.lower()
>
> + # Store the name of the test in the Test object
> + value.name = key
> +
> # If there is already a test of that value in the tree it is an error
> if not self.__allow_reassignment and key in self.__container:
> if self.__container[key] != value:
> error = (
> 'Further, the two tests are not the same,\n'
> 'The original test has this command: "{0}"\n'
> 'The new test has this command: "{1}"'.format(
> ' '.join(self.__container[key].command),
> ' '.join(value.command))
> )
> @@ -316,22 +319,23 @@ class TestProfile(object):
> opts = collections.OrderedDict()
> for n in self.forced_test_list:
> if self.options['ignore_missing'] and n not in self.test_list:
> opts[n] = DummyTest(n, status.NOTRUN)
> else:
> opts[n] = self.test_list[n]
> else:
> opts = self.test_list # pylint: disable=redefined-variable-type
>
> for k, v in six.iteritems(opts):
> - if all(f(k, v) for f in self.filters):
> - yield k, v
> + assert v.name.lower() == k
same here?
> + if all(f(v) for f in self.filters):
> + yield v
>
>
> def load_test_profile(filename):
> """Load a python module and return it's profile attribute.
>
> All of the python test files provide a profile attribute which is a
> TestProfile instance. This loads that module and returns it or raises an
> error.
>
> This method doesn't care about file extensions as a way to be backwards
> @@ -388,56 +392,56 @@ def run(profiles, logger, backend, concurrency):
> # The logger needs to know how many tests are running. Because of filters
> # there's no way to do that without making a concrete list out of the
> # filters profiles.
> profiles = [(p, list(p.itertests())) for p in profiles]
> log = LogManager(logger, sum(len(l) for _, l in profiles))
>
> # check that after the filters are run there are actually tests to run.
> if not any(l for _, l in profiles):
> raise exceptions.PiglitUserError('no matching tests')
>
> - def test(name, test, profile, this_pool=None):
> + def test(test, profile, this_pool=None):
> """Function to call test.execute from map"""
> - with backend.write_test(name) as w:
> - test.execute(name, log.get(), profile.options)
> + with backend.write_test(test) as w:
> + test.execute(log.get(), profile.options)
> w(test.result)
> if profile.options['monitor'].abort_needed:
> this_pool.terminate()
>
> def run_threads(pool, profile, test_list, filterby=None):
> """ Open a pool, close it, and join it """
> if filterby:
> # Although filterby could be attached to TestProfile as a filter,
> # it would have to be removed when run_threads exits, requiring
> # more code, and adding side-effects
> test_list = (x for x in test_list if filterby(x))
>
> - pool.imap(lambda pair: test(pair[0], pair[1], profile, pool),
> + pool.imap(lambda t: test(t, profile, pool),
> test_list, chunksize)
>
> def run_profile(profile, test_list):
> """Run an individual profile."""
> profile.setup()
> if concurrency == "all":
> run_threads(multi, profile, test_list)
> elif concurrency == "none":
> run_threads(single, profile, test_list)
> else:
> assert concurrency == "some"
> # Filter and return only thread safe tests to the threaded pool
> run_threads(multi, profile, test_list,
> - lambda x: x[1].run_concurrent)
> + lambda x: x.run_concurrent)
>
> # Filter and return the non thread safe tests to the single
> # pool
> run_threads(single, profile, test_list,
> - lambda x: not x[1].run_concurrent)
> + lambda x: not x.run_concurrent)
> profile.teardown()
>
> # Multiprocessing.dummy is a wrapper around Threading that provides a
> # multiprocessing compatible API
> #
> # The default value of pool is the number of virtual processor cores
> single = multiprocessing.dummy.Pool(1)
> multi = multiprocessing.dummy.Pool()
>
> try:
> diff --git a/framework/programs/run.py b/framework/programs/run.py
> index 5fe841992..4acccf348 100644
> --- a/framework/programs/run.py
> +++ b/framework/programs/run.py
> @@ -428,21 +428,21 @@ def resume(input_):
> p.dmesg = dmesg.get_dmesg(results.options['dmesg'])
>
> if results.options['monitoring']:
> p.options['monitor'] = monitoring.Monitoring(
> results.options['monitoring'])
>
> if results.options['ignore_missing']:
> p.options['ignore_missing'] = results.options['ignore_missing']
>
> if exclude_tests:
> - p.filters.append(lambda n, _: n not in exclude_tests)
> + p.filters.append(lambda test: test.name.lower() not in exclude_tests)
and here
> if results.options['exclude_filter']:
> p.filters.append(
> profile.RegexFilter(results.options['exclude_filter'],
> inverse=True))
> if results.options['include_filter']:
> p.filters.append(
> profile.RegexFilter(results.options['include_filter']))
>
> if results.options['forced_test_list']:
> p.forced_test_list = results.options['forced_test_list']
> diff --git a/framework/test/base.py b/framework/test/base.py
> index a8f81f547..d7b9432f5 100644
> --- a/framework/test/base.py
> +++ b/framework/test/base.py
> @@ -165,46 +165,47 @@ class Test(object):
> from the command line, run() is a more basic method for running the test,
> and is called internally by execute(), but is can be useful outside of it.
>
> Arguments:
> command -- a value to be passed to subprocess.Popen
>
> Keyword Arguments:
> run_concurrent -- If True the test is thread safe. Default: False
>
> """
> - __slots__ = ['run_concurrent', 'env', 'result', 'cwd', '_command']
> + __slots__ = ['name', 'run_concurrent', 'env', 'result', 'cwd', '_command']
> timeout = None
>
> def __init__(self, command, run_concurrent=False):
> assert isinstance(command, list), command
>
> + self.name = ''
Maybe store this as None instead of an empty string?
> self.run_concurrent = run_concurrent
> self._command = copy.copy(command)
> self.env = {}
> self.result = TestResult()
> self.cwd = None
>
> - def execute(self, path, log, options):
> + def execute(self, log, options):
Yay, that path thing has annoyed me for a long time, but I've never bothered to
fix it.
> """ Run a test
>
> Run a test, but with features. This times the test, uses dmesg checking
> (if requested), and runs the logger.
>
> Arguments:
> path -- the name of the test
> log -- a log.Log instance
> options -- a dictionary containing dmesg and monitoring objects
> """
> - log.start(path)
> + log.start(self.name)
> # Run the test
> - self.result.root = path
> + self.result.root = self.name
> if OPTIONS.execute:
> try:
> self.result.time.start = time.time()
> options['dmesg'].update_dmesg()
> options['monitor'].update_monitoring()
> self.run()
> self.result.time.end = time.time()
> self.result = options['dmesg'].update_result(self.result)
> options['monitor'].check_monitoring()
> # This is a rare case where a bare exception is okay, since we're
> @@ -380,21 +381,21 @@ class Test(object):
>
> def __ne__(self, other):
> return not self == other
>
>
> class DummyTest(Test):
> def __init__(self, name, result):
> super(DummyTest, self).__init__([name])
> self.result.result = result
>
> - def execute(self, path, log, options):
> + def execute(self, log, options):
> pass
>
> def interpret_result(self):
> pass
>
>
> class WindowResizeMixin(object):
> """ Mixin class that deals with spurious window resizes
>
> On gnome (and possible other DE's) the window manager may decide to resize
> diff --git a/tests/all.py b/tests/all.py
> index fdb4a3e71..0c75de54e 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -5013,11 +5013,11 @@ with profile.test_list.group_manager(
> g(['arb_bindless_texture-conversions'], 'conversions')
> g(['arb_bindless_texture-errors'], 'errors')
> g(['arb_bindless_texture-handles'], 'handles')
> g(['arb_bindless_texture-illegal'], 'illegal')
> g(['arb_bindless_texture-legal'], 'legal')
> g(['arb_bindless_texture-limit'], 'limit')
> g(['arb_bindless_texture-uint64_attribs'], 'uint64_attribs')
> g(['arb_bindless_texture-uniform'], 'uniform')
>
> if platform.system() is 'Windows':
> - profile.filters.append(lambda p, _: not p.startswith('glx'))
> + profile.filters.append(lambda test: not test.name.lower().startswith('glx'))
> diff --git a/tests/cpu.py b/tests/cpu.py
> index 65d999062..f8347b36f 100644
> --- a/tests/cpu.py
> +++ b/tests/cpu.py
> @@ -14,18 +14,18 @@ from __future__ import (
> absolute_import, division, print_function, unicode_literals
> )
> from tests.quick import profile as _profile
> from framework.test import GLSLParserTest
>
> __all__ = ['profile']
>
> profile = _profile.copy() # pylint: disable=invalid-name
>
>
> -def filter_gpu(name, test):
> +def filter_gpu(test):
> """Remove all tests that are run on the GPU."""
> - if isinstance(test, GLSLParserTest) or name.startswith('asmparsertest'):
> + if isinstance(test, GLSLParserTest) or test.name.lower().startswith('asmparsertest'):
> return True
> return False
>
>
> profile.filters.append(filter_gpu)
> diff --git a/tests/glslparser.py b/tests/glslparser.py
> index 5d0facbc2..c74632d6a 100644
> --- a/tests/glslparser.py
> +++ b/tests/glslparser.py
> @@ -4,11 +4,11 @@ from __future__ import (
> absolute_import, division, print_function, unicode_literals
> )
>
> from framework.test import GLSLParserTest
> from tests.all import profile as _profile
>
> __all__ = ['profile']
>
> profile = _profile.copy() # pylint: disable=invalid-name
>
> -profile.filters.append(lambda _, t: isinstance(t, GLSLParserTest))
> +profile.filters.append(lambda t: isinstance(t, GLSLParserTest))
> diff --git a/tests/gpu.py b/tests/gpu.py
> index fce550c67..0c4de3dbb 100644
> --- a/tests/gpu.py
> +++ b/tests/gpu.py
> @@ -7,12 +7,12 @@ from __future__ import (
> )
>
> from tests.quick import profile as _profile
> from framework.test import GLSLParserTest
>
> __all__ = ['profile']
>
> profile = _profile.copy() # pylint: disable=invalid-name
>
> # Remove all parser tests, as they are compiler test
> -profile.filters.append(lambda p, t: not isinstance(t, GLSLParserTest))
> -profile.filters.append(lambda n, _: not n.startswith('asmparsertest'))
> +profile.filters.append(lambda t: not isinstance(t, GLSLParserTest))
> +profile.filters.append(lambda t: not t.name.lower().startswith('asmparsertest'))
> diff --git a/tests/quick.py b/tests/quick.py
> index c587357f6..f6d9225f7 100644
> --- a/tests/quick.py
> +++ b/tests/quick.py
> @@ -25,22 +25,22 @@ __all__ = ['profile']
> # pylint: disable=bad-continuation
>
>
> class FilterVsIn(object):
> """Filter out 80% of the Vertex Attrib 64 vs_in tests."""
>
> def __init__(self):
> self.random = random.Random()
> self.random.seed(42)
>
> - def __call__(self, name, _):
> - if 'vs_in' in name:
> + def __call__(self, test):
> + if 'vs_in' in test.name.lower():
> # 20%
> return self.random.random() <= .2
> return True
>
>
> profile = _profile.copy() # pylint: disable=invalid-name
>
> GleanTest.GLOBAL_PARAMS += ["--quick"]
>
> # Set the --quick flag on a few image_load_store_tests
> @@ -57,12 +57,12 @@ with profile.test_list.group_manager(
> 'shader-mem-barrier')
>
> # Set the --quick flag on the image_size test
> with profile.test_list.group_manager(
> PiglitGLTest,
> grouptools.join('spec', 'arb_shader_image_size')) as g:
> with profile.test_list.allow_reassignment:
> g(['arb_shader_image_size-builtin', '--quick'], 'builtin')
>
> # These take too long
> -profile.filters.append(lambda n, _: '-explosion' not in n)
> +profile.filters.append(lambda t: '-explosion' not in t.name.lower())
> profile.filters.append(FilterVsIn())
> diff --git a/tests/shader.py b/tests/shader.py
> index d283a577c..2518d990e 100644
> --- a/tests/shader.py
> +++ b/tests/shader.py
> @@ -4,11 +4,11 @@ from __future__ import (
> absolute_import, division, print_function, unicode_literals
> )
>
> from framework.test.shader_test import ShaderTest, MultiShaderTest
> from tests.all import profile as _profile
>
> __all__ = ['profile']
>
> profile = _profile.copy() # pylint: disable=invalid-name
>
> -profile.filters.append(lambda _, t: isinstance(t, (ShaderTest, MultiShaderTest)))
> +profile.filters.append(lambda t: isinstance(t, (ShaderTest, MultiShaderTest)))
> diff --git a/tests/xts-render.py b/tests/xts-render.py
> index d2cd843f2..db921577f 100644
> --- a/tests/xts-render.py
> +++ b/tests/xts-render.py
> @@ -23,20 +23,22 @@ from __future__ import (
> absolute_import, division, print_function, unicode_literals
> )
>
> from tests.xts import profile as _profile
>
> __all__ = ['profile']
>
> profile = _profile.copy() # pylint: disable=invalid-name
>
>
> -def xts_render_filter(path, test):
> +def xts_render_filter(test):
> + path = test.name.lower()
> +
> # Keep any tests that aren't from xts.
> if 'xts5' not in path:
> return True
>
> # All of Xlib9 is for rendering.
> return 'xlib9' in path
>
>
> profile.filters.append(xts_render_filter)
> --
> 2.11.0
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20171018/05ceb966/attachment-0001.sig>
More information about the Piglit
mailing list