← Back to team overview

testtools-dev team mailing list archive

Re: [Merge] lp:~jml/testtools/more-content-convenience into lp:testtools

 

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
-- 
https://code.launchpad.net/~jml/testtools/more-content-convenience/+merge/44870
Your team testtools developers is requested to review the proposed merge of lp:~jml/testtools/more-content-convenience into lp:testtools.



References