Refactored types to simplify pickling.

expect_tests/type_definitions.py

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import copy
 import inspect
 import os
 import re
 
 from collections import namedtuple
 
 # These have to do with deriving classes from namedtuple return values.
 # Pylint can't tell that namedtuple returns a new-style type() object.
 #
 # "no __init__ method" pylint: disable=W0232
@@ skipping 126 matching lines @@
 
   def __call__(self, *args, **kwargs):
     f = self.bind(args, kwargs)
     assert f.fully_bound
     return f.func(*f.args, **f.kwargs)
 
   def __repr__(self):
     return 'FuncCall(%r, *%r, **%r)' % (self.func, self.args, self.kwargs)
 
 
-_Test = namedtuple(
-    'Test', 'name func_call expect_dir expect_base ext covers breakpoints')
+class Test(object):
+  TEST_COVERS_MATCH = re.compile('.*/test/([^/]+)_test\.py$')
 
-TestInfo = namedtuple(
-  'TestInfo', 'name  expect_dir expect_base ext')
-
-
-class Test(_Test):
-  TEST_COVERS_MATCH = re.compile('.*/test/([^/]*)_test\.py$')
-
-  def __new__(cls, name, func_call, expect_dir=None, expect_base=None,
+  def __init__(self, name, func_call, expect_dir=None, expect_base=None,
               ext='json', covers=None, breakpoints=None, break_funcs=()):
     """Create a new test.
 
     @param name: The name of the test. Will be used as the default expect_base
 
     @param func_call: A FuncCall object
 
     @param expect_dir: The directory which holds the expectation file for this
                        Test.
     @param expect_base: The basename (without extension) of the expectation
                         file. Defaults to |name|.
     @param ext: The extension of the expectation file. Affects the serializer
                 used to write the expectations to disk. Valid values are
                 'json' and 'yaml' (Keys in SERIALIZERS).
     @param covers: A list of coverage file patterns to include for this Test.
                    By default, a Test covers the file in which its function
                    was defined, as well as the source file matching the test
                    according to TEST_COVERS_MATCH.
 
     @param breakpoints: A list of (path, lineno, func_name) tuples. These will
                         turn into breakpoints when the tests are run in 'debug'
                         mode. See |break_funcs| for an easier way to set this.
     @param break_funcs: A list of functions for which to set breakpoints.
     """
-    breakpoints = breakpoints or []
-    if not breakpoints or break_funcs:
-      for f in (break_funcs or (func_call.func,)):
-        if hasattr(f, 'im_func'):
-          f = f.im_func
-        breakpoints.append((f.func_code.co_filename,
-                            f.func_code.co_firstlineno,
-                            f.func_code.co_name))
-
+    self._func_call = func_call
+    self._name = name
     if expect_dir:
       expect_dir = expect_dir.rstrip('/')
-    return super(Test, cls).__new__(cls, name, func_call, expect_dir,
-                                    expect_base, ext, covers, breakpoints)
+    self._expect_dir = expect_dir
+    self._expect_base = expect_base
+    self._ext = ext
+    self._covers = covers
+    breakpoints = breakpoints or []
+    self._breakpoints = breakpoints

[dnj, 2014/09/17 20:10:52]

self._breakpoints = copy.copy(breakpoints or [])
...
build onto 'self._breakpoints'?

 
+    if self._func_call:
+      if not breakpoints or break_funcs:
+        breakpoints = copy.copy(breakpoints)

[dnj, 2014/09/17 20:10:52]

You create a 'breakpoints' copy and build onto it, but never retain it anywhere.
Is this supposed to be in 'self._breakpoints'?

Since this isn't a terribly difficult calculation, consider performing it in the
'breakpoints' property function, just b/c it's that much less to retain/pickle.

[pgervais, 2014/09/17 21:32:51]

Done.

+        for f in (break_funcs or (func_call.func,)):
+          if hasattr(f, 'im_func'):
+            f = f.im_func
+          breakpoints.append((f.func_code.co_filename,
+                              f.func_code.co_firstlineno,
+                              f.func_code.co_name))
 
-  @staticmethod
-  def covers_obj(obj):
+  @property
+  def name(self):
+    return self._name
+
+  @property
+  def func_call(self):
+    return self._func_call
+
+  @property
+  def expect_dir(self):
+    return self._expect_dir
+
+  @property
+  def expect_base(self):
+    return self._expect_base
+
+  @property
+  def ext(self):
+    return self._ext
+
+  @property
+  def covers(self):
+    return self._covers
+
+  @property
+  def breakpoints(self):
+    return self._breakpoints
+
+  def run(self, context=None):
+    return self.func_call(context=context)
+
+  def process(self, func=lambda test: test.run()):
+    """Applies |func| to the test, and yields (self.get_info(), func(self)).
+
+    For duck-typing compatibility with MultiTest.
+
+    Bind(name='context') if used by your test function, is bound to None.
+
+    Used internally by expect_tests, you're not expected to call this yourself.
+    """
+    yield self.get_info(), func(self.bind(context=None))
+
+  def bind(self, *args, **kwargs):

[dnj, 2014/09/17 20:10:52]

Consider implementing '__copy__' or a 'copy()' function. Then this can be:
t = self.copy()
t._func_call = self._func_call.bind(*args, **kwargs)
return t

Can also use this with 'get_info'.

[pgervais, 2014/09/17 21:06:14]

I don't get the point. I think copy will be something like:

def copy(self):
   return Test(self._name,
        self._func_call,
        expect_dir=self.expect_dir,
        expect_base=self.expect_base,
        ext=self._ext,
        covers=self._covers,
        breakpoints=self._breakpoints)

So what's the gain?

+    return Test(self._name,
+                self._func_call.bind(*args, **kwargs),
+                expect_dir=self.expect_dir,
+                expect_base=self.expect_base,
+                ext=self._ext,
+                covers=self._covers,
+                breakpoints=self._breakpoints)
+
+  def restrict(self, tests):
+    assert tests[0] is self
+    return self
+
+  def get_info(self):
+    """Strips test instance of information required for running test.
+
+    Returns a TestInfo instance.
+    """
+    return Test(self.name,
+                None,
+                expect_dir=self._expect_dir,
+                expect_base=self._expect_base,
+                ext=self._ext,
+                covers=self._covers,
+                breakpoints=self._breakpoints)
+
+  def coverage_includes(self):
+    if self._covers is not None:
+      return self._covers
+    return self.covers_obj(self._func_call.func)
+
+  @classmethod
+  def covers_obj(cls, obj):
     test_file = inspect.getabsfile(obj)
     covers = [test_file]
-    match = Test.TEST_COVERS_MATCH.match(test_file)
+    match = cls.TEST_COVERS_MATCH.match(test_file)
     if match:
       covers.append(os.path.join(
           os.path.dirname(os.path.dirname(test_file)),
           match.group(1) + '.py'
       ))
     return covers
 
+  def expect_path(self, ext=None):
+    expect_dir = self.expect_dir
+    if expect_dir is None:
+      expect_dir = self.expect_dir_obj(self._func_call.func)
+    name = self._expect_base or self.name
+    name = ''.join('_' if c in '<>:"\\/|?*\0' else c for c in name)
+    return os.path.join(expect_dir, name + ('.%s' % (ext or self.ext)))

[dnj, 2014/09/17 20:10:52]

(ext or self.ext,) if following explicit tupling convention.

[pgervais, 2014/09/17 21:32:50]

Done.

+
   @staticmethod
   def expect_dir_obj(obj):
     test_file = inspect.getabsfile(obj)
     return os.path.splitext(test_file)[0] + '.expected'
 
-  def coverage_includes(self):
-    if self.covers is not None:
-      return self.covers
-    return self.covers_obj(self.func_call.func)
 
-  def expect_path(self, ext=None):
-    expect_dir = self.expect_dir
-    if expect_dir is None:
-      expect_dir = self.expect_dir_obj(self.func_call.func)
-    name = self.expect_base or self.name
-    name = ''.join('_' if c in '<>:"\\/|?*\0' else c for c in name)
-    return os.path.join(expect_dir, name + ('.%s' % (ext or self.ext)))
-
-  def run(self, context=None):
-    return self.func_call(context=context)
-
-  def process(self, func=lambda test: test.run()):
-    """Applies |func| to the test, and yields (self, func(self)).
-
-    For duck-typing compatibility with MultiTest.
-
-    Bind(name='context') if used by your test function, is bound to None.
-
-    Used interally by expect_tests, you're not expected to call this yourself.
-    """
-    yield self, func(self.bind(context=None))
-
-  def bind(self, *args, **kwargs):
-    return self._replace(func_call=self.func_call.bind(*args, **kwargs))
-
-  def restrict(self, tests):
-    assert tests[0] is self
-    return self
-
-  def get_info(self):
-    """Strips test instance of hard-to-pickle stuff
-
-    Returns a TestInfo instance.
-    """
-    return TestInfo(self.name, self.expect_dir, self.expect_base, self.ext)
-
-
-_MultiTest = namedtuple(
-    'MultiTest', 'name make_ctx_call destroy_ctx_call tests atomic')
-
-MultiTestInfo = namedtuple('MultiTestInfo', 'name tests atomic')
-
-
-class MultiTest(_MultiTest):
+class MultiTest(object):
   """A wrapper around one or more Test instances.
 
   Allows the entire group to have common pre- and post- actions and an optional
   shared context between the Test methods (represented by Bind(name='context')).
 
   Args:
     name - The name of the MultiTest. Each Test's name should be prefixed with
         this name, though this is not enforced.
     make_ctx_call - A FuncCall which will be called once before any test in this
         MultiTest runs. The return value of this FuncCall will become bound
         to the name 'context' for both the |destroy_ctx_call| as well as every
         test in |tests|.
     destroy_ctx_call - A FuncCall which will be called once after all tests in
         this MultiTest runs. The context object produced by |make_ctx_call| is
         bound to the name 'context'.
     tests - A list of Test instances. The context object produced by
         |make_ctx_call| is bound to the name 'context'.
     atomic - A boolean which indicates that this MultiTest must be executed
         either all at once, or not at all (i.e., subtests may not be filtered).
   """
 
-  def restrict(self, tests):
-    """A helper method to re-cast the MultiTest with fewer subtests.
+  def __init__(self, name, make_ctx_call, destroy_ctx_call, tests, atomic):
+    self._name = name
+    self._make_ctx_call = make_ctx_call
+    self._destroy_ctx_call = destroy_ctx_call
+    self._tests = tests
+    self._atomic = atomic
 
-    All fields will be identical except for tests. If this MultiTest is atomic,
-    then this method returns |self|.
+  @property
+  def name(self):
+    return self._name
 
-    Used interally by expect_tests, you're not expected to call this yourself.
-    """
-    if self.atomic:
-      return self
-    assert all(t in self.tests for t in tests)
-    return self._replace(tests=tests)
+  @property
+  def make_ctx_call(self):
+    return self._make_ctx_call
+
+  @property
+  def destroy_ctx_call(self):
+    return self._destroy_ctx_call
+
+  @property
+  def tests(self):
+    return self._tests
+
+  @property
+  def atomic(self):
+    return self._atomic
 
   def process(self, func=lambda test: test.run()):
     """Applies |func| to each sub-test, with properly bound context.
 
     make_ctx_call will be called before any test, and its return value becomes
     bound to the name 'context'. All sub-tests will be bound with this value
     as well as destroy_ctx_call, which will be invoked after all tests have
     been yielded.
 
     Optionally, you may specify a different function to apply to each test
     (by default it is `lambda test: test.run()`). The context will be bound
     to the test before your function recieves it.
 
     Used interally by expect_tests, you're not expected to call this yourself.
     """
     # TODO(iannucci): pass list of test names?
     ctx_object = self.make_ctx_call()
     try:
       for test in self.tests:
-        yield test, func(test.bind(context=ctx_object))
+        yield test.get_info(), func(test.bind(context=ctx_object))
     finally:
       self.destroy_ctx_call.bind(context=ctx_object)()
 
-  @staticmethod
-  def expect_path(_ext=None):
-    return None
+  def restrict(self, tests):
+    """A helper method to re-cast the MultiTest with fewer subtests.
+
+    All fields will be identical except for tests. If this MultiTest is atomic,
+    then this method returns |self|.
+
+    Used internally by expect_tests, you're not expected to call this yourself.
+    """
+    if self.atomic:
+      return self
+    assert all(t in self.tests for t in tests)
+    return MultiTest(self._name, self._make_ctx_call, self._destroy_ctx_call,
+                     tests, self._atomic)
 
   def get_info(self):
     """Strips MultiTest instance of hard-to-pickle stuff
 
     Returns a MultiTestInfo instance.
     """
     all_tests = [test.get_info() for test in self.tests]
-    test = MultiTestInfo(name=self.name,
-                         tests=all_tests,
-                         atomic=self.atomic
-                         )
+    test = MultiTest(self._name, None, None, all_tests, self._atomic)
     return test
 
+  @staticmethod
+  def expect_path(_ext=None):
+    return None
+
+
 
 class Handler(object):
   """Handler object.
 
   Defines 3 handler methods for each stage of the test pipeline. The pipeline
   looks like:
 
                          ->           ->
                          ->    jobs   ->                   (main)
   GenStage -> test_queue ->      *    -> result_queue -> ResultStage
@@ skipping 102 matching lines @@
         print 'UNHANDLED:', obj
       return Failure()
 
     def finalize(self, aborted):
       """Called after __call__() has been called for all results.
 
       @param aborted: True if the user aborted the run.
       @type aborted: bool
       """
       pass