subunit

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

  1. xfail
  2. Merge into 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
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
6 BUG FIXES:
7
8+ * ``xfail`` outcomes are now passed to python TestResult's via
9+ addExpectedFailure if it is present on the TestResult. Python 2.6 and
10+ earlier which do not have this function will have ``xfail`` outcomes
11+ passed through as success outcomes as earlier versions of subunit did.
12+
13 API CHANGES:
14
15 * When a progress: directive is encountered in a subunit stream, the
16
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 self.current_test_description == line[offset:-1]):
22 self.state = TestProtocolServer.OUTSIDE_TEST
23 self.current_test_description = None
24- self.client.addSuccess(self._current_test)
25+ xfail = getattr(self.client, 'addExpectedFailure', None)
26+ if callable(xfail):
27+ xfail(self._current_test, RemoteError())
28+ else:
29+ self.client.addSuccess(self._current_test)
30 self.client.stopTest(self._current_test)
31 elif (self.state == TestProtocolServer.TEST_STARTED and
32 self.current_test_description + " [" == line[offset:-1]):
33@@ -203,10 +207,16 @@
34 self.current_test_description = None
35 self._skip_or_error(self._message)
36 self.client.stopTest(self._current_test)
37- elif self.state in (
38- TestProtocolServer.READING_SUCCESS,
39- TestProtocolServer.READING_XFAIL,
40- ):
41+ elif self.state == TestProtocolServer.READING_XFAIL:
42+ self.state = TestProtocolServer.OUTSIDE_TEST
43+ self.current_test_description = None
44+ xfail = getattr(self.client, 'addExpectedFailure', None)
45+ if callable(xfail):
46+ xfail(self._current_test, RemoteError(self._message))
47+ else:
48+ self.client.addSuccess(self._current_test)
49+ self.client.stopTest(self._current_test)
50+ elif self.state == TestProtocolServer.READING_SUCCESS:
51 self._succeedTest()
52 else:
53 self.stdOutLineReceived(line)
54
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
60
61 class MockTestProtocolServerClient(object):
62- """A mock protocol server client to test callbacks."""
63+ """A mock protocol server client to test callbacks.
64+
65+ Note that this is deliberately not python 2.7 complete, to allow
66+ testing compatibility.
67+ """
68
69 def __init__(self):
70 self.end_calls = []
71@@ -555,41 +559,74 @@
72 class TestTestProtocolServerAddxFail(unittest.TestCase):
73 """Tests for the xfail keyword.
74
75- In Python this thunks through to Success due to stdlib limitations (see
76+ In Python this can thunk through to Success due to stdlib limitations (see
77 README).
78 """
79
80- def setUp(self):
81- """Setup a test object ready to be xfailed."""
82- self.client = MockTestProtocolServerClient()
83+ def capture_expected_failure(self, test, err):
84+ self._calls.append((test, err))
85+
86+ def setup_python26(self):
87+ """Setup a test object ready to be xfailed and thunk to success."""
88+ self.client = MockTestProtocolServerClient()
89+ self.setup_protocol()
90+
91+ def setup_python27(self):
92+ """Setup a test object ready to be xfailed and thunk to success."""
93+ self.client = MockTestProtocolServerClient()
94+ self.client.addExpectedFailure = self.capture_expected_failure
95+ self._calls = []
96+ self.setup_protocol()
97+
98+ def setup_protocol(self):
99+ """Setup the protocol based on self.client."""
100 self.protocol = subunit.TestProtocolServer(self.client)
101 self.protocol.lineReceived("test mcdonalds farm\n")
102 self.test = self.client.start_calls[-1]
103
104- def simple_xfail_keyword(self, keyword):
105+ def simple_xfail_keyword(self, keyword, as_success):
106 self.protocol.lineReceived("%s mcdonalds farm\n" % keyword)
107 self.assertEqual(self.client.start_calls, [self.test])
108 self.assertEqual(self.client.end_calls, [self.test])
109 self.assertEqual(self.client.error_calls, [])
110 self.assertEqual(self.client.failure_calls, [])
111- self.assertEqual(self.client.success_calls, [self.test])
112+ self.check_success_or_xfail(as_success)
113+
114+ def check_success_or_xfail(self, as_success):
115+ if as_success:
116+ self.assertEqual(self.client.success_calls, [self.test])
117+ else:
118+ self.assertEqual(1, len(self._calls))
119+ self.assertEqual(self.test, self._calls[0][0])
120
121 def test_simple_xfail(self):
122- self.simple_xfail_keyword("xfail")
123+ self.setup_python26()
124+ self.simple_xfail_keyword("xfail", True)
125+ self.setup_python27()
126+ self.simple_xfail_keyword("xfail", False)
127
128 def test_simple_xfail_colon(self):
129- self.simple_xfail_keyword("xfail:")
130+ self.setup_python26()
131+ self.simple_xfail_keyword("xfail:", True)
132+ self.setup_python27()
133+ self.simple_xfail_keyword("xfail:", False)
134
135 def test_xfail_empty_message(self):
136+ self.setup_python26()
137+ self.empty_message(True)
138+ self.setup_python27()
139+ self.empty_message(False)
140+
141+ def empty_message(self, as_success):
142 self.protocol.lineReceived("xfail mcdonalds farm [\n")
143 self.protocol.lineReceived("]\n")
144 self.assertEqual(self.client.start_calls, [self.test])
145 self.assertEqual(self.client.end_calls, [self.test])
146 self.assertEqual(self.client.error_calls, [])
147 self.assertEqual(self.client.failure_calls, [])
148- self.assertEqual(self.client.success_calls, [self.test])
149+ self.check_success_or_xfail(as_success)
150
151- def xfail_quoted_bracket(self, keyword):
152+ def xfail_quoted_bracket(self, keyword, as_success):
153 # This tests it is accepted, but cannot test it is used today, because
154 # of not having a way to expose it in Python so far.
155 self.protocol.lineReceived("%s mcdonalds farm [\n" % keyword)
156@@ -599,13 +636,19 @@
157 self.assertEqual(self.client.end_calls, [self.test])
158 self.assertEqual(self.client.error_calls, [])
159 self.assertEqual(self.client.failure_calls, [])
160- self.assertEqual(self.client.success_calls, [self.test])
161+ self.check_success_or_xfail(as_success)
162
163 def test_xfail_quoted_bracket(self):
164- self.xfail_quoted_bracket("xfail")
165+ self.setup_python26()
166+ self.xfail_quoted_bracket("xfail", True)
167+ self.setup_python27()
168+ self.xfail_quoted_bracket("xfail", False)
169
170 def test_xfail_colon_quoted_bracket(self):
171- self.xfail_quoted_bracket("xfail:")
172+ self.setup_python26()
173+ self.xfail_quoted_bracket("xfail:", True)
174+ self.setup_python27()
175+ self.xfail_quoted_bracket("xfail:", False)
176
177
178 class TestTestProtocolServerAddSkip(unittest.TestCase):

Subscribers

People subscribed via source and target branches