Merge lp:~ev/apport/group-suspend-resume-failures into lp:~apport-hackers/apport/trunk

[Evan]

This branch adds report.crash_signature() support for suspend/resume KernelOops reports. The signature is of the form:

suspend/resume:MachineType:BiosVersion

This was suggested by Andy Whitcroft and Colin King.

[Martin Pitt (pitti)]

The code does not actually check whether this is a suspend/resume failure, it computes that signature for any kind of KernelOops. Presumably this is how it is supposed to work, so could you please adjust the doc string and test case name, as well as the comment (line 20) accordingly? Otherwise, if this really should just happen for suspend/resume, then report.py needs to check the value of 'Failure'.

Thank you!

review: Needs Fixing

[lp:~ev/apport/group-suspend-resume-failures]

2626. By Evan on 2013-04-25

Add a test for a signature without a BIOS version

2627. By Evan on 2013-04-25

Check that this in indeed a suspend/resume failure.

[Evan (ev)]

I believe r2627 fixes this.

[Martin Pitt (pitti)]

Thanks, LGTM now! Please commit with an appropriate NEWS entry.

review: Approve

=== modified file 'apport/report.py'
--- apport/report.py	2013-04-03 09:05:41 +0000
+++ apport/report.py	2013-04-25 10:55:40 +0000
@@ -1209,6 +1209,10 @@
 
         For Python crashes, this concatenates the ExecutablePath, exception
         name, and Traceback function names, again separated by a colon.
+
+        For suspend/resume failures, this concatenates whether it was a suspend
+        or resume failure with the hardware identifier and the BIOS version, if
+        it exists.
         '''
         if 'ExecutablePath' not in self:
             if not self['ProblemType'] in ('KernelCrash', 'KernelOops'):
@@ -1277,6 +1281,16 @@
 
             return self['ExecutablePath'] + ':' + trace[-1].split(':')[0] + sig
 
+        if self['ProblemType'] == 'KernelOops' and 'Failure' in self:
+            if 'suspend' in self['Failure'] or 'resume' in self['Failure']:
+                # Suspend / resume failure
+                sig = self['Failure']
+                if self.get('MachineType'):
+                    sig += ':%s' % self['MachineType']
+                if self.get('dmi.bios.version'):
+                    sig += ':%s' % self['dmi.bios.version']
+                return sig
+
         # KernelOops crashes
         if 'OopsText' in self:
             in_trace_body = False

=== modified file 'test/test_report.py'
--- test/test_report.py	2013-04-25 10:38:30 +0000
+++ test/test_report.py	2013-04-25 10:55:40 +0000
@@ -2128,5 +2128,19 @@
         finally:
             os.getuid = orig_getuid
 
+    def test_suspend_resume(self):
+        pr = apport.report.Report()
+        pr['ProblemType'] = 'KernelOops'
+        pr['Failure'] = 'suspend/resume'
+        pr['MachineType'] = 'Cray XT5'
+        pr['dmi.bios.version'] = 'ABC123 (1.0)'
+        expected = 'suspend/resume:Cray XT5:ABC123 (1.0)'
+        self.assertEqual(expected, pr.crash_signature())
+
+        # There will not always be a BIOS version
+        del pr['dmi.bios.version']
+        expected = 'suspend/resume:Cray XT5'
+        self.assertEqual(expected, pr.crash_signature())
+
 if __name__ == '__main__':
     unittest.main()