On Fri, Feb 4, 2011 at 2:11 PM, Hyrum K Wright <hyrum_at_hyrumwright.org> wrote:
> On Fri, Feb 4, 2011 at 7:03 PM, Blair Zajac <blair_at_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,
> ]]]
>
Received on 2011-02-04 20:21:06 CET