Quoting Nicolai Hähnle (2017-10-11 03:26:52) > From: Nicolai Hähnle <nicolai.haeh...@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/piglit
signature.asc
Description: signature
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit