On Fri, Feb 4, 2011 at 7:03 PM, Blair Zajac <bl...@orcaware.com> wrote:
>
> On Feb 4, 2011, at 9:15 AM, Hyrum K Wright wrote:
>
>> We currently mark tests XFail (or Skip, or something else) by wrapping
>> them in the test_list in the test suite. Rather than doing it there,
>> I think it makes more sense to use Python's decorator syntax to mark
>> tests as XFail right at their definition, rather than down in the test
>> list. Keeping all attributes of a test in close proximity is a Good
>> Thing, imo. Attached is a patch which demonstrates this.
>>
>> Decorators were added to Python in 2.4, which is the minimal version
>> required for our test suite. In addition to the functional
>> decorators, we should be able to add ones which record other
>> information, such as the issues which the tests were added for. (In
>> some future world, we could also divide up the test suite into
>> "levels", and decorators could be added to indicate that.)
>>
>> Thoughts?
>
> Sounds good to me.
>
> The decorators could have a required issue version number as a parameter,
> thereby forcing an issue to exist in the issue tracker.
I don't think we should require all tests to have an issue number
associated with them, but having this information programatically is a
Good Thing. I've updated the patch to introduce an "issues"
decorator, and resolve conflicts introduced by Paul's recent commit.
[I plan on cleaning up references and the like before committing.]
-Hyrum
[[[
Index: subversion/tests/cmdline/svntest/testcase.py
===================================================================
--- subversion/tests/cmdline/svntest/testcase.py (revision 1067239)
+++ subversion/tests/cmdline/svntest/testcase.py (working copy)
@@ -108,6 +108,13 @@ class TestCase:
"""
return self._delegate.get_sandbox_name()
+ def set_issues(self, issues):
+ """Set the issues associated with this test."""
+ if type(issues) == type(0):
+ self.issues = [issues]
+ else:
+ self.issues = issues
+
def run(self, sandbox):
"""Run the test within the given sandbox."""
return self._delegate.run(sandbox)
@@ -134,7 +141,7 @@ class FunctionTestCase(TestCase):
is derived from the file name in which FUNC was defined)
"""
- def __init__(self, func):
+ def __init__(self, func, issues=None):
# it better be a function that accepts an sbox parameter and has a
# docstring on it.
assert isinstance(func, types.FunctionType)
@@ -158,7 +165,7 @@ class FunctionTestCase(TestCase):
assert doc[0].lower() == doc[0], \
"%s's docstring should not be capitalized" % name
- TestCase.__init__(self, doc=doc)
+ TestCase.__init__(self, doc=doc, issues=issues)
self.func = func
def get_function_name(self):
@@ -207,6 +214,27 @@ class XFail(TestCase):
return self._delegate.list_mode() or 'XFAIL'
+def xfail_deco(func):
+ if isinstance(func, TestCase):
+ return XFail(func, issues=func.issues)
+ else:
+ return XFail(func)
+
+
+def issue_deco(issues):
+ def _second(func):
+ if isinstance(func, TestCase):
+ # if the wrapped thing is already a test case, just set the issues
+ func.set_issues(issues)
+ return func
+
+ else:
+ # we need to wrap the function
+ return create_test_case(func, issues=issues)
+
+ return _second
+
+
class Wimp(XFail):
"""Like XFail, but indicates a work-in-progress: an unexpected pass
is not considered a test failure."""
@@ -259,8 +287,8 @@ class SkipUnless(Skip):
Skip.__init__(self, test_case, lambda c=cond_func: not c())
-def create_test_case(func):
+def create_test_case(func, issues=None):
if isinstance(func, TestCase):
return func
else:
- return FunctionTestCase(func)
+ return FunctionTestCase(func, issues=issues)
Index: subversion/tests/cmdline/basic_tests.py
===================================================================
--- subversion/tests/cmdline/basic_tests.py (revision 1067239)
+++ subversion/tests/cmdline/basic_tests.py (working copy)
@@ -1961,6 +1961,8 @@ def basic_rm_urls_one_repo(sbox):
expected_status)
# Test for issue #1199
+@svntest.testcase.xfail_deco
+@svntest.testcase.issue_deco(1199)
def basic_rm_urls_multi_repos(sbox):
"remotely remove directories from two repositories"
@@ -2721,7 +2723,7 @@ test_list = [ None,
delete_keep_local_twice,
windows_paths_in_repos,
basic_rm_urls_one_repo,
- XFail(basic_rm_urls_multi_repos, issues=1199),
+ basic_rm_urls_multi_repos,
automatic_conflict_resolution,
info_nonexisting_file,
basic_relative_url_using_current_dir,
]]]
