Merge lp:~james-w/linaro-image-tools/tarfile-matchers into lp:linaro-image-tools/11.11

[James Westby]

Hi,

This adds some matchers that I will use for testing the output of
hwpack-create. It's a lot easier to read and write the tests with this
style, rather than exercising the tarfile API directly.

I don't much like these tests though, suggestions for how to improve them
welcome.

Thanks,

James

[Michael Hudson-Doyle (mwhudson)]

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

[lp:~james-w/linaro-image-tools/tarfile-matchers]

77. By James Westby on 2010-08-31

Merged tarfile-improvements into tarfile-matchers.

78. By James Westby on 2010-08-31

Merged tarfile-improvements into tarfile-matchers.

79. By James Westby on 2010-08-31

Merged tarfile-improvements into tarfile-matchers.

80. By James Westby on 2010-08-31

Merged tarfile-improvements into tarfile-matchers.

81. By James Westby on 2010-08-31

Reduce duplication in the tests. Thanks Michael.

82. By James Westby on 2010-08-31

Add __eq__ and __hash__ on the mismatches.

83. By James Westby on 2010-08-31

Take advantage of the new __eq__ when checking TarfileWrongValueMismatch.

84. By James Westby on 2010-08-31

Also test the values set on TarfileMissingPathMismatch.

[James Westby (james-w)]

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

[Michael Hudson-Doyle (mwhudson)]

I think the test_tarfile helper has improved things.

For the second of my suggestions, I actually meant constructing the mismatch in the test and then using assertEquals. But I don't know if that would be any better either (probably not).

I feel bad for causing you to write all those hash and eq tests!

[lp:~james-w/linaro-image-tools/tarfile-matchers]

85. By James Westby on 2010-08-31

Merged tarfile-improvements into tarfile-matchers.

=== added file 'hwpack/tarfile_matchers.py'
--- hwpack/tarfile_matchers.py	1970-01-01 00:00:00 +0000
+++ hwpack/tarfile_matchers.py	2010-09-01 20:48:43 +0000
@@ -0,0 +1,130 @@
+from testtools.matchers import Matcher, Mismatch
+
+
+class TarfileMissingPathMismatch(Mismatch):
+    """A Mismatch indicating that a required path was missing from a tarfile.
+    """
+
+    def __init__(self, tarball, path):
+        """Create a TarfileMissingPathMismatch Mismatch.
+
+        :param tarball: the tarfile that was checked.
+        :param path: the path that was expected to be present.
+        """
+        self.tarball = tarball
+        self.path = path
+
+    def describe(self):
+        return '"%s" has no path "%s"' % (self.tarball, self.path)
+
+    def __eq__(self, other):
+        return self.tarball == other.tarball and self.path == other.path
+
+    def __hash__(self):
+        return hash((self.tarball, self.path))
+
+
+class TarfileWrongValueMismatch(Mismatch):
+    """A Mismatch indicating that an entry in a tarfile was not as expected.
+    """
+
+    def __init__(self, attribute, tarball, path, expected, actual):
+        """Create a TarfileWrongValueMismatch Mismatch.
+
+        :param attribute: the attribute that was not as expected.
+        :type attribute: str
+        :param tarball: the tarfile that was checked.
+        :param path: the path that was checked.
+        :param expected: the expected value of the attribute.
+        :param actual: the value that was found.
+        """
+        self.attribute = attribute
+        self.tarball = tarball
+        self.path = path
+        self.expected = expected
+        self.actual = actual
+
+    def describe(self):
+        return 'The path "%s" in "%s" has %s %s, expected %s' % (
+            self.path, self.tarball, self.attribute, self.actual,
+            self.expected)
+
+    def __eq__(self, other):
+        return (self.attribute == other.attribute
+                and self.tarball == other.tarball
+                and self.path == other.path
+                and self.expected == other.expected
+                and self.actual == other.actual)
+
+    def __hash__(self):
+        return hash(
+            (self.attribute, self.tarball, self.path, self.expected,
+             self.actual))
+
+
+class TarfileHasFile(Matcher):
+    """Check that a tarfile has an entry with certain values."""
+
+    def __init__(self, path, type=None, size=None, mtime=None, mode=None,
+                 linkname=None, uid=None, gid=None, uname=None, gname=None,
+                 content=None):
+        """Create a TarfileHasFile Matcher.
+
+        :param path: the path that must be present.
+        :type path: str
+        :param type: the type of TarInfo that must be at `path`, or None
+            to not check.
+        :param size: the size that the entry at `path` must have, or None
+            to not check.
+        :param mtime: the mtime that the entry at `path` must have, or None
+            to not check.
+        :param mode: the mode that the entry at `path` must have, or None
+            to not check.
+        :param linkname: the linkname that the entry at `path` must have,
+            or None to not check.
+        :param uid: the user id that the entry at `path` must have, or None
+            to not check.
+        :param gid: the group id that the entry at `path` must have, or None
+            to not check.
+        :param uname: the username that the entry at `path` must have, or
+            None to not check.
+        :param gname: the group name that the entry at `path` must have, or
+            None to not check.
+        :param content: the content that `path` must have when extracted,
+            or None to not check.
+        """
+        self.path = path
+        self.type = type
+        self.size = size
+        self.mtime = mtime
+        self.mode = mode
+        self.linkname = linkname
+        self.uid = uid
+        self.gid = gid
+        self.uname = uname
+        self.gname = gname
+        self.content = content
+
+    def match(self, tarball):
+        """Match a tarfile.TarFile against the expected values."""
+        if self.path not in tarball.getnames():
+            return TarfileMissingPathMismatch(tarball, self.path)
+        info = tarball.getmember(self.path)
+        for attr in (
+            "type", "size", "mtime", "mode", "linkname", "uid", "gid",
+            "uname", "gname"):
+            expected = getattr(self, attr, None)
+            if expected is not None:
+                actual = getattr(info, attr)
+                if expected != actual:
+                    return TarfileWrongValueMismatch(
+                        attr, tarball, self.path, expected, actual)
+        if self.content is not None:
+            actual = tarball.extractfile(self.path).read()
+            if actual != self.content:
+                return TarfileWrongValueMismatch(
+                    "content", tarball, self.path, self.content, actual)
+        return None
+
+    def __str__(self):
+        return 'tarfile has file "%s"' % (self.path, )

=== added file 'hwpack/testing.py'
--- hwpack/testing.py	1970-01-01 00:00:00 +0000
+++ hwpack/testing.py	2010-09-01 20:48:43 +0000
@@ -0,0 +1,39 @@
+from contextlib import contextmanager
+from StringIO import StringIO
+import tarfile
+
+from hwpack.better_tarfile import writeable_tarfile
+
+
+@contextmanager
+def test_tarfile(contents=[], **kwargs):
+    """Create a tarfile with the given contents, then re-open it for reading.
+
+    This context manager creates a tarfile with the given contents, then
+    reopens it for reading and yields it for use in a with block. When
+    the block ends the tarfile will be closed.
+
+    The contents can be specified as a list of tuples of (path, contents),
+    where if the path ends with '/' it is considered to be a directory and
+    the contents ignored.
+
+    :param contents: the contents to put in the tarball, defaults to the
+        empty list.
+    :type contents: a list of tuples of (str, str)
+    :param kwargs: keyword arguments for the better_tarfile.TarFile
+        constructor.
+    """
+    backing_file = StringIO()
+    with writeable_tarfile(backing_file, **kwargs) as tf:
+        for path, content in contents:
+            if path[-1] == "/":
+                tf.create_dir(path)
+            else:
+                tf.create_file_from_string(path, content)
+    if contents:
+        backing_file.seek(0)
+    tf = tarfile.TarFile.open(mode="r", fileobj=backing_file)
+    try:
+        yield tf
+    finally:
+        tf.close()

=== modified file 'hwpack/tests/__init__.py'
--- hwpack/tests/__init__.py	2010-09-01 20:48:43 +0000
+++ hwpack/tests/__init__.py	2010-09-01 20:48:43 +0000
@@ -3,6 +3,7 @@
 def test_suite():
     module_names = ['hwpack.tests.test_config',
                     'hwpack.tests.test_better_tarfile',
+                    'hwpack.tests.test_tarfile_matchers',
                    ]
     loader = unittest.TestLoader()
     suite = loader.loadTestsFromNames(module_names)

=== added file 'hwpack/tests/test_tarfile_matchers.py'
--- hwpack/tests/test_tarfile_matchers.py	1970-01-01 00:00:00 +0000
+++ hwpack/tests/test_tarfile_matchers.py	2010-09-01 20:48:43 +0000
@@ -0,0 +1,228 @@
+from StringIO import StringIO
+import tarfile
+
+from testtools import TestCase
+
+from hwpack.tarfile_matchers import (
+    TarfileHasFile,
+    TarfileMissingPathMismatch,
+    TarfileWrongValueMismatch,
+    )
+from hwpack.testing import test_tarfile
+
+
+class TarfileMissingPathMismatchTests(TestCase):
+
+    def test_describe(self):
+        mismatch = TarfileMissingPathMismatch("foo", "bar")
+        self.assertEqual('"foo" has no path "bar"', mismatch.describe())
+
+    def test_eq(self):
+        mismatch1 = TarfileMissingPathMismatch("foo", "bar")
+        mismatch2 = TarfileMissingPathMismatch("foo", "bar")
+        self.assertEqual(mismatch1, mismatch2)
+
+    def test_no_eq_different_tarball(self):
+        mismatch1 = TarfileMissingPathMismatch("foo", "bar")
+        mismatch2 = TarfileMissingPathMismatch("baz", "bar")
+        self.assertNotEqual(mismatch1, mismatch2)
+
+    def test_no_eq_different_path(self):
+        mismatch1 = TarfileMissingPathMismatch("foo", "bar")
+        mismatch2 = TarfileMissingPathMismatch("foo", "baz")
+        self.assertNotEqual(mismatch1, mismatch2)
+
+    def test_hash_equal(self):
+        mismatch1 = TarfileMissingPathMismatch("foo", "bar")
+        mismatch2 = TarfileMissingPathMismatch("foo", "bar")
+        self.assertEqual(hash(mismatch1), hash(mismatch2))
+
+    def test_different_tarball_different_hash(self):
+        mismatch1 = TarfileMissingPathMismatch("foo", "bar")
+        mismatch2 = TarfileMissingPathMismatch("baz", "bar")
+        self.assertNotEqual(hash(mismatch1), hash(mismatch2))
+
+    def test_different_path_different_hash(self):
+        mismatch1 = TarfileMissingPathMismatch("foo", "bar")
+        mismatch2 = TarfileMissingPathMismatch("foo", "baz")
+        self.assertNotEqual(hash(mismatch1), hash(mismatch2))
+
+
+class TarfileWrongTypeMismatchTests(TestCase):
+
+    def test_describe(self):
+        mismatch = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        self.assertEqual(
+            'The path "bar" in "foo" has type 2, expected 1',
+            mismatch.describe())
+
+    def test_eq(self):
+        mismatch1 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        mismatch2 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        self.assertEqual(mismatch1, mismatch2)
+
+    def test_not_eq_different_attribute(self):
+        mismatch1 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        mismatch2 = TarfileWrongValueMismatch("size", "foo", "bar", 1, 2)
+        self.assertNotEqual(mismatch1, mismatch2)
+
+    def test_not_eq_different_tarball(self):
+        mismatch1 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        mismatch2 = TarfileWrongValueMismatch("type", "baz", "bar", 1, 2)
+        self.assertNotEqual(mismatch1, mismatch2)
+
+    def test_not_eq_different_path(self):
+        mismatch1 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        mismatch2 = TarfileWrongValueMismatch("type", "foo", "baz", 1, 2)
+        self.assertNotEqual(mismatch1, mismatch2)
+
+    def test_not_eq_different_expected(self):
+        mismatch1 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        mismatch2 = TarfileWrongValueMismatch("type", "foo", "bar", 3, 2)
+        self.assertNotEqual(mismatch1, mismatch2)
+
+    def test_not_eq_different_actual(self):
+        mismatch1 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        mismatch2 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 3)
+        self.assertNotEqual(mismatch1, mismatch2)
+
+    def test_hash_equal(self):
+        mismatch1 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        mismatch2 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        self.assertEqual(hash(mismatch1), hash(mismatch2))
+
+    def test_different_attribute_different_hash(self):
+        mismatch1 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        mismatch2 = TarfileWrongValueMismatch("size", "foo", "bar", 1, 2)
+        self.assertNotEqual(hash(mismatch1), hash(mismatch2))
+
+    def test_different_tarball_different_hash(self):
+        mismatch1 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        mismatch2 = TarfileWrongValueMismatch("type", "baz", "bar", 1, 2)
+        self.assertNotEqual(hash(mismatch1), hash(mismatch2))
+
+    def test_different_path_different_hash(self):
+        mismatch1 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        mismatch2 = TarfileWrongValueMismatch("type", "foo", "baz", 1, 2)
+        self.assertNotEqual(hash(mismatch1), hash(mismatch2))
+
+    def test_different_expected_different_hash(self):
+        mismatch1 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        mismatch2 = TarfileWrongValueMismatch("type", "foo", "bar", 3, 2)
+        self.assertNotEqual(hash(mismatch1), hash(mismatch2))
+
+    def test_different_actual_different_hash(self):
+        mismatch1 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 2)
+        mismatch2 = TarfileWrongValueMismatch("type", "foo", "bar", 1, 3)
+        self.assertNotEqual(hash(mismatch1), hash(mismatch2))
+
+
+class TarfileHasFileTests(TestCase):
+
+    def test_str(self):
+        matcher = TarfileHasFile("foo")
+        self.assertEqual('tarfile has file "foo"', str(matcher))
+
+    def test_matches(self):
+        backing_file = StringIO()
+        with test_tarfile(contents=[("foo", "")]) as tf:
+            matcher = TarfileHasFile("foo")
+            self.assertIs(None, matcher.match(tf))
+
+    def test_mismatches_missing_path(self):
+        backing_file = StringIO()
+        with test_tarfile() as tf:
+            matcher = TarfileHasFile("foo")
+            mismatch = matcher.match(tf)
+            self.assertIsInstance(mismatch, TarfileMissingPathMismatch)
+            self.assertEqual(TarfileMissingPathMismatch(tf, "foo"), mismatch)
+
+    def assertValueMismatch(self, mismatch, tarball, path, attribute,
+                            expected, actual):
+        self.assertIsInstance(mismatch, TarfileWrongValueMismatch)
+        expected_mismatch = TarfileWrongValueMismatch(
+            attribute, tarball, path, expected, actual)
+        self.assertEqual(expected_mismatch, mismatch)
+
+    def test_mismatches_wrong_type(self):
+        backing_file = StringIO()
+        with test_tarfile(contents=[("foo", "")]) as tf:
+            matcher = TarfileHasFile("foo", type=tarfile.DIRTYPE)
+            mismatch = matcher.match(tf)
+            self.assertValueMismatch(
+                mismatch, tf, "foo", "type", tarfile.DIRTYPE,
+                tarfile.REGTYPE)
+
+    def test_mismatches_wrong_size(self):
+        backing_file = StringIO()
+        with test_tarfile(contents=[("foo", "")]) as tf:
+            matcher = TarfileHasFile("foo", size=1235)
+            mismatch = matcher.match(tf)
+            self.assertValueMismatch(
+                mismatch, tf, "foo", "size", 1235, 0)
+
+    def test_mismatches_wrong_mtime(self):
+        backing_file = StringIO()
+        with test_tarfile(contents=[("foo", "")], default_mtime=12345) as tf:
+            matcher = TarfileHasFile("foo", mtime=54321)
+            mismatch = matcher.match(tf)
+            self.assertValueMismatch(
+                mismatch, tf, "foo", "mtime", 54321, 12345)
+
+    def test_mismatches_wrong_mode(self):
+        backing_file = StringIO()
+        with test_tarfile(contents=[("foo", "")]) as tf:
+            matcher = TarfileHasFile("foo", mode=0000)
+            mismatch = matcher.match(tf)
+            self.assertValueMismatch(
+                mismatch, tf, "foo", "mode", 0000, 0644)
+
+    def test_mismatches_wrong_linkname(self):
+        backing_file = StringIO()
+        with test_tarfile(contents=[("foo", "")]) as tf:
+            matcher = TarfileHasFile("foo", linkname="somelink")
+            mismatch = matcher.match(tf)
+            self.assertValueMismatch(
+                mismatch, tf, "foo", "linkname", "somelink", "")
+
+    def test_mismatches_wrong_uid(self):
+        backing_file = StringIO()
+        with test_tarfile(contents=[("foo", "")], default_uid=100) as tf:
+            matcher = TarfileHasFile("foo", uid=99)
+            mismatch = matcher.match(tf)
+            self.assertValueMismatch(
+                mismatch, tf, "foo", "uid", 99, 100)
+
+    def test_mismatches_wrong_gid(self):
+        backing_file = StringIO()
+        with test_tarfile(contents=[("foo", "")], default_gid=100) as tf:
+            matcher = TarfileHasFile("foo", gid=99)
+            mismatch = matcher.match(tf)
+            self.assertValueMismatch(
+                mismatch, tf, "foo", "gid", 99, 100)
+
+    def test_mismatches_wrong_uname(self):
+        backing_file = StringIO()
+        with test_tarfile(
+            contents=[("foo", "")], default_uname="someuser") as tf:
+            matcher = TarfileHasFile("foo", uname="otheruser")
+            mismatch = matcher.match(tf)
+            self.assertValueMismatch(
+                mismatch, tf, "foo", "uname", "otheruser", "someuser")
+
+    def test_mismatches_wrong_gname(self):
+        backing_file = StringIO()
+        with test_tarfile(
+            contents=[("foo", "")], default_gname="somegroup") as tf:
+            matcher = TarfileHasFile("foo", gname="othergroup")
+            mismatch = matcher.match(tf)
+            self.assertValueMismatch(
+                mismatch, tf, "foo", "gname", "othergroup", "somegroup")
+
+    def test_mismatches_wrong_content(self):
+        backing_file = StringIO()
+        with test_tarfile(contents=[("foo", "somecontent")]) as tf:
+            matcher = TarfileHasFile("foo", content="othercontent")
+            mismatch = matcher.match(tf)
+            self.assertValueMismatch(
+                mismatch, tf, "foo", "content", "othercontent", "somecontent")