Merge lp:~lifeless/subunit/xfail into lp:~subunit/subunit/trunk
- xfail
- Merge into trunk
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 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jonathan Lange | Approve | ||
Review via email: mp+9748@code.launchpad.net |
Commit message
Description of the change
Robert Collins (lifeless) wrote : | # |
Jonathan Lange (jml) wrote : | # |
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/
> --- python/
> +++ python/
> @@ -120,7 +120,11 @@
> self.current_
> self.state = TestProtocolSer
> self.current_
> - self.client.
> + xfail = getattr(
> + 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.
> + else:
> + self.client.
> self.client.
> elif (self.state == TestProtocolSer
> self.current_
> @@ -203,10 +207,16 @@
> self.current_
> self._skip_
> self.client.
> - elif self.state in (
> - TestProtocolSer
> - TestProtocolSer
> - ):
> + elif self.state == TestProtocolSer
> + self.state = TestProtocolSer
> + self.current_
> + xfail = getattr(
> + if callable(xfail):
> + xfail(self.
This duplicates a bit of logic above.
> + else:
> + self.client.
> + self.client.
> + elif self.state == TestProtocolSer
> self._succeedTest()
> else:
> self.stdOutLine
>
> === modified file 'python/
> --- python/
> +++ python/
> @@ -28,7 +28,11 @@
>
>
> class MockTest...
Jonathan Lange (jml) wrote : | # |
Approved with tweaks, see my other comment.
Robert Collins (lifeless) wrote : | # |
On Thu, 2009-08-06 at 09:21 +0000, Jonathan Lange wrote:
>
> > === modified file 'python/
> > --- python/
> > +++ python/
> > @@ -120,7 +120,11 @@
> > self.current_
> > self.state = TestProtocolSer
> > self.current_
> > - self.client.
> > + xfail = getattr(
> 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.
> RemoteError(
>
> 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
1 | === modified file 'NEWS' | |||
2 | --- NEWS 2009-08-02 22:55:36 +0000 | |||
3 | +++ NEWS 2009-08-06 08:48:01 +0000 | |||
4 | @@ -32,6 +32,11 @@ | |||
5 | 32 | 32 | ||
6 | 33 | BUG FIXES: | 33 | BUG FIXES: |
7 | 34 | 34 | ||
8 | 35 | * ``xfail`` outcomes are now passed to python TestResult's via | ||
9 | 36 | addExpectedFailure if it is present on the TestResult. Python 2.6 and | ||
10 | 37 | earlier which do not have this function will have ``xfail`` outcomes | ||
11 | 38 | passed through as success outcomes as earlier versions of subunit did. | ||
12 | 39 | |||
13 | 35 | API CHANGES: | 40 | API CHANGES: |
14 | 36 | 41 | ||
15 | 37 | * When a progress: directive is encountered in a subunit stream, the | 42 | * When a progress: directive is encountered in a subunit stream, the |
16 | 38 | 43 | ||
17 | === modified file 'python/subunit/__init__.py' | |||
18 | --- python/subunit/__init__.py 2009-08-02 22:55:36 +0000 | |||
19 | +++ python/subunit/__init__.py 2009-08-06 08:48:01 +0000 | |||
20 | @@ -120,7 +120,11 @@ | |||
21 | 120 | self.current_test_description == line[offset:-1]): | 120 | self.current_test_description == line[offset:-1]): |
22 | 121 | self.state = TestProtocolServer.OUTSIDE_TEST | 121 | self.state = TestProtocolServer.OUTSIDE_TEST |
23 | 122 | self.current_test_description = None | 122 | self.current_test_description = None |
25 | 123 | self.client.addSuccess(self._current_test) | 123 | xfail = getattr(self.client, 'addExpectedFailure', None) |
26 | 124 | if callable(xfail): | ||
27 | 125 | xfail(self._current_test, RemoteError()) | ||
28 | 126 | else: | ||
29 | 127 | self.client.addSuccess(self._current_test) | ||
30 | 124 | self.client.stopTest(self._current_test) | 128 | self.client.stopTest(self._current_test) |
31 | 125 | elif (self.state == TestProtocolServer.TEST_STARTED and | 129 | elif (self.state == TestProtocolServer.TEST_STARTED and |
32 | 126 | self.current_test_description + " [" == line[offset:-1]): | 130 | self.current_test_description + " [" == line[offset:-1]): |
33 | @@ -203,10 +207,16 @@ | |||
34 | 203 | self.current_test_description = None | 207 | self.current_test_description = None |
35 | 204 | self._skip_or_error(self._message) | 208 | self._skip_or_error(self._message) |
36 | 205 | self.client.stopTest(self._current_test) | 209 | self.client.stopTest(self._current_test) |
41 | 206 | elif self.state in ( | 210 | elif self.state == TestProtocolServer.READING_XFAIL: |
42 | 207 | TestProtocolServer.READING_SUCCESS, | 211 | self.state = TestProtocolServer.OUTSIDE_TEST |
43 | 208 | TestProtocolServer.READING_XFAIL, | 212 | self.current_test_description = None |
44 | 209 | ): | 213 | xfail = getattr(self.client, 'addExpectedFailure', None) |
45 | 214 | if callable(xfail): | ||
46 | 215 | xfail(self._current_test, RemoteError(self._message)) | ||
47 | 216 | else: | ||
48 | 217 | self.client.addSuccess(self._current_test) | ||
49 | 218 | self.client.stopTest(self._current_test) | ||
50 | 219 | elif self.state == TestProtocolServer.READING_SUCCESS: | ||
51 | 210 | self._succeedTest() | 220 | self._succeedTest() |
52 | 211 | else: | 221 | else: |
53 | 212 | self.stdOutLineReceived(line) | 222 | self.stdOutLineReceived(line) |
54 | 213 | 223 | ||
55 | === modified file 'python/subunit/tests/test_test_protocol.py' | |||
56 | --- python/subunit/tests/test_test_protocol.py 2009-08-02 22:55:36 +0000 | |||
57 | +++ python/subunit/tests/test_test_protocol.py 2009-08-06 08:48:01 +0000 | |||
58 | @@ -28,7 +28,11 @@ | |||
59 | 28 | 28 | ||
60 | 29 | 29 | ||
61 | 30 | class MockTestProtocolServerClient(object): | 30 | class MockTestProtocolServerClient(object): |
63 | 31 | """A mock protocol server client to test callbacks.""" | 31 | """A mock protocol server client to test callbacks. |
64 | 32 | |||
65 | 33 | Note that this is deliberately not python 2.7 complete, to allow | ||
66 | 34 | testing compatibility. | ||
67 | 35 | """ | ||
68 | 32 | 36 | ||
69 | 33 | def __init__(self): | 37 | def __init__(self): |
70 | 34 | self.end_calls = [] | 38 | self.end_calls = [] |
71 | @@ -555,41 +559,74 @@ | |||
72 | 555 | class TestTestProtocolServerAddxFail(unittest.TestCase): | 559 | class TestTestProtocolServerAddxFail(unittest.TestCase): |
73 | 556 | """Tests for the xfail keyword. | 560 | """Tests for the xfail keyword. |
74 | 557 | 561 | ||
76 | 558 | In Python this thunks through to Success due to stdlib limitations (see | 562 | In Python this can thunk through to Success due to stdlib limitations (see |
77 | 559 | README). | 563 | README). |
78 | 560 | """ | 564 | """ |
79 | 561 | 565 | ||
83 | 562 | def setUp(self): | 566 | def capture_expected_failure(self, test, err): |
84 | 563 | """Setup a test object ready to be xfailed.""" | 567 | self._calls.append((test, err)) |
85 | 564 | self.client = MockTestProtocolServerClient() | 568 | |
86 | 569 | def setup_python26(self): | ||
87 | 570 | """Setup a test object ready to be xfailed and thunk to success.""" | ||
88 | 571 | self.client = MockTestProtocolServerClient() | ||
89 | 572 | self.setup_protocol() | ||
90 | 573 | |||
91 | 574 | def setup_python27(self): | ||
92 | 575 | """Setup a test object ready to be xfailed and thunk to success.""" | ||
93 | 576 | self.client = MockTestProtocolServerClient() | ||
94 | 577 | self.client.addExpectedFailure = self.capture_expected_failure | ||
95 | 578 | self._calls = [] | ||
96 | 579 | self.setup_protocol() | ||
97 | 580 | |||
98 | 581 | def setup_protocol(self): | ||
99 | 582 | """Setup the protocol based on self.client.""" | ||
100 | 565 | self.protocol = subunit.TestProtocolServer(self.client) | 583 | self.protocol = subunit.TestProtocolServer(self.client) |
101 | 566 | self.protocol.lineReceived("test mcdonalds farm\n") | 584 | self.protocol.lineReceived("test mcdonalds farm\n") |
102 | 567 | self.test = self.client.start_calls[-1] | 585 | self.test = self.client.start_calls[-1] |
103 | 568 | 586 | ||
105 | 569 | def simple_xfail_keyword(self, keyword): | 587 | def simple_xfail_keyword(self, keyword, as_success): |
106 | 570 | self.protocol.lineReceived("%s mcdonalds farm\n" % keyword) | 588 | self.protocol.lineReceived("%s mcdonalds farm\n" % keyword) |
107 | 571 | self.assertEqual(self.client.start_calls, [self.test]) | 589 | self.assertEqual(self.client.start_calls, [self.test]) |
108 | 572 | self.assertEqual(self.client.end_calls, [self.test]) | 590 | self.assertEqual(self.client.end_calls, [self.test]) |
109 | 573 | self.assertEqual(self.client.error_calls, []) | 591 | self.assertEqual(self.client.error_calls, []) |
110 | 574 | self.assertEqual(self.client.failure_calls, []) | 592 | self.assertEqual(self.client.failure_calls, []) |
112 | 575 | self.assertEqual(self.client.success_calls, [self.test]) | 593 | self.check_success_or_xfail(as_success) |
113 | 594 | |||
114 | 595 | def check_success_or_xfail(self, as_success): | ||
115 | 596 | if as_success: | ||
116 | 597 | self.assertEqual(self.client.success_calls, [self.test]) | ||
117 | 598 | else: | ||
118 | 599 | self.assertEqual(1, len(self._calls)) | ||
119 | 600 | self.assertEqual(self.test, self._calls[0][0]) | ||
120 | 576 | 601 | ||
121 | 577 | def test_simple_xfail(self): | 602 | def test_simple_xfail(self): |
123 | 578 | self.simple_xfail_keyword("xfail") | 603 | self.setup_python26() |
124 | 604 | self.simple_xfail_keyword("xfail", True) | ||
125 | 605 | self.setup_python27() | ||
126 | 606 | self.simple_xfail_keyword("xfail", False) | ||
127 | 579 | 607 | ||
128 | 580 | def test_simple_xfail_colon(self): | 608 | def test_simple_xfail_colon(self): |
130 | 581 | self.simple_xfail_keyword("xfail:") | 609 | self.setup_python26() |
131 | 610 | self.simple_xfail_keyword("xfail:", True) | ||
132 | 611 | self.setup_python27() | ||
133 | 612 | self.simple_xfail_keyword("xfail:", False) | ||
134 | 582 | 613 | ||
135 | 583 | def test_xfail_empty_message(self): | 614 | def test_xfail_empty_message(self): |
136 | 615 | self.setup_python26() | ||
137 | 616 | self.empty_message(True) | ||
138 | 617 | self.setup_python27() | ||
139 | 618 | self.empty_message(False) | ||
140 | 619 | |||
141 | 620 | def empty_message(self, as_success): | ||
142 | 584 | self.protocol.lineReceived("xfail mcdonalds farm [\n") | 621 | self.protocol.lineReceived("xfail mcdonalds farm [\n") |
143 | 585 | self.protocol.lineReceived("]\n") | 622 | self.protocol.lineReceived("]\n") |
144 | 586 | self.assertEqual(self.client.start_calls, [self.test]) | 623 | self.assertEqual(self.client.start_calls, [self.test]) |
145 | 587 | self.assertEqual(self.client.end_calls, [self.test]) | 624 | self.assertEqual(self.client.end_calls, [self.test]) |
146 | 588 | self.assertEqual(self.client.error_calls, []) | 625 | self.assertEqual(self.client.error_calls, []) |
147 | 589 | self.assertEqual(self.client.failure_calls, []) | 626 | self.assertEqual(self.client.failure_calls, []) |
149 | 590 | self.assertEqual(self.client.success_calls, [self.test]) | 627 | self.check_success_or_xfail(as_success) |
150 | 591 | 628 | ||
152 | 592 | def xfail_quoted_bracket(self, keyword): | 629 | def xfail_quoted_bracket(self, keyword, as_success): |
153 | 593 | # This tests it is accepted, but cannot test it is used today, because | 630 | # This tests it is accepted, but cannot test it is used today, because |
154 | 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. |
155 | 595 | self.protocol.lineReceived("%s mcdonalds farm [\n" % keyword) | 632 | self.protocol.lineReceived("%s mcdonalds farm [\n" % keyword) |
156 | @@ -599,13 +636,19 @@ | |||
157 | 599 | self.assertEqual(self.client.end_calls, [self.test]) | 636 | self.assertEqual(self.client.end_calls, [self.test]) |
158 | 600 | self.assertEqual(self.client.error_calls, []) | 637 | self.assertEqual(self.client.error_calls, []) |
159 | 601 | self.assertEqual(self.client.failure_calls, []) | 638 | self.assertEqual(self.client.failure_calls, []) |
161 | 602 | self.assertEqual(self.client.success_calls, [self.test]) | 639 | self.check_success_or_xfail(as_success) |
162 | 603 | 640 | ||
163 | 604 | def test_xfail_quoted_bracket(self): | 641 | def test_xfail_quoted_bracket(self): |
165 | 605 | self.xfail_quoted_bracket("xfail") | 642 | self.setup_python26() |
166 | 643 | self.xfail_quoted_bracket("xfail", True) | ||
167 | 644 | self.setup_python27() | ||
168 | 645 | self.xfail_quoted_bracket("xfail", False) | ||
169 | 606 | 646 | ||
170 | 607 | def test_xfail_colon_quoted_bracket(self): | 647 | def test_xfail_colon_quoted_bracket(self): |
172 | 608 | self.xfail_quoted_bracket("xfail:") | 648 | self.setup_python26() |
173 | 649 | self.xfail_quoted_bracket("xfail:", True) | ||
174 | 650 | self.setup_python27() | ||
175 | 651 | self.xfail_quoted_bracket("xfail:", False) | ||
176 | 609 | 652 | ||
177 | 610 | 653 | ||
178 | 611 | class TestTestProtocolServerAddSkip(unittest.TestCase): | 654 | class TestTestProtocolServerAddSkip(unittest.TestCase): |
make xfail use addExpectedFailure