Merge lp:~bjornt/landscape-client/remove-test-mixins into lp:~landscape/landscape-client/trunk

[Björn Tillenius]

Straightforward branch that gets rid of a bunch of mixin test cases and
folds them into the main test cases. The mixins were created during the
smart-to-apt migration to have the same tests being run for both smart
and apt. Now that smart is gone, having them there only makes the tests
harder to understand.

[Alberto Donato (ack)]

Looks good, +1!

#1:
+ # depending on build_skeleton_apt working correctly, which makes
+ # it harder to to TDD for these tests.

typo "to do TDD".

review: Approve

[Free Ekanayaka (free.ekanayaka)]

Looks good to me, +1!

review: Approve

[lp:~bjornt/landscape-client/remove-test-mixins]

577. By Björn Tillenius on 2012-05-16

Typo.

=== modified file 'landscape/package/tests/test_changer.py'
--- landscape/package/tests/test_changer.py	2012-05-16 08:35:27 +0000
+++ landscape/package/tests/test_changer.py	2012-05-16 09:05:23 +0000
@@ -25,7 +25,95 @@
 from landscape.manager.manager import FAILED
 
 
-class PackageChangerTestMixin(object):
+class AptPackageChangerTest(LandscapeTest):
+
+    helpers = [AptFacadeHelper, SimpleRepositoryHelper, BrokerServiceHelper]
+
+    def setUp(self):
+
+        def set_up(ignored):
+
+            self.store = PackageStore(self.makeFile())
+            self.config = PackageChangerConfiguration()
+            self.config.data_path = self.makeDir()
+            os.mkdir(self.config.package_directory)
+            os.mkdir(self.config.binaries_path)
+            touch_file(self.config.update_stamp_filename)
+            self.changer = PackageChanger(
+                self.store, self.facade, self.remote, self.config)
+            service = self.broker_service
+            service.message_store.set_accepted_types(["change-packages-result",
+                                                      "operation-result"])
+
+        result = super(AptPackageChangerTest, self).setUp()
+        return result.addCallback(set_up)
+
+    def set_pkg1_installed(self):
+        """Return the hash of a package that is installed."""
+        self._add_system_package("foo")
+        self.facade.reload_channels()
+        [foo] = self.facade.get_packages_by_name("foo")
+        return self.facade.get_package_hash(foo)
+
+    def set_pkg2_satisfied(self):
+        """Return the hash of a package that can be installed."""
+        self._add_package_to_deb_dir(self.repository_dir, "bar")
+        self.facade.reload_channels()
+        [bar] = self.facade.get_packages_by_name("bar")
+        return self.facade.get_package_hash(bar)
+
+    def set_pkg1_and_pkg2_satisfied(self):
+        """Make a package depend on another package.
+
+        Return the hashes of the two packages.
+        """
+        self._add_package_to_deb_dir(
+            self.repository_dir, "foo", control_fields={"Depends": "bar"})
+        self._add_package_to_deb_dir(self.repository_dir, "bar")
+        self.facade.reload_channels()
+        [foo] = self.facade.get_packages_by_name("foo")
+        [bar] = self.facade.get_packages_by_name("bar")
+        return (
+            self.facade.get_package_hash(foo),
+            self.facade.get_package_hash(bar))
+
+    def set_pkg2_upgrades_pkg1(self):
+        """Make it so that one package upgrades another.
+
+        Return the hashes of the two packages.
+        """
+        self._add_system_package("foo", version="1.0")
+        self._add_package_to_deb_dir(self.repository_dir, "foo", version="2.0")
+        self.facade.reload_channels()
+        foo_1, foo_2 = sorted(self.facade.get_packages_by_name("foo"))
+        return (
+            self.facade.get_package_hash(foo_1),
+            self.facade.get_package_hash(foo_2))
+
+    def remove_pkg2(self):
+        """Remove package name2 from its repository."""
+        packages_file = os.path.join(self.repository_dir, "Packages")
+        packages_contents = read_file(packages_file)
+        packages_contents = "\n\n".join(
+            [stanza for stanza in packages_contents.split("\n\n")
+             if "Package: name2" not in stanza])
+        create_file(packages_file, packages_contents)
+
+    def get_transaction_error_message(self):
+        """Return part of the apt transaction error message."""
+        return "Unable to correct problems"
+
+    def get_binaries_channels(self, binaries_path):
+        """Return the channels that will be used for the binaries."""
+        return [{"baseurl": "file://%s" % binaries_path,
+                 "components": "",
+                 "distribution": "./",
+                 "type": "deb"}]
+
+    def get_package_name(self, version):
+        """Return the name of the package."""
+        return version.package.name
+
 
     def disable_clear_channels(self):
         """Disable clear_channels(), so that it doesn't remove test setup.
@@ -859,95 +947,6 @@
         self.assertFalse(os.path.exists(existing_deb_path))
 
 
-class AptPackageChangerTest(LandscapeTest, PackageChangerTestMixin):
-
-    helpers = [AptFacadeHelper, SimpleRepositoryHelper, BrokerServiceHelper]
-
-    def setUp(self):
-
-        def set_up(ignored):
-
-            self.store = PackageStore(self.makeFile())
-            self.config = PackageChangerConfiguration()
-            self.config.data_path = self.makeDir()
-            os.mkdir(self.config.package_directory)
-            os.mkdir(self.config.binaries_path)
-            touch_file(self.config.update_stamp_filename)
-            self.changer = PackageChanger(
-                self.store, self.facade, self.remote, self.config)
-            service = self.broker_service
-            service.message_store.set_accepted_types(["change-packages-result",
-                                                      "operation-result"])
-
-        result = super(AptPackageChangerTest, self).setUp()
-        return result.addCallback(set_up)
-
-    def set_pkg1_installed(self):
-        """Return the hash of a package that is installed."""
-        self._add_system_package("foo")
-        self.facade.reload_channels()
-        [foo] = self.facade.get_packages_by_name("foo")
-        return self.facade.get_package_hash(foo)
-
-    def set_pkg2_satisfied(self):
-        """Return the hash of a package that can be installed."""
-        self._add_package_to_deb_dir(self.repository_dir, "bar")
-        self.facade.reload_channels()
-        [bar] = self.facade.get_packages_by_name("bar")
-        return self.facade.get_package_hash(bar)
-
-    def set_pkg1_and_pkg2_satisfied(self):
-        """Make a package depend on another package.
-
-        Return the hashes of the two packages.
-        """
-        self._add_package_to_deb_dir(
-            self.repository_dir, "foo", control_fields={"Depends": "bar"})
-        self._add_package_to_deb_dir(self.repository_dir, "bar")
-        self.facade.reload_channels()
-        [foo] = self.facade.get_packages_by_name("foo")
-        [bar] = self.facade.get_packages_by_name("bar")
-        return (
-            self.facade.get_package_hash(foo),
-            self.facade.get_package_hash(bar))
-
-    def set_pkg2_upgrades_pkg1(self):
-        """Make it so that one package upgrades another.
-
-        Return the hashes of the two packages.
-        """
-        self._add_system_package("foo", version="1.0")
-        self._add_package_to_deb_dir(self.repository_dir, "foo", version="2.0")
-        self.facade.reload_channels()
-        foo_1, foo_2 = sorted(self.facade.get_packages_by_name("foo"))
-        return (
-            self.facade.get_package_hash(foo_1),
-            self.facade.get_package_hash(foo_2))
-
-    def remove_pkg2(self):
-        """Remove package name2 from its repository."""
-        packages_file = os.path.join(self.repository_dir, "Packages")
-        packages_contents = read_file(packages_file)
-        packages_contents = "\n\n".join(
-            [stanza for stanza in packages_contents.split("\n\n")
-             if "Package: name2" not in stanza])
-        create_file(packages_file, packages_contents)
-
-    def get_transaction_error_message(self):
-        """Return part of the apt transaction error message."""
-        return "Unable to correct problems"
-
-    def get_binaries_channels(self, binaries_path):
-        """Return the channels that will be used for the binaries."""
-        return [{"baseurl": "file://%s" % binaries_path,
-                 "components": "",
-                 "distribution": "./",
-                 "type": "deb"}]
-
-    def get_package_name(self, version):
-        """Return the name of the package."""
-        return version.package.name
-
     def test_binaries_available_in_cache(self):
         """
         If binaries are included in the changes-packages message, those

=== modified file 'landscape/package/tests/test_reporter.py'
--- landscape/package/tests/test_reporter.py	2012-05-10 00:34:13 +0000
+++ landscape/package/tests/test_reporter.py	2012-05-16 09:05:23 +0000
@@ -40,7 +40,52 @@
         self.assertTrue(config.force_apt_update)
 
 
-class PackageReporterTestMixin(object):
+class PackageReporterAptTest(LandscapeTest):
+
+    helpers = [AptFacadeHelper, SimpleRepositoryHelper, BrokerServiceHelper]
+
+    Facade = AptFacade
+
+    def setUp(self):
+
+        def set_up(ignored):
+            self.store = PackageStore(self.makeFile())
+            self.config = PackageReporterConfiguration()
+            self.reporter = PackageReporter(
+                self.store, self.facade, self.remote, self.config)
+            self.config.data_path = self.makeDir()
+            os.mkdir(self.config.package_directory)
+
+        result = super(PackageReporterAptTest, self).setUp()
+        return result.addCallback(set_up)
+
+    def _clear_repository(self):
+        """Remove all packages from self.repository."""
+        create_file(self.repository_dir + "/Packages", "")
+
+    def set_pkg1_upgradable(self):
+        """Make it so that package "name1" is considered to be upgradable.
+
+        Return the hash of the package that upgrades "name1".
+        """
+        self._add_package_to_deb_dir(
+            self.repository_dir, "name1", version="version2")
+        self.facade.reload_channels()
+        name1_upgrade = sorted(self.facade.get_packages_by_name("name1"))[1]
+        return self.facade.get_package_hash(name1_upgrade)
+
+    def set_pkg1_installed(self):
+        """Make it so that package "name1" is considered installed."""
+        self._install_deb_file(os.path.join(self.repository_dir, PKGNAME1))
+
+    def _make_fake_apt_update(self, out="output", err="error", code=0):
+        """Create a fake apt-update executable"""
+        self.reporter.apt_update_filename = self.makeFile(
+            "#!/bin/sh\n"
+            "echo -n %s\n"
+            "echo -n %s >&2\n"
+            "exit %d" % (out, err, code))
+        os.chmod(self.reporter.apt_update_filename, 0755)
 
     def test_set_package_ids_with_all_known(self):
         self.store.add_hash_id_request(["hash1", "hash2"])
@@ -1059,54 +1104,6 @@
         deferred.addCallback(check_result)
         return deferred
 
-
-class PackageReporterAptTest(LandscapeTest, PackageReporterTestMixin):
-
-    helpers = [AptFacadeHelper, SimpleRepositoryHelper, BrokerServiceHelper]
-
-    Facade = AptFacade
-
-    def setUp(self):
-
-        def set_up(ignored):
-            self.store = PackageStore(self.makeFile())
-            self.config = PackageReporterConfiguration()
-            self.reporter = PackageReporter(
-                self.store, self.facade, self.remote, self.config)
-            self.config.data_path = self.makeDir()
-            os.mkdir(self.config.package_directory)
-
-        result = super(PackageReporterAptTest, self).setUp()
-        return result.addCallback(set_up)
-
-    def _clear_repository(self):
-        """Remove all packages from self.repository."""
-        create_file(self.repository_dir + "/Packages", "")
-
-    def set_pkg1_upgradable(self):
-        """Make it so that package "name1" is considered to be upgradable.
-
-        Return the hash of the package that upgrades "name1".
-        """
-        self._add_package_to_deb_dir(
-            self.repository_dir, "name1", version="version2")
-        self.facade.reload_channels()
-        name1_upgrade = sorted(self.facade.get_packages_by_name("name1"))[1]
-        return self.facade.get_package_hash(name1_upgrade)
-
-    def set_pkg1_installed(self):
-        """Make it so that package "name1" is considered installed."""
-        self._install_deb_file(os.path.join(self.repository_dir, PKGNAME1))
-
-    def _make_fake_apt_update(self, out="output", err="error", code=0):
-        """Create a fake apt-update executable"""
-        self.reporter.apt_update_filename = self.makeFile(
-            "#!/bin/sh\n"
-            "echo -n %s\n"
-            "echo -n %s >&2\n"
-            "exit %d" % (out, err, code))
-        os.chmod(self.reporter.apt_update_filename, 0755)
-
     def test_run_apt_update(self):
         """
         The L{PackageReporter.run_apt_update} method should run apt-update.

=== modified file 'landscape/package/tests/test_skeleton.py'
--- landscape/package/tests/test_skeleton.py	2012-05-08 23:21:07 +0000
+++ landscape/package/tests/test_skeleton.py	2012-05-16 09:05:23 +0000
@@ -36,16 +36,32 @@
             PKGDEB_OR_RELATIONS)
 
 
-class SkeletonTestMixin(object):
-    """Tests for building a skeleton from a package.
-
-    This class should be mixed in to test different backends, like smart
-    and apt.
-
-    The main test case classes need to implement C{get_package(name)} to
-    get a package by name, and C{build_skeleton(package, with_info,
-    with_unicode}, which builds the skeleton.
-    """
+class SkeletonAptTest(LandscapeTest):
+    """C{PackageSkeleton} tests for apt packages."""
+
+    helpers = [AptFacadeHelper, SkeletonTestHelper]
+
+    def setUp(self):
+        super(SkeletonAptTest, self).setUp()
+        self.facade.add_channel_deb_dir(self.skeleton_repository_dir)
+        # Don't use reload_channels(), since that causes the test setup
+        # depending on build_skeleton_apt working correctly, which makes
+        # it harder to do TDD for these tests.
+        self.facade._cache.open(None)
+        self.facade._cache.update(None)
+        self.facade._cache.open(None)
+
+    def get_package(self, name):
+        """Return the package with the specified name."""
+        # Don't use get_packages(), since that causes the test setup
+        # depending on build_skeleton_apt working correctly, which makes
+        # it harder to to TDD for these tests.
+        package = self.facade._cache[name]
+        return package.candidate
+
+    def build_skeleton(self, *args, **kwargs):
+        """Build the skeleton to be tested."""
+        return build_skeleton_apt(*args, **kwargs)
 
     def test_build_skeleton(self):
         """
@@ -242,31 +258,3 @@
             (DEB_UPGRADES, "or-relations < 1.0")]
         self.assertEqual(relations, skeleton.relations)
         self.assertEqual(HASH_OR_RELATIONS, skeleton.get_hash())
-
-
-class SkeletonAptTest(LandscapeTest, SkeletonTestMixin):
-    """C{PackageSkeleton} tests for apt packages."""
-
-    helpers = [AptFacadeHelper, SkeletonTestHelper]
-
-    def setUp(self):
-        super(SkeletonAptTest, self).setUp()
-        self.facade.add_channel_deb_dir(self.skeleton_repository_dir)
-        # Don't use reload_channels(), since that causes the test setup
-        # depending on build_skeleton_apt working correctly, which makes
-        # it harder to to TDD for these tests.
-        self.facade._cache.open(None)
-        self.facade._cache.update(None)
-        self.facade._cache.open(None)
-
-    def get_package(self, name):
-        """Return the package with the specified name."""
-        # Don't use get_packages(), since that causes the test setup
-        # depending on build_skeleton_apt working correctly, which makes
-        # it harder to to TDD for these tests.
-        package = self.facade._cache[name]
-        return package.candidate
-
-    def build_skeleton(self, *args, **kwargs):
-        """Build the skeleton to be tested."""
-        return build_skeleton_apt(*args, **kwargs)