Merge lp:~adeuring/launchpad/test-hwsubm into lp:launchpad

[Abel Deuring]

[r=flacoste][bug=835103] allow to process HWDB submissions from the checkbox version in Lucid, Maverick, Natty.

This branch allows to parse HWDB reports that come from the Maverick and Natty versions of checkbox. These versions do not provide information we need to store data about SCSI devices in the hardwrae database: the reports have no <sysfs-qttributes> node. This node is supposed to be the source for the vendor annd model name of SCSI devcies.

We could still get the vendor/model name for block devices, by looking into sub-nodes of the "main" udev node for these devcies, but:

- we urgently need a fix
- Data about SCSI block devcies is probably the least interesting data; more intersting are other devices, like tape robots or scanners: support tends to be more fragile that support for disks or CD drives.

This branch changes the RelaxNG schema so that raw submissions without a <sysfs-attributes> node pass; it ensures that the further porcessing of the data works.

The main change is in is_scsi_device (@@ -2731,6 +2747,12 @@): This property is used by the proeprty scsi_vendor and scsi_model to check if the vendor/model name exist; if the latter properties are None, the "potential scsi device" is not considered to be a SCSI devcie, an no data about is stored.

("Potential SCSI devices": Many non-SCSI devices, like ATA disks, USB disks or USB card readers, use the SCSI command set to communicate with the host computer. These devices are treated like real SCSI devcies, but they don't have the sysfs attributes vendor/model; this is already used by the HWDB submission processing script to decide, if data about a given "potentail SCSI device" should be stored.

[Francis J. Lacoste (flacoste)]

<flacoste> adeuring: just FYI, Lucid is also affected, we have very few valid Lucid submissions, and I guess they date from before the problematic checkbox changes
<adeuring> flacoste: right; I'll check a few lucid reports too
<flacoste> adeuring: do we have hwdb integration tests? If we do, is it worth adding one for a problematic report?
<adeuring> flacoste: right, I am aware that such a test is missing -- the main point is: I am too tired to write one today...
<flacoste> adeuring: but do we have such tests already?
<adeuring> flacoste: only one or two: for correct data and for "trivially wrong data", IIRC
<flacoste> adeuring: ok
<flacoste> adeuring: r=me with the note that Lucid and later are affected, and with a reference to the bug report

review: Approve

=== modified file 'lib/canonical/launchpad/scripts/hardware-1_0.rng'
--- lib/canonical/launchpad/scripts/hardware-1_0.rng	2009-09-14 09:16:45 +0000
+++ lib/canonical/launchpad/scripts/hardware-1_0.rng	2011-08-29 20:46:29 +0000
@@ -148,11 +148,17 @@
                         <element name="dmi">
                             <text/>
                         </element>
-                        <element name="sysfs-attributes">
-                            <zeroOrMore>
-                                <text/>
-                            </zeroOrMore>
-                        </element>
+                        <optional>
+                            <!-- Unfortunately, the checkbox version in
+                                 Maverick and Natty do not provide sysfs
+                                 data.
+                            -->
+                            <element name="sysfs-attributes">
+                                <zeroOrMore>
+                                    <text/>
+                                </zeroOrMore>
+                            </element>
+                        </optional>
                     </interleave>
                 </group>
             </choice>

=== modified file 'lib/canonical/launchpad/scripts/tests/test_hwdb_submission_parser.py'
--- lib/canonical/launchpad/scripts/tests/test_hwdb_submission_parser.py	2011-08-12 11:37:08 +0000
+++ lib/canonical/launchpad/scripts/tests/test_hwdb_submission_parser.py	2011-08-29 20:46:29 +0000
@@ -986,6 +986,33 @@
             result,
             'Invalid parsing result for <hardware>')
 
+    def testHardware_no_sysfs_node(self):
+        """If teh <sysfs-attributes> node is missing, parseHardware()
+        returns a dicitionary where the entry for this node is None.
+        """
+        parser = self.MockSubmissionParserParseHardwareTest(self.log)
+
+        node = etree.fromstring("""
+            <hardware>
+                <hal/>
+                <processors/>
+                <aliases/>
+                <udev/>
+                <dmi/>
+            </hardware>
+            """)
+        result = parser._parseHardware(node)
+        self.assertEqual({
+            'hal': 'parsed HAL data',
+            'processors': 'parsed processor data',
+            'aliases': 'parsed alias data',
+            'udev': 'parsed udev data',
+            'dmi': 'parsed DMI data',
+            'sysfs-attributes': None,
+            },
+            result,
+            'Invalid parsing result for <hardware>')
+
     def test_parseHardware_sub_parsers_fail(self):
         """Test of SubmissionParser._parseHardware().
 
@@ -2162,6 +2189,17 @@
             parser.checkUdevScsiProperties(
                 [self.udev_root_device, self.udev_scsi_device], sysfs_data))
 
+    def testCheckUdevScsiProperties_data_is_none(self):
+        """Test of SubmissionParser.checkUdevScsiProperties().
+
+        checkUdevScsiProperties() even if no sysfs properties are
+        available.
+        """
+        parser = SubmissionParser()
+        self.assertTrue(
+            parser.checkUdevScsiProperties(
+                [self.udev_root_device, self.udev_scsi_device], None))
+
     def testCheckUdevScsiProperties_missing_devtype(self):
         """Test of SubmissionParser.checkUdevScsiProperties().
 

=== modified file 'lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py'
--- lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py	2011-08-13 04:07:10 +0000
+++ lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py	2011-08-29 20:46:29 +0000
@@ -349,6 +349,29 @@
         self.assertIs(
             None, parser.devices[pci_ethernet_controller_path].sysfs)
 
+    def test_buildUdevDeviceList_no_sysfs_data(self):
+        """Sysfs data is not required (maverick and natty submissions)."""
+        root_device_path = '/devices/LNXSYSTM:00'
+        pci_pci_bridge_path = '/devices/pci0000:00/0000:00:1c.0'
+        pci_ethernet_controller_path = (
+            '/devices/pci0000:00/0000:00:1c.0/0000:02:00.0')
+
+        udev_paths = (
+            root_device_path, pci_pci_bridge_path,
+            pci_ethernet_controller_path)
+
+        sysfs_data = None
+
+        parsed_data = self.makeUdevDeviceParsedData(udev_paths, sysfs_data)
+        parser = SubmissionParser()
+        parser.buildUdevDeviceList(parsed_data)
+
+        self.assertIs(
+            None, parser.devices[pci_pci_bridge_path].sysfs)
+
+        self.assertIs(
+            None, parser.devices[pci_ethernet_controller_path].sysfs)
+
     def test_buildUdevDeviceList_invalid_device_path(self):
         """Test the creation of UdevDevice instances for a submission.
 
@@ -3875,6 +3898,20 @@
         device = UdevDevice(None, self.root_device)
         self.assertFalse(device.is_scsi_device)
 
+    def test_is_scsi_device__no_sysfs_data(self):
+        """Test of UdevDevice.is_scsi_device.
+
+        If there is no sysfs data for a real SCSI device, is it not
+        considered as a real SCSI device.
+
+        Reason: Without sysfs data, we don't know the vendor and
+        model name, making it impossible to store data about the
+        device in the database.
+        """
+        device = UdevDevice(
+            None, self.scsi_scanner_device_data, None)
+        self.assertFalse(device.is_scsi_device)
+
     def test_scsi_vendor(self):
         """Test of UdevDevice.scsi_vendor."""
         device = UdevDevice(

=== modified file 'lib/lp/hardwaredb/scripts/hwdbsubmissions.py'
--- lib/lp/hardwaredb/scripts/hwdbsubmissions.py	2011-05-27 19:53:20 +0000
+++ lib/lp/hardwaredb/scripts/hwdbsubmissions.py	2011-08-29 20:46:29 +0000
@@ -680,7 +680,13 @@
                  where the values are the parsing results of _parseHAL,
                  _parseProcessors, _parseAliases.
         """
-        hardware_data = {}
+        # Submissions from checkbox for Lucid, Maverick and Natty
+        # unfortunately do not contain a <sysfs-attributes> node.
+        # A default value here allows us to mark these submissions.
+        # See also bug 835103.
+        hardware_data = {
+            'sysfs-attributes': None,
+            }
         for node in hardware_node.getchildren():
             parser = self._parse_hardware_section[node.tag]
             result = parser(node)
@@ -1328,6 +1334,13 @@
         should have a corresponding sysfs node, and this node should
         define the attributes 'vendor', 'model', 'type'.
         """
+        # Broken submissions from Lucid, Maverick and Natty. We'll have
+        # to deal with incomplete data for SCSI devices in this case if
+        # we don't want to drop the entire submission, so just pretend
+        # that things are fine.
+        # See also bug 835103.
+        if sysfs_data is None:
+            return True
         for device in udev_data:
             subsystem = device['E'].get('SUBSYSTEM')
             if subsystem != 'scsi':
@@ -1456,13 +1469,19 @@
         dmi_data = parsed_data['hardware']['dmi']
         for udev_data in parsed_data['hardware']['udev']:
             device_path = udev_data['P']
+            if sysfs_data is not None:
+                sysfs_data_for_device = sysfs_data.get(device_path)
+            else:
+                # broken Lucid, Maverick and Natty submissions.
+                # See also bug 835103.
+                sysfs_data_for_device = None
             if device_path == UDEV_ROOT_PATH:
                 device = UdevDevice(
-                    self, udev_data, sysfs_data=sysfs_data.get(device_path),
+                    self, udev_data, sysfs_data=sysfs_data_for_device,
                     dmi_data=dmi_data)
             else:
                 device = UdevDevice(
-                    self, udev_data, sysfs_data=sysfs_data.get(device_path))
+                    self, udev_data, sysfs_data=sysfs_data_for_device)
             self.devices[device_path] = device
 
         # The parent-child relations are derived from the path names of
@@ -2731,6 +2750,13 @@
         # udev sets the property SUBSYSTEM to "scsi" for a number of
         # different nodes: SCSI hosts, SCSI targets and SCSI devices.
         # They are distiguished by the property DEVTYPE.
+
+        # Hack for broken submissions from Lucid, Maverick and Natty:
+        # If we don't have sysfs information, pretend that no SCSI
+        # related node corresponds to a real device.
+        # See also bug 835103.
+        if self.sysfs is None:
+            return False
         properties = self.udev['E']
         return (properties.get('SUBSYSTEM') == 'scsi' and
                 properties.get('DEVTYPE') == 'scsi_device')

=== modified file 'lib/lp/hardwaredb/tests/test_hwdb_submission_validation.py'
--- lib/lp/hardwaredb/tests/test_hwdb_submission_validation.py	2011-08-12 11:37:08 +0000
+++ lib/lp/hardwaredb/tests/test_hwdb_submission_validation.py	2011-08-29 20:46:29 +0000
@@ -697,8 +697,8 @@
         is required, and either <hal> or all three tags <udev>, <dmi>,
         <sysfs-attributes> must be present.
         """
-        # Omitting any of the three tags <udev>, <dmi>, <sysfs-attributes>
-        # makes the data invalid.
+        # Omitting one of the tags <udev>, <dmi> makes the data invalid.
+        # Omitting <sysfs-attributes> is tolerated.
         all_tags = ['udev', 'dmi', 'sysfs-attributes']
         for index, missing_tag in enumerate(all_tags):
             test_tags = all_tags[:]
@@ -712,10 +712,13 @@
                 from_text='<hal',
                 to_text='</hal>')
             result, submission_id = self.runValidator(sample_data)
-            self.assertErrorMessage(
-                submission_id, result,
-                'Expecting an element %s, got nothing' % missing_tag,
-                'missing tag <%s> in <hardware>' % missing_tag)
+            if missing_tag != 'sysfs-attributes':
+                self.assertErrorMessage(
+                    submission_id, result,
+                    'Expecting an element %s, got nothing' % missing_tag,
+                    'missing tag <%s> in <hardware>' % missing_tag)
+            else:
+                self.assertFalse(result is None)
 
     def testHardwareSubTagHalMixedWithUdev(self):
         """Mixing <hal> with <udev>, <dmi>, <sysfs-attributes> is impossible.