[master 3/5] Simplify and correct StorageDevice.packages property.

mulkieran installerbot-noreply at redhat.com
Wed Mar 4 01:53:40 UTC 2015


From: mulhern <amulhern at redhat.com>

It's an error to check the parent's format's packages in the loop,
just because it's possible to check self's format's packages.

The for statement was a null op, because all the packages that it considered
had already been added by the magic of virtual method invocation.
This would not be the case if any device classes extended Device w/out
extending StorageDevice...in that case there might have been an
AttributeError due to a missing format attribuate.

packages is not supposed to have duplicates, so make sure not to add
duplicates when adding format's packages.

So, StorageDevice.packages might previously have had duplicates, but now
it should not, as was clearly originally intended.

Update Device.packages so that method docstring is correct and simplify
body slightly.

Signed-off-by: mulhern <amulhern at redhat.com>
---
 blivet/devices/device.py                   | 10 ++++-----
 blivet/devices/storage.py                  | 14 +-----------
 tests/devices_test/device_packages_test.py | 34 ++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 18 deletions(-)
 create mode 100644 tests/devices_test/device_packages_test.py

diff --git a/blivet/devices/device.py b/blivet/devices/device.py
index d478fed..90a2718 100644
--- a/blivet/devices/device.py
+++ b/blivet/devices/device.py
@@ -276,15 +276,15 @@ def ancestors(self):
 
     @property
     def packages(self):
-        """ List of packages required to manage devices of this type.
+        """ List of packages required to manage this device and all its
+            ancestor devices. Does not contain duplicates.
 
-            This list includes the packages required by its parent devices.
+            :returns: names of packages required by device and all ancestors
+            :rtype: list of str
         """
         packages = self._packages
         for parent in self.parents:
-            for package in parent.packages:
-                if package not in packages:
-                    packages.append(package)
+            packages.extend(p for p in parent.packages if p not in packages)
 
         return packages
 
diff --git a/blivet/devices/storage.py b/blivet/devices/storage.py
index 61b5733..f5c7c11 100644
--- a/blivet/devices/storage.py
+++ b/blivet/devices/storage.py
@@ -148,20 +148,8 @@ def __str__(self):
 
     @property
     def packages(self):
-        """ List of packages required to manage devices of this type.
-
-            This list includes the packages required by this device's
-            format type as well those required by all of its parent
-            devices.
-        """
         packages = super(StorageDevice, self).packages
-        packages.extend(self.format.packages)
-        for parent in self.parents:
-            for package in parent.format.packages:
-                if package not in packages:
-                    packages.append(package)
-
-        return packages
+        return packages + [p for p in self.format.packages if p not in packages]
 
     @property
     def disks(self):
diff --git a/tests/devices_test/device_packages_test.py b/tests/devices_test/device_packages_test.py
new file mode 100644
index 0000000..a3295f5
--- /dev/null
+++ b/tests/devices_test/device_packages_test.py
@@ -0,0 +1,34 @@
+#!/usr/bin/python
+# vim:set fileencoding=utf-8
+
+import unittest
+
+from blivet.devices import DiskDevice
+from blivet.devices import LUKSDevice
+from blivet.devices import MDRaidArrayDevice
+
+from blivet.formats import getFormat
+
+class DevicePackagesTestCase(unittest.TestCase):
+    """Test device name validation"""
+
+    def testPackages(self):
+        dev1 = DiskDevice("name", fmt=getFormat("mdmember"))
+
+        dev2 = DiskDevice("other", fmt=getFormat("mdmember"))
+        dev = MDRaidArrayDevice("dev", level="raid1", parents=[dev1,dev2])
+        luks = LUKSDevice("luks", parents=[dev])
+        packages = luks.packages
+
+        # no duplicates in list of packages
+        self.assertListEqual(packages, list(set(packages)))
+
+        # several packages that ought to be included are
+        for package in dev1.packages + dev2.packages + dev.packages:
+            self.assertIn(package, packages)
+
+        for package in dev1.format.packages + dev2.format.packages + dev.format.packages:
+            self.assertIn(package, packages)
+
+if __name__ == "__main__":
+    unittest.main()


-- 
To view this commit on github, visit https://github.com/rhinstaller/blivet/commit/f1231f96f595430af096336ff9612427fcf8a3a7


More information about the anaconda-patches mailing list