Opened 10 years ago

Closed 9 years ago

#3909 defect closed fixed (fixed)

Decorated test methods don't run when trial'ed directly

Reported by: ivank Owned by: Jean-Paul Calderone
Priority: lowest Milestone:
Component: trial Keywords:
Cc: therve Branch: branches/trial-decorated-methods-3909
branch-diff, diff-cov, branch-cov, buildbot
Author: mishok13

Description

When using decorated tests:

from twisted.trial import unittest

print "Print works in this environment."

def a_decorator(fn):
        print 'Decorating', fn 
        def newfunc(*args, **kwargs):
                print "I am decorated."
                fn(*args, **kwargs)
        return newfunc

class TestA(unittest.TestCase):
        def test_something(self):
                self.assertEqual(1, 1)
        test_something = a_decorator(test_something)

trial decorated_test.TestA works, but trial decorated_test.TestA.test_something does not:

# trial decorated_test.TestA.test_something
Print works in this environment.
Decorating <function test_something at 0x7f101729d5f0>
Traceback (most recent call last):
  File "/usr/local/bin/trial", line 22, in <module>
    run()
  File "/opt/Python-latest/lib/python2.7/site-packages/twisted/scripts/trial.py", line 328, in run
    suite = _getSuite(config)
  File "/opt/Python-latest/lib/python2.7/site-packages/twisted/scripts/trial.py", line 286, in _getSuite
    return loader.loadByNames(config['tests'], recurse)
  File "/opt/Python-latest/lib/python2.7/site-packages/twisted/trial/runner.py", line 657, in loadByNames
    for thing in set(things)]
  File "/opt/Python-latest/lib/python2.7/site-packages/twisted/trial/runner.py", line 621, in loadAnything
    return self.loadMethod(thing)
  File "/opt/Python-latest/lib/python2.7/site-packages/twisted/trial/runner.py", line 524, in loadMethod
    return self._makeCase(method.im_class, method.__name__)
  File "/opt/Python-latest/lib/python2.7/site-packages/twisted/trial/runner.py", line 527, in _makeCase
    return klass(methodName)
  File "/opt/Python-latest/lib/python2.7/site-packages/twisted/trial/unittest.py", line 672, in __init__
    super(TestCase, self).__init__(methodName)
  File "/opt/Python-latest/lib/python2.7/unittest.py", line 363, in __init__
    (self.__class__, methodName))
ValueError: no such test method in <class 'decorated_test.TestA'>: newfunc

Attachments (3)

misnamed-method.patch (5.3 KB) - added by kelly 10 years ago.
misnamed-method-2.patch (9.8 KB) - added by kelly 10 years ago.
Updated version of the patch which is slightly more conformant to the style guidelines.
3909.patch (10.5 KB) - added by Andrii V. Mishkovskyi 9 years ago.
Reresubmitted version of the patch

Download all attachments as: .zip

Change History (15)

comment:1 Changed 10 years ago by ivank

Type: enhancementdefect

comment:2 Changed 10 years ago by kelly

This problem arises because for a decorated method:

method = decorated_test.TestA.test_something
getattr(method.im_class, method.__name__) != method

A workaround would be to set newfunc.__name__ = fn.__name__ before returning from a_decorator. functools.update_wrapper, functools.wrap and twisted.python.util.mergeFunctionMetadata are alternatives depending on the version of python you are running.

from functools import wrap
def a_decorator(fn):
        print 'Decorating', fn
        @wrap(fn)
        def newfunc(*args, **kwargs):
                print "I am decorated."
                fn(*args, **kwargs)
        return newfunc

Attached is a patch to twisted/trial/runner.py which resolves the issue by searching the attributes of method.im_class for the method object.

comment:3 Changed 10 years ago by ivank

Keywords: review added
Owner: Jonathan Lange deleted

Changed 10 years ago by kelly

Attachment: misnamed-method.patch added

comment:4 Changed 10 years ago by kelly

Original patch broke a couple of tests in twisted.trial.test.test_runner.TestRunner. Uploaded a new version.

Changed 10 years ago by kelly

Attachment: misnamed-method-2.patch added

Updated version of the patch which is slightly more conformant to the style guidelines.

comment:5 Changed 10 years ago by therve

Cc: therve added
Keywords: review removed
Owner: set to kelly

Nice fix! 2 comments:

  • the object() trick in the getattr call is probably not necessary.
  • the second hasattr call seems unnecessary (it's better to pass a second argument to the getattr call.
  • please use the assert* check methods instead of the failUnless* ones
  • some blank lines missing in the sample file.

Thanks!

comment:6 Changed 9 years ago by Andrii V. Mishkovskyi

Keywords: review added
Owner: kelly deleted

OK, as the original submitter didn't fix the patch, I've cleaned it up and resubmitted as 3909.patch, please review.

Changed 9 years ago by Andrii V. Mishkovskyi

Attachment: 3909.patch added

Reresubmitted version of the patch

comment:7 Changed 9 years ago by Jean-Paul Calderone

Owner: set to Jean-Paul Calderone
Status: newassigned

comment:8 Changed 9 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/trial-decorated-methods-3909

(In [31105]) Branching to 'trial-decorated-methods-3909'

comment:9 Changed 9 years ago by Jean-Paul Calderone

(In [31106]) Apply patch

refs #3909

comment:10 Changed 9 years ago by Jean-Paul Calderone

Author: exarkunmishok13
Keywords: review removed

Woo, thanks!

A few stylistic things:

  1. assertEquals is preferred over assertEqual
  2. Tests should be named like test_camelCase rather than test_under_scores
  3. Likewise for other functions, goodDecorator rather than good_decorator
  4. Docstrings on the sample test methods explaining what state about them is interesting would help readability somewhat.
  5. A news fragment describing this bugfix for the NEWS file is needed too

This is all simple, so I'll take care of it and merge. Thanks!

comment:11 Changed 9 years ago by Jean-Paul Calderone

(In [31107]) Fix some style issues

refs #3909

comment:12 Changed 9 years ago by Jean-Paul Calderone

Resolution: fixed
Status: assignedclosed

(In [31110]) Merge trial-decorated-methods-3909

Author: kelly, mishok13 Reviewer: exarkun Fixes: #3909

Add the ability to load test methods with names that do not match their name in their containing class to trial's test loader.

Note: See TracTickets for help on using tickets.