[Piglit] [PATCH v2 17/26] framework/profile: Don't alter the TestProfile before running
Dylan Baker
dylan at pnwbakers.com
Thu Oct 27 21:31:45 UTC 2016
This patch changes the way that the tests from TestProfile are
processed. Rather than having a process_test_list method that modifies
TestProfile.test_list, a new itertests() method creates an iterator that
yields tests that are not excluded by filters.
This saves a bit of code, increases the memory usage, and reduces
surprise.
It would be nice if there wasn't a need to create a concrete test list,
but there wouldn't be any way to get the number of tests otherwise.
Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
---
framework/profile.py | 104 +++++++++-------------------
framework/programs/print_commands.py | 3 +-
unittests/framework/test_profile.py | 6 +--
3 files changed, 38 insertions(+), 75 deletions(-)
diff --git a/framework/profile.py b/framework/profile.py
index 7bf3015..048fd64 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -186,35 +186,6 @@ class TestDict(collections.MutableMapping):
yield
self.__allow_reassignment -= 1
- def filter(self, callable):
- """Filter tests out of the testdict before running.
-
- This method destructively filters results out of the test test
- dictionary list using the callable provided.
-
- Arguments:
- callable -- a callable object that returns truthy if the item remains,
- falsy if it should be removed
-
- """
- for k, v in list(six.iteritems(self)):
- if not callable(k, v):
- del self[k]
-
- def reorder(self, order):
- """Reorder the TestDict to match the order of the provided list."""
- new = collections.OrderedDict()
- try:
- for k in order:
- new[k] = self.__container[k]
- except KeyError:
- # If there is a name in order that isn't available in self there
- # will be a KeyError, this is expected. In this case fail
- # gracefully and report the error to the user.
- raise exceptions.PiglitFatalError(
- 'Cannot reorder test: "{}", it is not in the profile.'.format(k))
- self.__container = new
-
class TestProfile(object):
""" Class that holds a list of tests for execution
@@ -277,28 +248,6 @@ class TestProfile(object):
"""
self._monitoring = Monitoring(monitored)
- def prepare_test_list(self):
- """ Prepare tests for running
-
- Flattens the nested group hierarchy into a flat dictionary using '/'
- delimited groups by calling self.flatten_group_hierarchy(), then
- runs it's own filters plus the filters in the self.filters name
-
- """
- if self.forced_test_list:
- # Remove all tests not in the test list, then reorder the tests to
- # match the testlist. This still allows additional filters to be
- # run afterwards.
- self.test_list.filter(lambda n, _: n in self.forced_test_list)
- self.test_list.reorder(self.forced_test_list)
-
- # Filter out unwanted tests
- self.test_list.filter(lambda n, t: all(f(n, t) for f in self.filters))
-
- if not self.test_list:
- raise exceptions.PiglitFatalError(
- 'There are no tests scheduled to run. Aborting run.')
-
@contextlib.contextmanager
def group_manager(self, test_class, group, prefix=None, **default_args):
"""A context manager to make working with flat groups simple.
@@ -384,9 +333,8 @@ class TestProfile(object):
This method creates a copy with references to the original instance
(using copy.copy), except for the test_list attribute, which is copied
- using copy.deepcopy, which is necessary to ensure that
- prepare_test_list only affects the right instance. This allows profiles
- to be "subclassed" by other profiles, without modifying the original.
+ using copy.deepcopy. This allows profiles to be "subclassed" by other
+ profiles, without modifying the original.
"""
new = copy.copy(self)
new.test_list = copy.deepcopy(self.test_list)
@@ -394,6 +342,22 @@ class TestProfile(object):
new.filters = copy.copy(self.filters)
return new
+ def itertests(self):
+ """Iterate over tests while filtering.
+
+ This iterator is non-destructive.
+ """
+ if self.forced_test_list:
+ opts = collections.OrderedDict()
+ for n in self.forced_test_list:
+ 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
+
def load_test_profile(filename):
"""Load a python module and return it's profile attribute.
@@ -447,9 +411,11 @@ def run(profiles, logger, backend, concurrency):
"""
chunksize = 1
- for p in profiles:
- p.prepare_test_list()
- log = LogManager(logger, sum(len(p.test_list) for p in profiles))
+ # 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))
def test(name, test, profile, this_pool=None):
"""Function to call test.execute from map"""
@@ -459,31 +425,35 @@ def run(profiles, logger, backend, concurrency):
if profile.monitoring.abort_needed:
this_pool.terminate()
- def run_threads(pool, profile, filterby=None):
+ def run_threads(pool, profile, test_list, filterby=None):
""" Open a pool, close it, and join it """
- iterable = six.iteritems(profile.test_list)
if filterby:
- iterable = (x for x in iterable if filterby(x))
+ # 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),
- iterable, chunksize)
+ test_list, chunksize)
pool.close()
pool.join()
- def run_profile(profile):
+ def run_profile(profile, test_list):
"""Run an individual profile."""
if concurrency is ConcurrentMode.full:
- run_threads(multi, profile)
+ run_threads(multi, profile, test_list)
elif concurrency is ConcurrentMode.none:
- run_threads(single, profile)
+ run_threads(single, profile, test_list)
else:
assert concurrency is ConcurrentMode.some
# Filter and return only thread safe tests to the threaded pool
- run_threads(multi, profile, lambda x: x[1].run_concurrent)
+ run_threads(multi, profile, test_list,
+ lambda x: x[1].run_concurrent)
# Filter and return the non thread safe tests to the single
# pool
- run_threads(single, profile, lambda x: not x[1].run_concurrent)
+ run_threads(single, profile, test_list,
+ lambda x: not x[1].run_concurrent)
# Multiprocessing.dummy is a wrapper around Threading that provides a
# multiprocessing compatible API
@@ -494,7 +464,7 @@ def run(profiles, logger, backend, concurrency):
try:
for p in profiles:
- run_profile(p)
+ run_profile(*p)
finally:
log.get().summary()
diff --git a/framework/programs/print_commands.py b/framework/programs/print_commands.py
index 5811cd2..8accb2b 100644
--- a/framework/programs/print_commands.py
+++ b/framework/programs/print_commands.py
@@ -97,8 +97,7 @@ def main(input_):
piglit_dir = os.path.dirname(os.path.realpath(sys.argv[0]))
os.chdir(piglit_dir)
- profile_.prepare_test_list()
- for name, test in six.iteritems(profile_.test_list):
+ for name, test in profile_.itertests():
assert isinstance(test, Test)
print(args.format_string.format(
name=name,
diff --git a/unittests/framework/test_profile.py b/unittests/framework/test_profile.py
index c4e67b0..f8fef2b 100644
--- a/unittests/framework/test_profile.py
+++ b/unittests/framework/test_profile.py
@@ -212,12 +212,6 @@ class TestTestProfile(object):
assert fixture.test_list == new.test_list
assert fixture.test_list is not new.test_list
- def test_prepare_test_list(self, fixture):
- """The prepare_test_list method doesn't affect both."""
- new = fixture.copy()
- new.prepare_test_list()
- assert new.test_list != fixture.test_list
-
class TestTestDict(object):
"""Tests for the TestDict object."""
--
git-series 0.8.10
More information about the Piglit
mailing list