rooneg@tigris.org wrote:
> Author: rooneg
> Date: Thu Apr 27 10:18:09 2006
> New Revision: 19462
>
> Modified:
> trunk/subversion/tests/cmdline/README
> trunk/subversion/tests/cmdline/authz_tests.py
> trunk/subversion/tests/cmdline/davautocheck.sh
> trunk/subversion/tests/cmdline/svnsync_tests.py
> trunk/subversion/tests/cmdline/svntest/main.py
> trunk/subversion/tests/cmdline/svntest/testcase.py
> trunk/tools/test-scripts/svntest/mod_dav_svn.conf
>
[...]
>
> * subversion/tests/cmdline/svntest/testcase.py
> (XFail.__init__): Add a cond parameter, don't set up _result_text here,
> stash cond for later.
> (XFail.convert_result): Set up the _result_text here, and mention why.
> Also reverse the order of the pass and fail strings, due to the fact
> that we're changing the order of the call to this function.
>
[...]
> Modified: trunk/subversion/tests/cmdline/svntest/testcase.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/svntest/testcase.py?pathrev=19462&r1=19461&r2=19462
> ==============================================================================
> --- trunk/subversion/tests/cmdline/svntest/testcase.py (original)
> +++ trunk/subversion/tests/cmdline/svntest/testcase.py Thu Apr 27 10:18:09 2006
> @@ -107,18 +107,26 @@
> class XFail(TestCase):
> "A test that is expected to fail."
>
> - def __init__(self, test_case):
> + def __init__(self, test_case, cond=None):
> TestCase.__init__(self)
> self.test_case = create_test_case(test_case)
> - self._result_text = ['XPASS:', 'XFAIL:', self.test_case.run_text(2)]
> self._list_mode_text = self.test_case.list_mode() or 'XFAIL'
> # Delegate most methods to self.test_case:
> self.get_description = self.test_case.get_description
> self.need_sandbox = self.test_case.need_sandbox
> self.get_sandbox_name = self.test_case.get_sandbox_name
> self.run = self.test_case.run
> + self.cond = cond
My first nit is that this change to testcase.XFail was committed with a
raft of other changes that could have been committed separately.
This is going to cause confusion. The Skip class has a "cond"
constructor parameter too, but in that case it is a simple boolean value
(rather than a callable that returns a bool, as in your modified XFail).
Therefore I suggest that you either make the parameters consistent with
each other, or rename the XFail parameter.
Also, if you really want cond to be a callable, then I suggest the
default value "lambda: 1" instead of "None". Then you don't need the
first part of the "if self.cond is None or ..." test below.
> def convert_result(self, result):
> + # We delay the setting of _result_text to this point because if we do
> + # it in __init__ it turns out that useful bits of information (like the
> + # fact that we're running over a particular RA layer) are not available.
> + if self.cond is None or self.cond():
> + self._result_text = ['XFAIL:', 'XPASS:', self.test_case.run_text(2)]
> + else:
> + self._result_text = ['FAIL:', 'PASS:', self.test_case.run_text(2)]
> +
> # Conditions are reversed here: a failure is expected, therefore
> # it isn't an error; a pass is an error; but a skip remains a skip.
> return {0:1, 1:0, 2:2}[self.test_case.convert_result(result)]
>
Shouldn't the translation of the return value also be conditional on cond?
This change also makes the XFail wrapper mutable where it wasn't before.
As these objects are stored into static lists, one would tend to assume
that the objects are immutable (in fact, they used to be decorators in
the GoF sense).
Therefore, I suggest the attached patch to be applied on top of yours.
[[[
Alter the XFail decorator to be immutable again.
* subversion/tests/cmdline/svntest/testcase.py (XFail.__init__): Rename
cond parameter to cond_func. Initialize cond_func to lambda:1 instead
of None. Comment its deferred behavior here instead of in
convert_result().
(XFail.convert_result): Only reverse the result value if cond_func
returns true. Don't overwrite self._result_text. Omit "if
self.cond_func is None" test, as it is unnecessary now.
(XFail.run_text): If self.cond_func returns true, then output run_text
appropriate for a test that is expected to fail.
]]]
Yours,
Michael
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Apr 28 00:42:20 2006