Code review comment for lp:~lifeless/subunit/xfail

Revision history for this message
Robert Collins (lifeless) wrote :

On Thu, 2009-08-06 at 09:21 +0000, Jonathan Lange wrote:
>
> > === modified file 'python/subunit/__init__.py'
> > --- python/subunit/__init__.py 2009-08-02 22:55:36 +0000
> > +++ python/subunit/__init__.py 2009-08-06 08:48:01 +0000
> > @@ -120,7 +120,11 @@
> > self.current_test_description == line[offset:-1]):
> > self.state = TestProtocolServer.OUTSIDE_TEST
> > self.current_test_description = None
> > - self.client.addSuccess(self._current_test)
> > + xfail = getattr(self.client, 'addExpectedFailure',
> None)
> > + if callable(xfail):
>
> Is there a way to avoid using 'callable' here? It's a little look
> before you
> leap, and is not present in Python 3.
>
> One way to do it would be to call it if not None, and catch a
> TypeError.

Heh; python 2.6 adds a check for 'callable' to TestSuite.addTest. I
think we should do a python 3k migration/test all at once. For now, I'd
like to leave this as-is, simply because its trivial, likely not the
only issue, and I've a bunch of other user visible fish to fry in the
short term.
...
> > + if callable(xfail):
> > + xfail(self._current_test,
> RemoteError(self._message))
>
> This duplicates a bit of logic above.

it does - the whole state machine needs refactoring. I think we should
do that past 0.2.

I've done the other changes.

Cheers,
Rob

« Back to merge proposal