You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Hyrum K Wright <hy...@hyrumwright.org> on 2011/02/04 18:15:11 UTC

Proposal: Use decorators in test suite for XFail, Skip, etc

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?

-Hyrum

[[[
Index: subversion/tests/cmdline/svntest/testcase.py
===================================================================
--- subversion/tests/cmdline/svntest/testcase.py	(revision 1067180)
+++ subversion/tests/cmdline/svntest/testcase.py	(working copy)
@@ -207,6 +207,14 @@
     return self._delegate.list_mode() or 'XFAIL'


+def xfail_deco(f):
+  def _inner(sbox):
+    return f(sbox)
+
+  _inner.__doc__ = f.__doc__
+  return XFail(_inner)
+
+
 class Wimp(XFail):
   """Like XFail, but indicates a work-in-progress: an unexpected pass
   is not considered a test failure."""
Index: subversion/tests/cmdline/basic_tests.py
===================================================================
--- subversion/tests/cmdline/basic_tests.py	(revision 1067180)
+++ subversion/tests/cmdline/basic_tests.py	(working copy)
@@ -1961,6 +1961,7 @@
                                         expected_status)

 # Test for issue #1199
+@svntest.testcase.xfail_deco
 def basic_rm_urls_multi_repos(sbox):
   "remotely remove directories from two repositories"

@@ -2721,7 +2722,7 @@
               delete_keep_local_twice,
               windows_paths_in_repos,
               basic_rm_urls_one_repo,
-              XFail(basic_rm_urls_multi_repos),
+              basic_rm_urls_multi_repos,
               automatic_conflict_resolution,
               info_nonexisting_file,
               basic_relative_url_using_current_dir,
]]]

Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by Noorul Islam K M <no...@collab.net>.
Arwin Arni <ar...@collab.net> writes:

> On Saturday 05 February 2011 01:46 AM, Hyrum K Wright wrote:
>
>> On Fri, Feb 4, 2011 at 7:54 PM, C. Michael Pilato<cm...@collab.net>  wrote:
>>> On 02/04/2011 02:09 PM, Greg Stein wrote:
>>>> On Fri, Feb 4, 2011 at 12:15, Hyrum K Wright<hy...@hyrumwright.org>  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.
>>>> Sure.
>>>>
>>>>> ...
>>>>> +++ subversion/tests/cmdline/basic_tests.py     (working copy)
>>>> XFail = svntest.testcase.xfail_deco
>>>>
>>>>> @@ -1961,6 +1961,7 @@
>>>>>                                          expected_status)
>>>>>
>>>>>   # Test for issue #1199
>>>>> +@svntest.testcase.xfail_deco
>>>> @XFail
>>> Oh yes.  Much, much nicer to read.
>> Pilot committed in r1067273.  That rev only changes basic_tests.py; I
>> plan to hit the others shortly.
>>
>> -Hyrum
> I don't know much about Python decorators, but what if I have a test
> that does two things (like XFail and SkipUnless), do we have two
> decorators at the definition?
>

I think he handled this in r1067380.

Thanks and Regards
Noorul

Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Hyrum K Wright wrote on Mon, Feb 07, 2011 at 13:35:11 +0000:
> Just use them both.  Order *shouldn't* matter.  (I think.)

Pretty sure it *does* matter: e.g., when you combine XFail and Skip, one
of them causes the test to be run and one causes it not to be run ---
and we did have some instances of:

        # Triggers an OS bug to the point that a reboot is required to
        # make the system usable again.
        XFail(Skip(foo, is_windows), lambda: True),

Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Mon, Feb 7, 2011 at 6:48 AM, Arwin Arni <ar...@collab.net> wrote:
> On Saturday 05 February 2011 01:46 AM, Hyrum K Wright wrote:
>>
>> On Fri, Feb 4, 2011 at 7:54 PM, C. Michael Pilato<cm...@collab.net>
>>  wrote:
>>>
>>> On 02/04/2011 02:09 PM, Greg Stein wrote:
>>>>
>>>> On Fri, Feb 4, 2011 at 12:15, Hyrum K Wright<hy...@hyrumwright.org>
>>>>  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.
>>>>
>>>> Sure.
>>>>
>>>>> ...
>>>>> +++ subversion/tests/cmdline/basic_tests.py     (working copy)
>>>>
>>>> XFail = svntest.testcase.xfail_deco
>>>>
>>>>> @@ -1961,6 +1961,7 @@
>>>>>                                         expected_status)
>>>>>
>>>>>  # Test for issue #1199
>>>>> +@svntest.testcase.xfail_deco
>>>>
>>>> @XFail
>>>
>>> Oh yes.  Much, much nicer to read.
>>
>> Pilot committed in r1067273.  That rev only changes basic_tests.py; I
>> plan to hit the others shortly.
>>
>> -Hyrum
>
> I don't know much about Python decorators, but what if I have a test that
> does two things (like XFail and SkipUnless), do we have two decorators at
> the definition?

Just use them both.  Order *shouldn't* matter.  (I think.)

-Hyrum

Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by Arwin Arni <ar...@collab.net>.
On Saturday 05 February 2011 01:46 AM, Hyrum K Wright wrote:
> On Fri, Feb 4, 2011 at 7:54 PM, C. Michael Pilato<cm...@collab.net>  wrote:
>> On 02/04/2011 02:09 PM, Greg Stein wrote:
>>> On Fri, Feb 4, 2011 at 12:15, Hyrum K Wright<hy...@hyrumwright.org>  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.
>>> Sure.
>>>
>>>> ...
>>>> +++ subversion/tests/cmdline/basic_tests.py     (working copy)
>>> XFail = svntest.testcase.xfail_deco
>>>
>>>> @@ -1961,6 +1961,7 @@
>>>>                                          expected_status)
>>>>
>>>>   # Test for issue #1199
>>>> +@svntest.testcase.xfail_deco
>>> @XFail
>> Oh yes.  Much, much nicer to read.
> Pilot committed in r1067273.  That rev only changes basic_tests.py; I
> plan to hit the others shortly.
>
> -Hyrum
I don't know much about Python decorators, but what if I have a test 
that does two things (like XFail and SkipUnless), do we have two 
decorators at the definition?

Regards,
Arwin Arni


Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Fri, Feb 4, 2011 at 7:54 PM, C. Michael Pilato <cm...@collab.net> wrote:
> On 02/04/2011 02:09 PM, Greg Stein wrote:
>> On Fri, Feb 4, 2011 at 12:15, Hyrum K Wright <hy...@hyrumwright.org> 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.
>>
>> Sure.
>>
>>> ...
>>> +++ subversion/tests/cmdline/basic_tests.py     (working copy)
>>
>> XFail = svntest.testcase.xfail_deco
>>
>>> @@ -1961,6 +1961,7 @@
>>>                                         expected_status)
>>>
>>>  # Test for issue #1199
>>> +@svntest.testcase.xfail_deco
>>
>> @XFail
>
> Oh yes.  Much, much nicer to read.

Pilot committed in r1067273.  That rev only changes basic_tests.py; I
plan to hit the others shortly.

-Hyrum

Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 02/04/2011 02:09 PM, Greg Stein wrote:
> On Fri, Feb 4, 2011 at 12:15, Hyrum K Wright <hy...@hyrumwright.org> 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.
> 
> Sure.
> 
>> ...
>> +++ subversion/tests/cmdline/basic_tests.py     (working copy)
> 
> XFail = svntest.testcase.xfail_deco
> 
>> @@ -1961,6 +1961,7 @@
>>                                         expected_status)
>>
>>  # Test for issue #1199
>> +@svntest.testcase.xfail_deco
> 
> @XFail

Oh yes.  Much, much nicer to read.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by Greg Stein <gs...@gmail.com>.
On Fri, Feb 4, 2011 at 12:15, Hyrum K Wright <hy...@hyrumwright.org> 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.

Sure.

>...
> +++ subversion/tests/cmdline/basic_tests.py     (working copy)

XFail = svntest.testcase.xfail_deco

> @@ -1961,6 +1961,7 @@
>                                         expected_status)
>
>  # Test for issue #1199
> +@svntest.testcase.xfail_deco

@XFail

>...

Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Sat, Feb 5, 2011 at 3:22 AM, Benjamin Peterson <be...@python.org> wrote:
> Hyrum K Wright <hyrum <at> hyrumwright.org> writes:
>>
>> [[[
>>
>> 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):
>
> You should use isinstance(issues, int). Or even better see below.
>
>> +def issue_deco(issues):
>
> How about using *issues. Then you won't have to put issues in a list. Also you
> can simply set_issues().

Updated both of these in r1068256.  Thanks for the review!

-Hyrum

Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Benjamin Peterson wrote on Sat, Feb 05, 2011 at 03:22:06 +0000:
> Hyrum K Wright <hyrum <at> hyrumwright.org> writes:
> > +    if type(issues) == type(0):
> 
> You should use isinstance(issues, int). Or even better see below.

Thanks for your input; I'll remember it for next time.

Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by Benjamin Peterson <be...@python.org>.
Hyrum K Wright <hyrum <at> hyrumwright.org> writes:
> 
> [[[
> 
> 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):

You should use isinstance(issues, int). Or even better see below.

> +def issue_deco(issues):

How about using *issues. Then you won't have to put issues in a list. Also you
can simply set_issues().





Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Fri, Feb 4, 2011 at 7:20 PM, Paul Burba <pt...@gmail.com> wrote:
> On Fri, Feb 4, 2011 at 2:11 PM, Hyrum K Wright <hy...@hyrumwright.org> wrote:
>> 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,
>
> Not *every* issue, I agree, but perhaps requiring *XFails* to have an
> associated issue wouldn't be such a bad idea (once we have issues
> associated with all the current XFails of course).  It would certainly
> make the "What XFailing tests are release blockers?" question a lot
> easier to answer.

Agreed, and we could easily change the XFail decorator to require an
issue number.  I suppose we could also do that with the existing XFail
infrastructure, too.

The @Issue decorator could also be used with tests which PASS, for completeness.

-Hyrum

Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by Paul Burba <pt...@gmail.com>.
On Fri, Feb 4, 2011 at 2:11 PM, Hyrum K Wright <hy...@hyrumwright.org> wrote:
> 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,

Not *every* issue, I agree, but perhaps requiring *XFails* to have an
associated issue wouldn't be such a bad idea (once we have issues
associated with all the current XFails of course).  It would certainly
make the "What XFailing tests are release blockers?" question a lot
easier to answer.

Paul

> 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,
> ]]]
>

Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by Hyrum K Wright <hy...@hyrumwright.org>.
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,
]]]

Re: Proposal: Use decorators in test suite for XFail, Skip, etc

Posted by Blair Zajac <bl...@orcaware.com>.
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.

Blair