Code review comment for lp:~james-w/linaro-image-tools/tarfile-matchers

Revision history for this message
James Westby (james-w) wrote :

On Tue, 31 Aug 2010 02:44:41 -0000, Michael Hudson <email address hidden> wrote:
> Review: Approve
> It looks mostly fine. I agree the tests are a bit horrible.
>
> Could you fold this sort of thing:
>
> + backing_file = StringIO()
> + with writeable_tarfile(backing_file, default_mtime=12345) as tf:
> + tf.create_file_from_string("foo", "")
> + with standard_tarfile(backing_file) as tf:
>
> into some kind of context manager that would let you write:
>
> with test_tarfile(default_mtime=12345, contents=[('foo', '')]) as tf:
>
> ? That would help a bit.

Done, thanks for the suggestion.

> As for the assertions, could you construct a mismatch using keyword
> arguments and then test that the mismatches match? I don't know if
> that would be better, but it seems worth a try.

I took a stab at this, let me know what you think now.

Thanks,

James

« Back to merge proposal