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

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

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.

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.

review: Approve

« Back to merge proposal