Commits

Bobby Impollonia committed d5821eb

Overhaul the Attribute Selector plugin to fix the following bugs:
Issue #412 : Attributes don't work on inherited test method
Issue #411 : Plugin doesn't work on classes containing static methods
Issue #324 : Mixing method and class attributes doesn't work
Issue #381 : Plugin doesn't work on classes containing methods with no __dict__ (e.g., from boost.python)
Issue #382 : Dupe of #381 with less info
* When used as class decorator, attr() was only applying the attributes to methods with names starting with "test_". It would skip, e.g., my_cool_test.
* The class decorator form of attr() did not work for test methods inherited from parent classes, nor were the attributes inherited by child classes. They also didn't work for methods added to the class later (e.g., by another decorator).

The primary changes are to use getattr/ setattr exlusively rather than direct access to __dict__ and to remove the (unneeded and problematic) wantClass method. Most of the bugs were caused by __dict__ access or wantClass not doing the right thing. In addition to fixing the above bugs, these changes make the plugin smaller and simpler to understand.

Add regression tests for fixed bugs.

Change
assert not plug.wantFunction(h)
to
assert plug.wantFunction(h) is False
where plug.wantFunction only return None or False

Refactor duplicated code in functional tests into base class.

Comments (0)

Files changed (4)

functional_tests/support/att/test_attr.py

         pass
 
 
+class Superclass:
+    def test_method(self):
+        pass
+    test_method.from_super = True
+
+class TestSubclass(Superclass):
+    pass
+
+
+class Static:
+    def test_with_static(self):
+        pass
+    test_with_static.with_static = True
+
+    def static(self):
+        pass
+    static = staticmethod(static)
+
+
+class TestClassAndMethodAttrs(unittest.TestCase):
+    def test_method(self):
+        pass
+    test_method.meth_attr = 'method'
+TestClassAndMethodAttrs.cls_attr = 'class'
+
+
 class TestAttrClass:
+    from_super = True
+
+    def ends_with_test(self):
+        pass
 
     def test_one(self):
         pass
 
     def test_two(self):
         pass
+    test_two.from_super = False
 
 TestAttrClass = attr('a')(TestAttrClass)
+
+
+class TestAttrSubClass(TestAttrClass):
+    def test_sub_three(self):
+        pass
+
+def added_later_test(self):
+    pass
+
+TestAttrSubClass.added_later_test = added_later_test

functional_tests/test_attribute_plugin.py

 
 compat_24 = sys.version_info >= (2, 4)
 
-class TestSimpleAttribute(PluginTester, unittest.TestCase):
-    activate = "-a a"
-    args = ['-v']
+class AttributePluginTester(PluginTester, unittest.TestCase):
     plugins = [AttributeSelector()]
     suitepath = os.path.join(support, 'att')
+    # Some cases need -a to activate and others need -A, so
+    # let's treat -v as the activate argument and let individual
+    # cases specify their -a arguments as part of args
+    activate = '-v'
 
     def runTest(self):
         print '*' * 70
         print str(self.output)
         print '*' * 70
-        
+        self.verify()
+
+    def verify(self):
+        raise NotImplementedError()
+
+
+class TestSimpleAttribute(AttributePluginTester):
+    args = ["-a", "a"]
+
+    def verify(self):
         assert 'test_attr.test_one ... ok' in self.output
         assert 'test_attr.test_two ... ok' in self.output
         assert 'TestClass.test_class_one ... ok' in self.output
         assert 'test_case_three' not in self.output
         assert 'TestAttrClass.test_one ... ok' in self.output
         assert 'TestAttrClass.test_two ... ok' in self.output
+        assert 'TestAttrClass.ends_with_test ... ok' in self.output
 
 
-class TestNotSimpleAttribute(PluginTester, unittest.TestCase):
-    activate = "-a !a"
-    args = ['-v']
-    plugins = [AttributeSelector()]
-    suitepath = os.path.join(support, 'att')
+class TestNotSimpleAttribute(AttributePluginTester):
+    args = ["-a", "!a"]
 
-    def runTest(self):
-        print '*' * 70
-        print str(self.output)
-        print '*' * 70
-        
+    def verify(self):
         assert 'test_attr.test_one ... ok' not in self.output
         assert 'test_attr.test_two ... ok' not in self.output
         assert 'TestClass.test_class_one ... ok' not in self.output
         assert 'test_case_three' in self.output
 
 
-class TestAttributeValue(PluginTester, unittest.TestCase):
-    activate = "-a b=2"
-    args = ['-v']
-    plugins = [AttributeSelector()]
-    suitepath = os.path.join(support, 'att')
+class TestAttributeValue(AttributePluginTester):
+    args = ["-a", "b=2"]
 
-    def runTest(self):
-        print '*' * 70
-        print str(self.output)
-        print '*' * 70
-        
+    def verify(self):
         assert 'test_attr.test_one ... ok' not in self.output
         assert 'test_attr.test_two ... ok' not in self.output
         assert 'test_attr.test_three ... ok' not in self.output
         assert 'test_case_three' in self.output
 
 
-class TestAttributeArray(PluginTester, unittest.TestCase):
-    activate = "-a d=2"
-    args = ['-v']
-    plugins = [AttributeSelector()]
-    suitepath = os.path.join(support, 'att')
+class TestAttributeArray(AttributePluginTester):
+    args = ["-a", "d=2"]
 
-    def runTest(self):
-        print '*' * 70
-        print str(self.output)
-        print '*' * 70
-        
+    def verify(self):
         assert 'test_attr.test_one ... ok' in self.output
         assert 'test_attr.test_two ... ok' in self.output
         assert 'test_attr.test_three ... ok' not in self.output
         assert 'test_case_three' not in self.output
 
 
-class TestAttributeArrayAnd(PluginTester, unittest.TestCase):
-    activate = "-a d=1,d=2"
-    args = ['-v']
-    plugins = [AttributeSelector()]
-    suitepath = os.path.join(support, 'att')
+class TestAttributeArrayAnd(AttributePluginTester):
+    args = ["-a", "d=1,d=2"]
 
-    def runTest(self):
-        print '*' * 70
-        print str(self.output)
-        print '*' * 70
-        
+    def verify(self):
         assert 'test_attr.test_one ... ok' in self.output
         assert 'test_attr.test_two ... ok' not in self.output
         assert 'test_attr.test_three ... ok' not in self.output
         assert 'test_case_three' not in self.output
 
 
-class TestAttributeArrayOr(PluginTester, unittest.TestCase):
-    activate = "-v"
-    args = ['-a', 'd=1', '-a', 'd=2']
-    plugins = [AttributeSelector()]
-    suitepath = os.path.join(support, 'att')
+class TestAttributeArrayOr(AttributePluginTester):
+    args = ["-a", "d=1", "-a", "d=2"]
 
-    def runTest(self):
-        print '*' * 70
-        print str(self.output)
-        print '*' * 70
-        
+    def verify(self):
         assert 'test_attr.test_one ... ok' in self.output
         assert 'test_attr.test_two ... ok' in self.output
         assert 'test_attr.test_three ... ok' in self.output
         assert 'test_case_three' not in self.output
         
 
+class TestInheritance(AttributePluginTester):
+    # Issue #412
+    args = ["-a", "from_super"]
+
+    def verify(self):
+        assert 'TestSubclass.test_method ... ok' in self.output
+        assert 'TestAttrSubClass.test_sub_three ... ok' in self.output
+        assert 'TestAttrSubClass.test_one ... ok' in self.output
+        assert 'TestAttrSubClass.added_later_test ... ok' in self.output
+        assert 'test_two' not in self.output
+        assert 'test_case_one' not in self.output
+        assert 'test_case_three' not in self.output
+
+
+class TestStatic(AttributePluginTester):
+    # Issue #411
+    args = ["-a", "with_static"]
+    suitepath = os.path.join(support, 'att', 'test_attr.py:Static')
+
+    def verify(self):
+        assert 'Static.test_with_static ... ok' in self.output
+        assert 'test_case_two' not in self.output
+        assert 'test_case_one' not in self.output
+        assert 'test_case_three' not in self.output
+
+
+class TestClassAndMethodAttrs(AttributePluginTester):
+    # Issue #324
+    args = ["-a", "meth_attr=method,cls_attr=class"]
+
+    def verify(self):
+        assert '(test_attr.TestClassAndMethodAttrs) ... ok' in self.output
+        assert 'test_case_two' not in self.output
+        assert 'test_case_one' not in self.output
+        assert 'test_case_three' not in self.output
+
+
 if compat_24:
-    class TestAttributeEval(PluginTester, unittest.TestCase):
-        activate = "-A c>20"
-        args = ['-v']
-        plugins = [AttributeSelector()]
-        suitepath = os.path.join(support, 'att')
+    class TestAttributeEval(AttributePluginTester):
+        args = ["-A", "c>20"]
 
-        def runTest(self):
-            print '*' * 70
-            print str(self.output)
-            print '*' * 70
-
+        def verify(self):
             assert 'test_attr.test_one ... ok' not in self.output
             assert 'test_attr.test_two ... ok' not in self.output
             assert 'test_attr.test_three ... ok' not in self.output
             assert 'test_case_one' not in self.output
             assert 'test_case_three' not in self.output
 
+
+# Avoid trying to run base class as tests
+del AttributePluginTester
+
 if __name__ == '__main__':
     #import logging
     #logging.basicConfig(level=logging.DEBUG)

nose/plugins/attrib.py

 compat_24 = sys.version_info >= (2, 4)
 
 def attr(*args, **kwargs):
-    """Decorator that adds attributes to objects
+    """Decorator that adds attributes to classes or functions
     for use with the Attribute (-a) plugin.
     """
-    def apply_to_fn(fn):
+    def wrap_ob(ob):
         for name in args:
-            # these are just True flags:
-            fn.__dict__[name] = 1
-        fn.__dict__.update(kwargs)
-
-    def wrap_ob(ob):
-        if inspect.isclass(ob):
-            cls = ob
-            for k, v in cls.__dict__.items():
-                if not k.lower().startswith('test'):
-                    continue
-                fn = getattr(cls, k)
-                apply_to_fn(fn)
-        else:
-            apply_to_fn(ob)
+            setattr(ob, name, True)
+        for name, value in kwargs.iteritems():
+            setattr(ob, name, value)
         return ob
-
     return wrap_ob
 
+def get_method_attr(method, cls, attr_name, default = False):
+    """Look up an attribute on a method/ function. 
+    If the attribute isn't found there, looking it up in the
+    method's class, if any.
+    """
+    Missing = object()
+    value = getattr(method, attr_name, Missing)
+    if value is Missing and cls is not None:
+        value = getattr(cls, attr_name, Missing)
+    if value is Missing:
+        return default
+    return value
+
+
 class ContextHelper:
-    """Returns default values for dictionary lookups."""
-    def __init__(self, obj):
-        self.obj = obj
+    """Object that can act as context dictionary for eval and looks up
+    names as attributes on a method/ function and its class. 
+    """
+    def __init__(self, method, cls):
+        self.method = method
+        self.cls = cls
 
     def __getitem__(self, name):
-        return self.obj.get(name, False)
+        return get_method_attr(self.method, self.cls, name)
 
 
-class AttributeGetter:
-    """Helper for looking up attributes
-
-    First we check the method, and if the attribute is not present,
-    we check the method's class.
-    """
-    missing = object()
-
-    def __init__(self, cls, method):
-        self.cls = cls
-        self.method = method
-
-    def get(self, name, default=None):
-        log.debug('Get %s from %s.%s', name, self.cls, self.method)
-        val = self.method.__dict__.get(name, self.missing)
-        if val is self.missing:
-            log.debug('No attribute %s in method, getting from class',
-                      name)
-            val = getattr(self.cls, name, default)
-            log.debug('Class attribute %s value: %s', name, val)
-        return val
-
 class AttributeSelector(Plugin):
     """Selects test cases to be run based on their attributes.
     """
             for attr in eval_attr:
                 # "<python expression>"
                 # -> eval(expr) in attribute context must be True
-                def eval_in_context(expr, attribs):
-                    return eval(expr, None, ContextHelper(attribs))
+                def eval_in_context(expr, obj, cls):
+                    return eval(expr, None, ContextHelper(obj, cls))
                 self.attribs.append([(attr, eval_in_context)])
 
         # attribute requirements are a comma separated list of
         if self.attribs:
             self.enabled = True
 
-    def validateAttrib(self, attribs):
+    def validateAttrib(self, method, cls = None):
+        """Verify whether a method has the required attributes
+        The method is considered a match if it matches all attributes
+        for any attribute group.
+        ."""
         # TODO: is there a need for case-sensitive value comparison?
-        # within each group, all must match for the group to match
-        # if any group matches, then the attribute set as a whole
-        # has matched
         any = False
         for group in self.attribs:
             match = True
             for key, value in group:
-                obj_value = attribs.get(key)
+                attr = get_method_attr(method, cls, key)
                 if callable(value):
-                    if not value(key, attribs):
+                    if not value(key, method, cls):
                         match = False
                         break
                 elif value is True:
                     # value must exist and be True
-                    if not bool(obj_value):
+                    if not bool(attr):
                         match = False
                         break
                 elif value is False:
                     # value must not exist or be False
-                    if bool(obj_value):
+                    if bool(attr):
                         match = False
                         break
-                elif type(obj_value) in (list, tuple):
+                elif type(attr) in (list, tuple):
                     # value must be found in the list attribute
-
                     if not str(value).lower() in [str(x).lower()
-                                                  for x in obj_value]:
+                                                  for x in attr]:
                         match = False
                         break
                 else:
                     # value must match, convert to string and compare
-                    if (value != obj_value
-                        and str(value).lower() != str(obj_value).lower()):
+                    if (value != attr
+                        and str(value).lower() != str(attr).lower()):
                         match = False
                         break
             any = any or match
             return None
         return False
 
-    def wantClass(self, cls):
-        """Accept the class if the class or any method is wanted.
-        """
-        cls_attr = cls.__dict__
-        if self.validateAttrib(cls_attr) is not False:
-            return None
-        # Methods in __dict__.values() are functions, oddly enough.
-        methods = filter(isfunction, cls_attr.values())
-        wanted = filter(lambda m: m is not False,
-                        map(self.wantFunction, methods))
-        if wanted:
-            return None
-        return False
-
     def wantFunction(self, function):
         """Accept the function if its attributes match.
         """
-        return self.validateAttrib(function.__dict__)
+        return self.validateAttrib(function)
 
     def wantMethod(self, method):
         """Accept the method if its attributes match.
         """
-        attribs = AttributeGetter(method.im_class, method)
-        return self.validateAttrib(attribs)
+        try:
+            cls = method.im_class
+        except AttributeError:
+            return False
+        return self.validateAttrib(method, cls)

unit_tests/test_attribute_plugin.py

-
+# There are more attribute plugin unit tests in unit_tests/test_plugins.py
 from nose.tools import eq_
 from nose.plugins.attrib import attr
 
     eq_(test.role, 'integration')
 
 def test_class_attrs():
+    # @attr('slow', 'net', role='integration')
     class MyTest:
         def setUp():
             pass
         def test_two(self):
             pass
 
+    class SubClass(MyTest):
+        pass
+
     MyTest = attr('slow', 'net', role='integration')(MyTest)
-    for n in ('test_one', 'test_two'):
-        eq_(getattr(MyTest, n).slow, 1)
-        eq_(getattr(MyTest, n).net, 1)
-        eq_(getattr(MyTest, n).slow, 1)
+    eq_(MyTest.slow, 1)
+    eq_(MyTest.net, 1)
+    eq_(MyTest.role, 'integration')
+    eq_(SubClass.slow, 1)
 
     assert not hasattr(MyTest.setUp, 'slow')
-    
+    assert not hasattr(MyTest.test_two, 'slow')