Merge lp:~lifeless/subunit/xfail into lp:~subunit/subunit/trunk

Proposed by Robert Collins
Status: Merged
Merged at revision: not available
Proposed branch: lp:~lifeless/subunit/xfail
Merge into: lp:~subunit/subunit/trunk
Diff against target: None lines
To merge this branch: bzr merge lp:~lifeless/subunit/xfail
Reviewer Review Type Date Requested Status
Jonathan Lange Approve
Review via email: mp+9748@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

make xfail use addExpectedFailure

Revision history for this message
Jonathan Lange (jml) wrote :
Download full text (3.6 KiB)

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 MockTest...

Read more...

Revision history for this message
Jonathan Lange (jml) wrote :

Approved with tweaks, see my other comment.

review: Approve
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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2009-08-02 22:55:36 +0000
+++ NEWS 2009-08-06 08:48:01 +0000
@@ -32,6 +32,11 @@
3232
33 BUG FIXES:33 BUG FIXES:
3434
35 * ``xfail`` outcomes are now passed to python TestResult's via
36 addExpectedFailure if it is present on the TestResult. Python 2.6 and
37 earlier which do not have this function will have ``xfail`` outcomes
38 passed through as success outcomes as earlier versions of subunit did.
39
35 API CHANGES:40 API CHANGES:
3641
37 * When a progress: directive is encountered in a subunit stream, the42 * When a progress: directive is encountered in a subunit stream, the
3843
=== 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 @@
120 self.current_test_description == line[offset:-1]):120 self.current_test_description == line[offset:-1]):
121 self.state = TestProtocolServer.OUTSIDE_TEST121 self.state = TestProtocolServer.OUTSIDE_TEST
122 self.current_test_description = None122 self.current_test_description = None
123 self.client.addSuccess(self._current_test)123 xfail = getattr(self.client, 'addExpectedFailure', None)
124 if callable(xfail):
125 xfail(self._current_test, RemoteError())
126 else:
127 self.client.addSuccess(self._current_test)
124 self.client.stopTest(self._current_test)128 self.client.stopTest(self._current_test)
125 elif (self.state == TestProtocolServer.TEST_STARTED and129 elif (self.state == TestProtocolServer.TEST_STARTED and
126 self.current_test_description + " [" == line[offset:-1]):130 self.current_test_description + " [" == line[offset:-1]):
@@ -203,10 +207,16 @@
203 self.current_test_description = None207 self.current_test_description = None
204 self._skip_or_error(self._message)208 self._skip_or_error(self._message)
205 self.client.stopTest(self._current_test)209 self.client.stopTest(self._current_test)
206 elif self.state in (210 elif self.state == TestProtocolServer.READING_XFAIL:
207 TestProtocolServer.READING_SUCCESS,211 self.state = TestProtocolServer.OUTSIDE_TEST
208 TestProtocolServer.READING_XFAIL,212 self.current_test_description = None
209 ):213 xfail = getattr(self.client, 'addExpectedFailure', None)
214 if callable(xfail):
215 xfail(self._current_test, RemoteError(self._message))
216 else:
217 self.client.addSuccess(self._current_test)
218 self.client.stopTest(self._current_test)
219 elif self.state == TestProtocolServer.READING_SUCCESS:
210 self._succeedTest()220 self._succeedTest()
211 else:221 else:
212 self.stdOutLineReceived(line)222 self.stdOutLineReceived(line)
213223
=== 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 @@
2828
2929
30class MockTestProtocolServerClient(object):30class MockTestProtocolServerClient(object):
31 """A mock protocol server client to test callbacks."""31 """A mock protocol server client to test callbacks.
32
33 Note that this is deliberately not python 2.7 complete, to allow
34 testing compatibility.
35 """
3236
33 def __init__(self):37 def __init__(self):
34 self.end_calls = []38 self.end_calls = []
@@ -555,41 +559,74 @@
555class TestTestProtocolServerAddxFail(unittest.TestCase):559class TestTestProtocolServerAddxFail(unittest.TestCase):
556 """Tests for the xfail keyword.560 """Tests for the xfail keyword.
557561
558 In Python this thunks through to Success due to stdlib limitations (see562 In Python this can thunk through to Success due to stdlib limitations (see
559 README).563 README).
560 """564 """
561565
562 def setUp(self):566 def capture_expected_failure(self, test, err):
563 """Setup a test object ready to be xfailed."""567 self._calls.append((test, err))
564 self.client = MockTestProtocolServerClient()568
569 def setup_python26(self):
570 """Setup a test object ready to be xfailed and thunk to success."""
571 self.client = MockTestProtocolServerClient()
572 self.setup_protocol()
573
574 def setup_python27(self):
575 """Setup a test object ready to be xfailed and thunk to success."""
576 self.client = MockTestProtocolServerClient()
577 self.client.addExpectedFailure = self.capture_expected_failure
578 self._calls = []
579 self.setup_protocol()
580
581 def setup_protocol(self):
582 """Setup the protocol based on self.client."""
565 self.protocol = subunit.TestProtocolServer(self.client)583 self.protocol = subunit.TestProtocolServer(self.client)
566 self.protocol.lineReceived("test mcdonalds farm\n")584 self.protocol.lineReceived("test mcdonalds farm\n")
567 self.test = self.client.start_calls[-1]585 self.test = self.client.start_calls[-1]
568586
569 def simple_xfail_keyword(self, keyword):587 def simple_xfail_keyword(self, keyword, as_success):
570 self.protocol.lineReceived("%s mcdonalds farm\n" % keyword)588 self.protocol.lineReceived("%s mcdonalds farm\n" % keyword)
571 self.assertEqual(self.client.start_calls, [self.test])589 self.assertEqual(self.client.start_calls, [self.test])
572 self.assertEqual(self.client.end_calls, [self.test])590 self.assertEqual(self.client.end_calls, [self.test])
573 self.assertEqual(self.client.error_calls, [])591 self.assertEqual(self.client.error_calls, [])
574 self.assertEqual(self.client.failure_calls, [])592 self.assertEqual(self.client.failure_calls, [])
575 self.assertEqual(self.client.success_calls, [self.test])593 self.check_success_or_xfail(as_success)
594
595 def check_success_or_xfail(self, as_success):
596 if as_success:
597 self.assertEqual(self.client.success_calls, [self.test])
598 else:
599 self.assertEqual(1, len(self._calls))
600 self.assertEqual(self.test, self._calls[0][0])
576601
577 def test_simple_xfail(self):602 def test_simple_xfail(self):
578 self.simple_xfail_keyword("xfail")603 self.setup_python26()
604 self.simple_xfail_keyword("xfail", True)
605 self.setup_python27()
606 self.simple_xfail_keyword("xfail", False)
579607
580 def test_simple_xfail_colon(self):608 def test_simple_xfail_colon(self):
581 self.simple_xfail_keyword("xfail:")609 self.setup_python26()
610 self.simple_xfail_keyword("xfail:", True)
611 self.setup_python27()
612 self.simple_xfail_keyword("xfail:", False)
582613
583 def test_xfail_empty_message(self):614 def test_xfail_empty_message(self):
615 self.setup_python26()
616 self.empty_message(True)
617 self.setup_python27()
618 self.empty_message(False)
619
620 def empty_message(self, as_success):
584 self.protocol.lineReceived("xfail mcdonalds farm [\n")621 self.protocol.lineReceived("xfail mcdonalds farm [\n")
585 self.protocol.lineReceived("]\n")622 self.protocol.lineReceived("]\n")
586 self.assertEqual(self.client.start_calls, [self.test])623 self.assertEqual(self.client.start_calls, [self.test])
587 self.assertEqual(self.client.end_calls, [self.test])624 self.assertEqual(self.client.end_calls, [self.test])
588 self.assertEqual(self.client.error_calls, [])625 self.assertEqual(self.client.error_calls, [])
589 self.assertEqual(self.client.failure_calls, [])626 self.assertEqual(self.client.failure_calls, [])
590 self.assertEqual(self.client.success_calls, [self.test])627 self.check_success_or_xfail(as_success)
591628
592 def xfail_quoted_bracket(self, keyword):629 def xfail_quoted_bracket(self, keyword, as_success):
593 # This tests it is accepted, but cannot test it is used today, because630 # This tests it is accepted, but cannot test it is used today, because
594 # of not having a way to expose it in Python so far.631 # of not having a way to expose it in Python so far.
595 self.protocol.lineReceived("%s mcdonalds farm [\n" % keyword)632 self.protocol.lineReceived("%s mcdonalds farm [\n" % keyword)
@@ -599,13 +636,19 @@
599 self.assertEqual(self.client.end_calls, [self.test])636 self.assertEqual(self.client.end_calls, [self.test])
600 self.assertEqual(self.client.error_calls, [])637 self.assertEqual(self.client.error_calls, [])
601 self.assertEqual(self.client.failure_calls, [])638 self.assertEqual(self.client.failure_calls, [])
602 self.assertEqual(self.client.success_calls, [self.test])639 self.check_success_or_xfail(as_success)
603640
604 def test_xfail_quoted_bracket(self):641 def test_xfail_quoted_bracket(self):
605 self.xfail_quoted_bracket("xfail")642 self.setup_python26()
643 self.xfail_quoted_bracket("xfail", True)
644 self.setup_python27()
645 self.xfail_quoted_bracket("xfail", False)
606646
607 def test_xfail_colon_quoted_bracket(self):647 def test_xfail_colon_quoted_bracket(self):
608 self.xfail_quoted_bracket("xfail:")648 self.setup_python26()
649 self.xfail_quoted_bracket("xfail:", True)
650 self.setup_python27()
651 self.xfail_quoted_bracket("xfail:", False)
609652
610653
611class TestTestProtocolServerAddSkip(unittest.TestCase):654class TestTestProtocolServerAddSkip(unittest.TestCase):

Subscribers

People subscribed via source and target branches