On Thu, Aug 6, 2009 at 10:10 AM, Robert
Collins<email address hidden> wrote:
> Robert Collins has proposed merging lp:~lifeless/subunit/xfail into lp:subunit.
>
> Requested reviews:
>    Subunit Developers (subunit)
>
> make xfail use addExpectedFailure

Hey Rob,

Only a few comments here.

> === modified file 'NEWS'
> --- NEWS	2009-08-02 22:55:36 +0000
> +++ NEWS	2009-08-06 08:48:01 +0000
> @@ -32,6 +32,11 @@
>
>    BUG FIXES:
>
> +    * ``xfail`` outcomes are now passed to python TestResult's via
> +      addExpectedFailure if it is present on the TestResult. Python 2.6 and
> +      earlier which do not have this function will have ``xfail`` outcomes
> +      passed through as success outcomes as earlier versions of subunit did.
> +
>    API CHANGES:
>
>      * When a progress: directive is encountered in a subunit stream, the
>
> === 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.

> +                xfail(self._current_test, RemoteError())
> +            else:
> +                self.client.addSuccess(self._current_test)
>              self.client.stopTest(self._current_test)
>          elif (self.state == TestProtocolServer.TEST_STARTED and
>              self.current_test_description + " [" == line[offset:-1]):
> @@ -203,10 +207,16 @@
>              self.current_test_description = None
>              self._skip_or_error(self._message)
>              self.client.stopTest(self._current_test)
> -        elif self.state in (
> -            TestProtocolServer.READING_SUCCESS,
> -            TestProtocolServer.READING_XFAIL,
> -            ):
> +        elif self.state == TestProtocolServer.READING_XFAIL:
> +            self.state = TestProtocolServer.OUTSIDE_TEST
> +            self.current_test_description = None
> +            xfail = getattr(self.client, 'addExpectedFailure', None)
> +            if callable(xfail):
> +                xfail(self._current_test, RemoteError(self._message))

This duplicates a bit of logic above.

> +            else:
> +                self.client.addSuccess(self._current_test)
> +            self.client.stopTest(self._current_test)
> +        elif self.state == TestProtocolServer.READING_SUCCESS:
>              self._succeedTest()
>          else:
>              self.stdOutLineReceived(line)
>
> === modified file 'python/subunit/tests/test_test_protocol.py'
> --- python/subunit/tests/test_test_protocol.py	2009-08-02 22:55:36 +0000
> +++ python/subunit/tests/test_test_protocol.py	2009-08-06 08:48:01 +0000
> @@ -28,7 +28,11 @@
>
>
>  class MockTestProtocolServerClient(object):
> -    """A mock protocol server client to test callbacks."""
> +    """A mock protocol server client to test callbacks.
> +
> +    Note that this is deliberately not python 2.7 complete, to allow

'Python' not 'python'.

> +    testing compatibility.

Can you please elaborate on _why_ not supporting Python 2.7 completely allows
testing compatibility?

> +    """
>
>      def __init__(self):
>          self.end_calls = []

Thanks,
jml