Code review comment for lp:~adeuring/launchpad/hwdb-class-udev-device-10

this brach is based on lp:~adeuring/launchpad/hwdb-class-udev-device-9, which has not yet landed. Here is the diff against that branch:

=== modified file 'lib/canonical/launchpad/scripts/hwdbsubmissions.py'
--- lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-19 10:45:31 +0000
+++ lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-19 12:14:40 +0000
@@ -2015,6 +2015,10 @@
         missing vendor/product information in order to store the
         data reliably in the HWDB.

+ raw_bus == 'acpi' is used in udev data for the main system,
+ for CPUs, power supply etc. Except for the main sytsem, none
+ of them provides a vendor or product id, so we ignore them.
+
         XXX Abel Deuring 2008-05-06: IEEE1394 devices are a bit
         nasty: The standard does not define any specification
         for product IDs or product names, hence HAL often uses
@@ -2038,7 +2042,7 @@
         ensure that we have vendor ID, product ID and product name.
         """
         bus = self.raw_bus
- if bus == 'unknown' and self.udi != ROOT_UDI:
+ if bus in ('unknown', 'acpi') and not self.is_root_device:
             # The root node is course a real device; storing data
             # about other devices with the bus "unkown" is pointless.
             return False
@@ -2059,11 +2063,11 @@
             # it.
             if self.real_bus != HWBus.IDE:
                 self.parser._logWarning(
- 'A HALDevice that is supposed to be a real device does '
+ 'A %s that is supposed to be a real device does '
                     'not provide bus, vendor ID, product ID or product name: '
                     '%r %r %r %r %s'
- % (self.real_bus, self.vendor_id, self.product_id,
- self.product, self.udi),
+ % (self.__class__.__name__, self.real_bus, self.vendor_id,
+ self.product_id, self.product, self.device_id),
                     self.parser.submission_key)
             return False
         return True

=== modified file 'lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py'
--- lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py 2009-10-19 10:45:31 +0000
+++ lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py 2009-10-19 13:24:07 +0000
@@ -3320,6 +3320,21 @@
             'E': {}
             }

+ self.cpu_device_data = {
+ 'P': '/devices/LNXSYSTM:00/LNXCPU:00',
+ 'E': {
+ 'DRIVER': 'processor',
+ 'SUBSYSTEM': 'acpi',
+ },
+ }
+
+ self.platform_device_data = {
+ 'P': '/devices/platform/dock.0',
+ 'E': {
+ 'SUBSYSTEM': 'platform',
+ },
+ }
+
     def test_device_id(self):
         """Test of UdevDevice.device_id."""
         device = UdevDevice(None, self.pci_sata_controller)
@@ -3881,6 +3896,110 @@
                    device.device_id == self.usb_storage_usb_device_path,
                    device.is_real_device))

+ def test_has_reliable_data_system(self):
+ """Test of UdevDevice.has_reliable_data for a system."""
+ root_device = UdevDevice(
+ None, self.root_device, dmi_data=self.root_device_dmi_data)
+ self.assertTrue(root_device.has_reliable_data)
+
+ def test_has_reliable_data_system_no_vendor_name(self):
+ """Test of UdevDevice.has_reliable_data for a system.
+
+ If the DMI data does not provide vendor name, has_reliable_data
+ is False.
+ """
+ del self.root_device_dmi_data['/sys/class/dmi/id/sys_vendor']
+ root_device = UdevDevice(
+ None, self.root_device, dmi_data=self.root_device_dmi_data)
+ parser = SubmissionParser(self.log)
+ parser.submission_key = 'root device without vendor name'
+ root_device.parser = parser
+ self.assertFalse(root_device.has_reliable_data)
+ self.assertWarningMessage(
+ parser.submission_key,
+ "A UdevDevice that is supposed to be a real device does not "
+ "provide bus, vendor ID, product ID or product name: "
+ "<DBItem HWBus.SYSTEM, (0) System> None 'LIFEBOOK E8210' "
+ "'LIFEBOOK E8210' /devices/LNXSYSTM:00")
+
+
+ def test_has_reliable_data_system_no_product_name(self):
+ """Test of UdevDevice.has_reliable_data for a system.
+
+ If the DMI data does not provide product name, has_reliable_data
+ is False.
+ """
+ del self.root_device_dmi_data['/sys/class/dmi/id/product_name']
+ root_device = UdevDevice(
+ None, self.root_device, dmi_data=self.root_device_dmi_data)
+ parser = SubmissionParser(self.log)
+ parser.submission_key = 'root device without product name'
+ root_device.parser = parser
+ self.assertFalse(root_device.has_reliable_data)
+ self.assertWarningMessage(
+ parser.submission_key,
+ "A UdevDevice that is supposed to be a real device does not "
+ "provide bus, vendor ID, product ID or product name: "
+ "<DBItem HWBus.SYSTEM, (0) System> 'FUJITSU SIEMENS' None None "
+ "/devices/LNXSYSTM:00")
+
+ def test_has_reliable_data_acpi_device(self):
+ """Test of UdevDevice.has_reliable_data for an ACPI device.
+
+ APCI devices are considered not to have reliable data. The only
+ exception is the root device, see test_has_reliable_data_system.
+ """
+ acpi_device = UdevDevice(None, self.cpu_device_data)
+ self.assertEqual('acpi', acpi_device.raw_bus)
+ self.assertFalse(acpi_device.has_reliable_data)
+
+ def test_has_reliable_data_platform_device(self):
+ """Test of UdevDevice.has_reliable_data for a "platform" device.
+
+ devices with raw_bus == 'platform' are considered not to have
+ reliable data.
+ """
+ platform_device = UdevDevice(None, self.platform_device_data)
+ self.assertFalse(platform_device.has_reliable_data)
+
+ def test_has_reliable_data_pci_device(self):
+ """Test of UdevDevice.has_reliable_data for a PCI device."""
+ devices = self.buildUdevDeviceHierarchy(
+ self.pci_bridge_pccard_hierarchy_data)
+ pci_device = devices[self.pci_pccard_bridge_path]
+ self.assertTrue(pci_device.has_reliable_data)
+
+ def test_has_reliable_data_usb_device(self):
+ """Test of UdevDevice.has_reliable_data for a USB device."""
+ usb_device = UdevDevice(None, self.usb_storage_usb_device_data)
+ self.assertTrue(usb_device.has_reliable_data)
+
+ def test_has_reliable_data_scsi_device(self):
+ """Test of UdevDevice.has_reliable_data for a SCSI device."""
+ devices = self.buildUdevDeviceHierarchy(
+ self.scsi_device_hierarchy_data)
+ scsi_device = devices[self.scsi_scanner_device_path]
+ self.assertTrue(scsi_device.has_reliable_data)
+
+ def test_has_reliable_data_usb_interface_device(self):
+ """Test of UdevDevice.has_reliable_data for a USB interface.
+
+ UdevDevice.has_reliable_data should only be called for nodes
+ where is_rel_device is True. If called for other nodes, we
+ may get a warning because they do not provide reqired data,
+ like a bus, vendor or product ID.
+ """
+ parser = SubmissionParser(self.log)
+ parser.submission_key = (
+ 'UdevDevice.has_reliable_data for a USB interface')
+ usb_interface = UdevDevice(parser, self.usb_storage_usb_interface)
+ self.assertFalse(usb_interface.has_reliable_data)
+ self.assertWarningMessage(
+ parser.submission_key,
+ 'A UdevDevice that is supposed to be a real device does not '
+ 'provide bus, vendor ID, product ID or product name: None None '
+ 'None None /devices/pci0000:00/0000:00:1d.7/usb1/1-1/1-1:1.0')
+

 class TestHWDBSubmissionTablePopulation(TestCaseHWDB):
     """Tests of the HWDB popoluation with submitted data."""