Merge lp:~jml/testtools/more-content-convenience into lp:~testtools-committers/testtools/trunk
- more-content-convenience
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Merge reported by: | Jonathan Lange | ||||
Merged at revision: | not available | ||||
Proposed branch: | lp:~jml/testtools/more-content-convenience | ||||
Merge into: | lp:~testtools-committers/testtools/trunk | ||||
Diff against target: |
545 lines (+293/-43) 9 files modified
NEWS (+4/-0) doc/for-test-authors.rst (+6/-6) testtools/compat.py (+13/-9) testtools/content.py (+115/-1) testtools/deferredruntest.py (+1/-3) testtools/tests/test_content.py (+146/-17) testtools/tests/test_run.py (+5/-4) testtools/tests/test_testresult.py (+1/-3) testtools/tests/test_testtools.py (+2/-0) |
||||
To merge this branch: | bzr merge lp:~jml/testtools/more-content-convenience | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
testtools developers | Pending | ||
Review via email: mp+44870@code.launchpad.net |
Commit message
Description of the change
This branch adds a bunch of convenience methods for the content APIs.
Specifically,
* Content.from_file, which makes a Content object based on file contents
* Content.
* TestCase.
I've also made Content.from_text, and made the text_content function a simple pointer to that.
There's some duplication between from_file and from_stream. Not sure what to do about that.
I added the eager loading (aka read_now) option because there's code in Launchpad that explicitly does eager loading in order to avoid bad interaction between fixture tear down and addDetail. Not sure if it should be the default.
I also don't know what the default chunk size should be. The value I've chosen is almost certainly wrong.
Robert Collins (lifeless) wrote : | # |
Jonathan Lange (jml) wrote : | # |
* I've dropped attachFile. We can add it later if the people clamor for it.
* I don't think they'd be better as stand-alone functions. At the very least, standalone functions can be implemented in terms of classmethods (e.g. content_from_file = Content.from_file), but classmethods can only be built from functions if they drop the class-based dispatch. As far as I can tell, it's a matter of taste. I am willing to budge on this one.
* I've added documentation to from_stream saying that it will close the stream when the iterator is exhausted, and made it actually close the stream. The reasoning is that testtools doesn't really offer any sort of hook for a sensible place to close the stream.
* I've dropped the read_now path from from_stream.
* I've changed the read_now path in from_file to be completely separate from the lazy read path.
* I couldn't get the fixture thing to work.
Unfortunately, without attachFile, we still don't have a nice syntax for the common case of "add this file as a detail during teardown". The best we have is:
self.
As opposed to:
self.
I'm going to go away and think some more.
Robert Collins (lifeless) wrote : | # |
Will look at the code todayish.
On Fri, Dec 31, 2010 at 3:38 AM, Jonathan Lange <email address hidden> wrote:
> * I've dropped attachFile. We can add it later if the people clamor for it.
>
> * I don't think they'd be better as stand-alone functions. At the very least, standalone functions can be implemented in terms of classmethods (e.g. content_from_file = Content.from_file), but classmethods can only be built from functions if they drop the class-based dispatch. As far as I can tell, it's a matter of taste. I am willing to budge on this one.
I think doing 'TracebackConte
I'll see about whipping up a comparitive patch.
> * I've added documentation to from_stream saying that it will close the stream when the iterator is exhausted, and made it actually close the stream. The reasoning is that testtools doesn't really offer any sort of hook for a sensible place to close the stream.
> * I've changed the read_now path in from_file to be completely separate from the lazy read path.
>
> * I couldn't get the fixture thing to work.
What didn't work?
> Unfortunately, without attachFile, we still don't have a nice syntax for the common case of "add this file as a detail during teardown". The best we have is:
> self.addCleanup
>
> As opposed to:
> self.addCleanup
I see, the lambda is needed because we want to force Content.from_file
to execute at cleanup time, not before.
And what we're really saying is 'try to read this thing during
cleanup, and cache the result cause its going to get deleted'.
So there are a few related bits here; the first is that the case I
expect this to happen in, is the case where the file being read is a
log file for a temporary server. I'd expect that server to have a
fixture, and the fixtures /details/ will be automatically captured
during cleanup. So the place that 'attachFile during cleanup' as a
convenience method is most needed is in fixtures. or somewhere where
it is usable by both TestCase and Fixture.
def attach_file(path, obj_with_details, name=None, lazy_read=False):
"""Attach the contents of the file at path to obj_with_details.
This function adds a 'details' object to obj_with_details which will
return the content of path. By default the content is read immediately
and cached.
:param path: The path of the file.
:param obj_with_details: The object to attach the file to -
generally a TestCase or Fixture.
:param name: Optional name for the file. If not specified the
basename is used.
:param lazy_read: If True the file content is not read when
attach_file is called, but later
when the content object is evaluated. Note that this may be
after any cleanups that
lazy_read may cause the file to
be read after it is deleted. To handle those cases, using
attach_file as a cleanup is
obj_with_details)
:return: None
"""
- 179. By Jonathan Lange
-
Merge trunk.
- 180. By Jonathan Lange
-
Copyright bump
- 181. By Jonathan Lange
-
Delete from_text.
- 182. By Jonathan Lange
-
Move Content.from_stream to be content_from_stream
- 183. By Jonathan Lange
-
Fix up the documentation.
- 184. By Jonathan Lange
-
Move attachFile from TestCase to be a standalone function.
- 185. By Jonathan Lange
-
Change the order of parameters to attach_file
- 186. By Jonathan Lange
-
read_now => lazy_read, as per review.
- 187. By Jonathan Lange
-
More tests for attach_file, make lazy read work.
Jonathan Lange (jml) wrote : | # |
I couldn't make the code look better by using fixtures. They just made it more cumbersome.
I've changed the classmethods to be functions (although I still think it looks uglier), and I've changed attachFile to be a standalone function too. I still think TestCase would benefit from a convenience method, but this is still a net improvement.
Robert Collins (lifeless) wrote : | # |
I've push up a riff on your branch - lp:~lifeless/testtools/details - if you like that, just land it. Otherwise lets chat on IRC/voice now that I've refreshed the branch in my head.
Robert Collins (lifeless) wrote : | # |
(if you like my riff, land with the riff)
Jonathan Lange (jml) wrote : | # |
In a nutshell,
* I like content_from_reader
* I think the decorator-
* But I also think it's a bit too clever (it is also missing tests)
Will probably land with your riff sans decorator.
Jonathan Lange (jml) wrote : | # |
Riff (lp:~lifeless/testtools/details) landed sans decorator.
Preview Diff
1 | === modified file 'NEWS' |
2 | --- NEWS 2011-01-22 16:47:31 +0000 |
3 | +++ NEWS 2011-02-01 18:58:11 +0000 |
4 | @@ -24,6 +24,10 @@ |
5 | |
6 | * ``MultiTestResult`` now documented in the manual. (Jonathan Lange, #661116) |
7 | |
8 | +* New content helpers ``content_from_file``, ``content_from_stream`` and |
9 | + ``attach_file`` make it easier to attach file-like objects to a |
10 | + test. (Jonathan Lange, #694126) |
11 | + |
12 | * New ``ExpectedException`` context manager to help write tests against things |
13 | that are expected to raise exceptions. (Aaron Bentley) |
14 | |
15 | |
16 | === modified file 'doc/for-test-authors.rst' |
17 | --- doc/for-test-authors.rst 2011-01-22 16:47:31 +0000 |
18 | +++ doc/for-test-authors.rst 2011-02-01 18:58:11 +0000 |
19 | @@ -737,6 +737,10 @@ |
20 | |
21 | image = Content(ContentType('image', 'png'), lambda: open('foo.png').read()) |
22 | |
23 | +Or you could use the convenience function:: |
24 | + |
25 | + image = content_from_file('foo.png', ContentType('image', 'png')) |
26 | + |
27 | The ``lambda`` helps make sure that the file is opened and the actual bytes |
28 | read only when they are needed – by default, when the test is finished. This |
29 | means that tests can construct and add Content objects freely without worrying |
30 | @@ -754,7 +758,7 @@ |
31 | might do it:: |
32 | |
33 | from testtools import TestCase |
34 | - from testtools.content import Content |
35 | + from testtools.content import attach_file, Content |
36 | from testtools.content_type import UTF8_TEXT |
37 | |
38 | from myproject import SomeServer |
39 | @@ -766,7 +770,7 @@ |
40 | self.server = SomeServer() |
41 | self.server.start_up() |
42 | self.addCleanup(self.server.shut_down) |
43 | - self.addCleanup(self.attach_log_file) |
44 | + self.addCleanup(attach_file, self.server.logfile, self) |
45 | |
46 | def attach_log_file(self): |
47 | self.addDetail( |
48 | @@ -781,10 +785,6 @@ |
49 | run. testtools will only display the log file for failing tests, so it's not |
50 | such a big deal. |
51 | |
52 | -Note the callable passed to ``Content`` reads the log file line-by-line. This |
53 | -is something of an optimization. If we naively returned all of the log file |
54 | -as one bytestring, ``Content`` would treat that as a list of byte chunks. |
55 | - |
56 | If the act of adding at detail is expensive, you might want to use |
57 | addOnException_ so that you only do it when a test actually raises an |
58 | exception. |
59 | |
60 | === modified file 'testtools/compat.py' |
61 | --- testtools/compat.py 2011-01-22 17:56:00 +0000 |
62 | +++ testtools/compat.py 2011-02-01 18:58:11 +0000 |
63 | @@ -2,24 +2,28 @@ |
64 | |
65 | """Compatibility support for python 2 and 3.""" |
66 | |
67 | - |
68 | -import codecs |
69 | -import linecache |
70 | -import locale |
71 | -import os |
72 | -import re |
73 | -import sys |
74 | -import traceback |
75 | - |
76 | __metaclass__ = type |
77 | __all__ = [ |
78 | '_b', |
79 | '_u', |
80 | 'advance_iterator', |
81 | 'str_is_unicode', |
82 | + 'StringIO', |
83 | 'unicode_output_stream', |
84 | ] |
85 | |
86 | +import codecs |
87 | +import linecache |
88 | +import locale |
89 | +import os |
90 | +import re |
91 | +import sys |
92 | +import traceback |
93 | + |
94 | +from testtools.helpers import try_imports |
95 | + |
96 | +StringIO = try_imports(['StringIO.StringIO', 'io.StringIO']) |
97 | + |
98 | |
99 | __u_doc = """A function version of the 'u' prefix. |
100 | |
101 | |
102 | === modified file 'testtools/content.py' |
103 | --- testtools/content.py 2011-01-22 17:56:00 +0000 |
104 | +++ testtools/content.py 2011-02-01 18:58:11 +0000 |
105 | @@ -1,8 +1,18 @@ |
106 | -# Copyright (c) 2009-2010 testtools developers. See LICENSE for details. |
107 | +# Copyright (c) 2009-2011 testtools developers. See LICENSE for details. |
108 | |
109 | """Content - a MIME-like Content object.""" |
110 | |
111 | +__all__ = [ |
112 | + 'attach_file', |
113 | + 'Content', |
114 | + 'content_from_file', |
115 | + 'content_from_stream', |
116 | + 'text_content', |
117 | + 'TracebackContent', |
118 | + ] |
119 | + |
120 | import codecs |
121 | +import os |
122 | |
123 | from testtools.compat import _b |
124 | from testtools.content_type import ContentType, UTF8_TEXT |
125 | @@ -12,6 +22,21 @@ |
126 | _join_b = _b("").join |
127 | |
128 | |
129 | +DEFAULT_CHUNK_SIZE = 4096 |
130 | + |
131 | + |
132 | +def _iter_chunks(stream, chunk_size): |
133 | + """Read 'stream' in chunks of 'chunk_size'. |
134 | + |
135 | + :param stream: A file-like object to read from. |
136 | + :param chunk_size: The size of each read from 'stream'. |
137 | + """ |
138 | + chunk = stream.read(chunk_size) |
139 | + while chunk: |
140 | + yield chunk |
141 | + chunk = stream.read(chunk_size) |
142 | + |
143 | + |
144 | class Content(object): |
145 | """A MIME-like Content object. |
146 | |
147 | @@ -100,3 +125,92 @@ |
148 | This is useful for adding details which are short strings. |
149 | """ |
150 | return Content(UTF8_TEXT, lambda: [text.encode('utf8')]) |
151 | + |
152 | + |
153 | +def content_from_file(path, content_type=None, chunk_size=None, |
154 | + lazy_read=True): |
155 | + """Create a `Content` object from a file on disk. |
156 | + |
157 | + Note that unless 'read_now' is explicitly passed in as True, the file |
158 | + will only be read from when ``iter_bytes`` is called. |
159 | + |
160 | + :param path: The path to the file to be used as content. |
161 | + :param content_type: The type of content. If not specified, defaults |
162 | + to UTF8-encoded text/plain. |
163 | + :param chunk_size: The size of chunks to read from the file. |
164 | + Defaults to `DEFAULT_CHUNK_SIZE`. |
165 | + :param lazy_read: If False, read the file from disk now and keep it in |
166 | + memory. Otherwise, only read when the content is serialized. |
167 | + """ |
168 | + if content_type is None: |
169 | + content_type = UTF8_TEXT |
170 | + if chunk_size is None: |
171 | + chunk_size = DEFAULT_CHUNK_SIZE |
172 | + def reader(): |
173 | + stream = open(path, 'rb') |
174 | + for chunk in _iter_chunks(stream, chunk_size): |
175 | + yield chunk |
176 | + stream.close() |
177 | + if not lazy_read: |
178 | + contents = list(reader()) |
179 | + reader = lambda: contents |
180 | + return Content(content_type, reader) |
181 | + |
182 | + |
183 | +def content_from_stream(stream, content_type=None, chunk_size=None, |
184 | + lazy_read=True): |
185 | + """Create a `Content` object from a file-like stream. |
186 | + |
187 | + Note that the stream will only be read from when ``iter_bytes`` is |
188 | + called. |
189 | + |
190 | + :param stream: A file-like object to read the content from. |
191 | + :param content_type: The type of content. If not specified, defaults |
192 | + to UTF8-encoded text/plain. |
193 | + :param chunk_size: The size of chunks to read from the file. |
194 | + Defaults to `DEFAULT_CHUNK_SIZE`. |
195 | + :param lazy_read: If False, reads from the stream right now. Otherwise, |
196 | + only reads when the content is serialized. Defaults to True. |
197 | + """ |
198 | + if content_type is None: |
199 | + content_type = UTF8_TEXT |
200 | + if chunk_size is None: |
201 | + chunk_size = DEFAULT_CHUNK_SIZE |
202 | + reader = lambda: _iter_chunks(stream, chunk_size) |
203 | + if not lazy_read: |
204 | + contents = list(reader()) |
205 | + reader = lambda: contents |
206 | + return Content(content_type, reader) |
207 | + |
208 | + |
209 | +def attach_file(detailed, path, name=None, content_type=None, |
210 | + chunk_size=None, lazy_read=False): |
211 | + """Attach a file to this test as a detail. |
212 | + |
213 | + This is a convenience method wrapping around `addDetail`. |
214 | + |
215 | + Note that unless 'read_now' is explicitly passed in as True, the file |
216 | + *must* exist when the test result is called with the results of this |
217 | + test, after the test has been torn down. |
218 | + |
219 | + :param detailed: An object with details |
220 | + :param path: The path to the file to attach. |
221 | + :param name: The name to give to the detail for the attached file. |
222 | + :param content_type: The content type of the file. If not provided, |
223 | + defaults to UTF8-encoded text/plain. |
224 | + :param chunk_size: The size of chunks to read from the file. Defaults |
225 | + to something sensible. |
226 | + :param lazy_read: If True the file content is not read when attach_file is |
227 | + called, but later when the content object is evaluated. Note that this |
228 | + may be after any cleanups that obj_with_details has, so if the file is |
229 | + a temporary file lazy_read may cause the file to be read after it is |
230 | + deleted. To handle those cases, using attach_file as a cleanup is |
231 | + recommended:: |
232 | + |
233 | + detailed.addCleanup(attach_file, 'foo.txt', detailed) |
234 | + """ |
235 | + if name is None: |
236 | + name = os.path.basename(os.path.abspath(path)) |
237 | + content_object = content_from_file( |
238 | + path, content_type, chunk_size, lazy_read) |
239 | + detailed.addDetail(name, content_object) |
240 | |
241 | === modified file 'testtools/deferredruntest.py' |
242 | --- testtools/deferredruntest.py 2011-01-22 17:56:00 +0000 |
243 | +++ testtools/deferredruntest.py 2011-02-01 18:58:11 +0000 |
244 | @@ -15,7 +15,7 @@ |
245 | |
246 | import sys |
247 | |
248 | -from testtools import try_imports |
249 | +from testtools.compat import StringIO |
250 | from testtools.content import ( |
251 | Content, |
252 | text_content, |
253 | @@ -34,8 +34,6 @@ |
254 | from twisted.python import log |
255 | from twisted.trial.unittest import _LogObserver |
256 | |
257 | -StringIO = try_imports(['StringIO.StringIO', 'io.StringIO']) |
258 | - |
259 | |
260 | class _DeferredRunTest(RunTest): |
261 | """Base for tests that return Deferreds.""" |
262 | |
263 | === modified file 'testtools/tests/test_content.py' |
264 | --- testtools/tests/test_content.py 2011-01-22 17:56:00 +0000 |
265 | +++ testtools/tests/test_content.py 2011-02-01 18:58:11 +0000 |
266 | @@ -1,11 +1,33 @@ |
267 | -# Copyright (c) 2008-2010 testtools developers. See LICENSE for details. |
268 | +# Copyright (c) 2008-2011 testtools developers. See LICENSE for details. |
269 | |
270 | +import os |
271 | +import tempfile |
272 | import unittest |
273 | + |
274 | from testtools import TestCase |
275 | -from testtools.compat import _b, _u |
276 | -from testtools.content import Content, TracebackContent, text_content |
277 | -from testtools.content_type import ContentType, UTF8_TEXT |
278 | -from testtools.matchers import MatchesException, Raises |
279 | +from testtools.compat import ( |
280 | + _b, |
281 | + _u, |
282 | + StringIO, |
283 | + ) |
284 | +from testtools.content import ( |
285 | + attach_file, |
286 | + Content, |
287 | + content_from_file, |
288 | + content_from_stream, |
289 | + TracebackContent, |
290 | + text_content, |
291 | + ) |
292 | +from testtools.content_type import ( |
293 | + ContentType, |
294 | + UTF8_TEXT, |
295 | + ) |
296 | +from testtools.matchers import ( |
297 | + Equals, |
298 | + MatchesException, |
299 | + Raises, |
300 | + raises, |
301 | + ) |
302 | from testtools.tests.helpers import an_exc_info |
303 | |
304 | |
305 | @@ -15,10 +37,11 @@ |
306 | class TestContent(TestCase): |
307 | |
308 | def test___init___None_errors(self): |
309 | - self.assertThat(lambda:Content(None, None), raises_value_error) |
310 | - self.assertThat(lambda:Content(None, lambda: ["traceback"]), |
311 | - raises_value_error) |
312 | - self.assertThat(lambda:Content(ContentType("text", "traceback"), None), |
313 | + self.assertThat(lambda: Content(None, None), raises_value_error) |
314 | + self.assertThat( |
315 | + lambda: Content(None, lambda: ["traceback"]), raises_value_error) |
316 | + self.assertThat( |
317 | + lambda: Content(ContentType("text", "traceback"), None), |
318 | raises_value_error) |
319 | |
320 | def test___init___sets_ivars(self): |
321 | @@ -64,12 +87,67 @@ |
322 | content = Content(content_type, lambda: [iso_version]) |
323 | self.assertEqual([text], list(content.iter_text())) |
324 | |
325 | + def test_from_file(self): |
326 | + fd, path = tempfile.mkstemp() |
327 | + self.addCleanup(os.remove, path) |
328 | + os.write(fd, 'some data') |
329 | + os.close(fd) |
330 | + content = content_from_file(path, UTF8_TEXT, chunk_size=2) |
331 | + self.assertThat( |
332 | + list(content.iter_bytes()), Equals(['so', 'me', ' d', 'at', 'a'])) |
333 | + |
334 | + def test_from_nonexistent_file(self): |
335 | + directory = tempfile.mkdtemp() |
336 | + nonexistent = os.path.join(directory, 'nonexistent-file') |
337 | + content = content_from_file(nonexistent) |
338 | + self.assertThat(content.iter_bytes, raises(IOError)) |
339 | + |
340 | + def test_from_file_default_type(self): |
341 | + content = content_from_file('/nonexistent/path') |
342 | + self.assertThat(content.content_type, Equals(UTF8_TEXT)) |
343 | + |
344 | + def test_from_file_eager_loading(self): |
345 | + fd, path = tempfile.mkstemp() |
346 | + os.write(fd, 'some data') |
347 | + os.close(fd) |
348 | + content = content_from_file(path, UTF8_TEXT, lazy_read=False) |
349 | + os.remove(path) |
350 | + self.assertThat( |
351 | + _b('').join(content.iter_bytes()), Equals('some data')) |
352 | + |
353 | + def test_from_stream(self): |
354 | + data = StringIO('some data') |
355 | + content = content_from_stream(data, UTF8_TEXT, chunk_size=2) |
356 | + self.assertThat( |
357 | + list(content.iter_bytes()), Equals(['so', 'me', ' d', 'at', 'a'])) |
358 | + |
359 | + def test_from_stream_default_type(self): |
360 | + data = StringIO('some data') |
361 | + content = content_from_stream(data) |
362 | + self.assertThat(content.content_type, Equals(UTF8_TEXT)) |
363 | + |
364 | + def test_from_stream_eager_loading(self): |
365 | + fd, path = tempfile.mkstemp() |
366 | + self.addCleanup(os.remove, path) |
367 | + os.write(fd, 'some data') |
368 | + stream = open(path, 'rb') |
369 | + content = content_from_stream(stream, UTF8_TEXT, lazy_read=False) |
370 | + os.write(fd, 'more data') |
371 | + os.close(fd) |
372 | + self.assertThat( |
373 | + _b('').join(content.iter_bytes()), Equals('some data')) |
374 | + |
375 | + def test_from_text(self): |
376 | + data = _u("some data") |
377 | + expected = Content(UTF8_TEXT, lambda: [data.encode('utf8')]) |
378 | + self.assertEqual(expected, text_content(data)) |
379 | + |
380 | |
381 | class TestTracebackContent(TestCase): |
382 | |
383 | def test___init___None_errors(self): |
384 | - self.assertThat(lambda:TracebackContent(None, None), |
385 | - raises_value_error) |
386 | + self.assertThat( |
387 | + lambda: TracebackContent(None, None), raises_value_error) |
388 | |
389 | def test___init___sets_ivars(self): |
390 | content = TracebackContent(an_exc_info, self) |
391 | @@ -81,12 +159,63 @@ |
392 | self.assertEqual(expected, ''.join(list(content.iter_text()))) |
393 | |
394 | |
395 | -class TestBytesContent(TestCase): |
396 | - |
397 | - def test_bytes(self): |
398 | - data = _u("some data") |
399 | - expected = Content(UTF8_TEXT, lambda: [data.encode('utf8')]) |
400 | - self.assertEqual(expected, text_content(data)) |
401 | +class TestAttachFile(TestCase): |
402 | + |
403 | + def make_file(self, data): |
404 | + fd, path = tempfile.mkstemp() |
405 | + self.addCleanup(os.remove, path) |
406 | + os.write(fd, data) |
407 | + os.close(fd) |
408 | + return path |
409 | + |
410 | + def test_simple(self): |
411 | + class SomeTest(TestCase): |
412 | + def test_foo(self): |
413 | + pass |
414 | + test = SomeTest('test_foo') |
415 | + data = 'some data' |
416 | + path = self.make_file(data) |
417 | + my_content = text_content(data) |
418 | + attach_file(test, path, name='foo') |
419 | + self.assertEqual({'foo': my_content}, test.getDetails()) |
420 | + |
421 | + def test_optional_name(self): |
422 | + # If no name is provided, attach_file just uses the base name of the |
423 | + # file. |
424 | + class SomeTest(TestCase): |
425 | + def test_foo(self): |
426 | + pass |
427 | + test = SomeTest('test_foo') |
428 | + path = self.make_file('some data') |
429 | + base_path = os.path.basename(path) |
430 | + attach_file(test, path) |
431 | + self.assertEqual([base_path], list(test.getDetails())) |
432 | + |
433 | + def test_lazy_read(self): |
434 | + class SomeTest(TestCase): |
435 | + def test_foo(self): |
436 | + pass |
437 | + test = SomeTest('test_foo') |
438 | + path = self.make_file('some data') |
439 | + attach_file(test, path, name='foo', lazy_read=True) |
440 | + content = test.getDetails()['foo'] |
441 | + content_file = open(path, 'w') |
442 | + content_file.write('new data') |
443 | + content_file.close() |
444 | + self.assertEqual(''.join(content.iter_bytes()), 'new data') |
445 | + |
446 | + def test_eager_read_by_default(self): |
447 | + class SomeTest(TestCase): |
448 | + def test_foo(self): |
449 | + pass |
450 | + test = SomeTest('test_foo') |
451 | + path = self.make_file('some data') |
452 | + attach_file(test, path, name='foo') |
453 | + content = test.getDetails()['foo'] |
454 | + content_file = open(path, 'w') |
455 | + content_file.write('new data') |
456 | + content_file.close() |
457 | + self.assertEqual(''.join(content.iter_bytes()), 'some data') |
458 | |
459 | |
460 | def test_suite(): |
461 | |
462 | === modified file 'testtools/tests/test_run.py' |
463 | --- testtools/tests/test_run.py 2011-01-22 17:56:00 +0000 |
464 | +++ testtools/tests/test_run.py 2011-02-01 18:58:11 +0000 |
465 | @@ -2,9 +2,9 @@ |
466 | |
467 | """Tests for the test runner logic.""" |
468 | |
469 | -from testtools.helpers import try_import, try_imports |
470 | +from testtools.compat import StringIO |
471 | +from testtools.helpers import try_import |
472 | fixtures = try_import('fixtures') |
473 | -StringIO = try_imports(['StringIO.StringIO', 'io.StringIO']) |
474 | |
475 | import testtools |
476 | from testtools import TestCase, run |
477 | @@ -41,7 +41,7 @@ |
478 | def test_run_list(self): |
479 | if fixtures is None: |
480 | self.skipTest("Need fixtures") |
481 | - package = self.useFixture(SampleTestFixture()) |
482 | + self.useFixture(SampleTestFixture()) |
483 | out = StringIO() |
484 | run.main(['prog', '-l', 'testtools.runexample.test_suite'], out) |
485 | self.assertEqual("""testtools.runexample.TestFoo.test_bar |
486 | @@ -51,7 +51,7 @@ |
487 | def test_run_load_list(self): |
488 | if fixtures is None: |
489 | self.skipTest("Need fixtures") |
490 | - package = self.useFixture(SampleTestFixture()) |
491 | + self.useFixture(SampleTestFixture()) |
492 | out = StringIO() |
493 | # We load two tests - one that exists and one that doesn't, and we |
494 | # should get the one that exists and neither the one that doesn't nor |
495 | @@ -71,6 +71,7 @@ |
496 | self.assertEqual("""testtools.runexample.TestFoo.test_bar |
497 | """, out.getvalue()) |
498 | |
499 | + |
500 | def test_suite(): |
501 | from unittest import TestLoader |
502 | return TestLoader().loadTestsFromName(__name__) |
503 | |
504 | === modified file 'testtools/tests/test_testresult.py' |
505 | --- testtools/tests/test_testresult.py 2011-01-22 17:56:00 +0000 |
506 | +++ testtools/tests/test_testresult.py 2011-02-01 18:58:11 +0000 |
507 | @@ -22,7 +22,6 @@ |
508 | TextTestResult, |
509 | ThreadsafeForwardingResult, |
510 | testresult, |
511 | - try_imports, |
512 | ) |
513 | from testtools.compat import ( |
514 | _b, |
515 | @@ -30,6 +29,7 @@ |
516 | _r, |
517 | _u, |
518 | str_is_unicode, |
519 | + StringIO, |
520 | ) |
521 | from testtools.content import Content |
522 | from testtools.content_type import ContentType, UTF8_TEXT |
523 | @@ -47,8 +47,6 @@ |
524 | ) |
525 | from testtools.testresult.real import utc |
526 | |
527 | -StringIO = try_imports(['StringIO.StringIO', 'io.StringIO']) |
528 | - |
529 | |
530 | class Python26Contract(object): |
531 | |
532 | |
533 | === modified file 'testtools/tests/test_testtools.py' |
534 | --- testtools/tests/test_testtools.py 2011-01-22 17:56:00 +0000 |
535 | +++ testtools/tests/test_testtools.py 2011-02-01 18:58:11 +0000 |
536 | @@ -3,7 +3,9 @@ |
537 | """Tests for extensions to the base test library.""" |
538 | |
539 | from pprint import pformat |
540 | +import os |
541 | import sys |
542 | +import tempfile |
543 | import unittest |
544 | |
545 | from testtools import ( |
Meta: I think these would be better as stand alone functions rather
than class methods.
> * TestCase. attachFile( name, *args), which is TestCase. addDetail( name, Content. from_file( *args))
I worry a little about expanding the TestCase contract; I won't block
this, but I think it would be clearer not to have an attachFile
method.
> I've also made Content.from_text, and made the text_content function a simple pointer to that.
>
> There's some duplication between from_file and from_stream. Not sure what to do about that.
I would address that by having a 'stream' fixture handed into a common
function, and both delegate to that - one can use a trivial lambda,
the other case opens a file and closes it.
Theres a lifetime ambiguity about from_stream - except for StringIO,
who will close the stream? We should document that, and how to handle
it.
The eager loading thing is fine I think, except that all the other
logic becomes irrelevant so it really should be a separate codepath
for the file case (because chunking on read-now is pointless), and the
stream case can be ignored - clients can call .read() instead - that
will be clearer.
I haven't looked at the tests in detail, I'm sure they will be fine
supporting cases for the changes you've made.
-Rob