Merge lp:~doanac/qa-dashboard/bad-crash-counts into lp:qa-dashboard

[Andy Doan]

fix bad crash-count logic for summary page

We were accidentally counting crashes from prior jenkins jobs when
newer ones had improved results. The refactoring of the
smokeng.views.overview was just done so that it would be possible to
test the "get_totals" API easier.

fix bad crash-count logic for summary page

We were accidentally counting crashes from prior jenkins jobs when
newer ones had improved results. The refactoring of the
smokeng.views.overview was just done so that it would be possible to
test the "get_totals" API easier.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:717
http://s-jenkins.ubuntu-ci:8080/job/dashboard-ci/301/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/dashboard-ci/301/rebuild

review: Approve (continuous-integration)

[Joe Talbott (joetalbott)]

+1 for getting rid of that nested loop.

review: Approve

=== modified file 'smokeng/tests.py'
--- smokeng/tests.py	2014-02-05 05:21:23 +0000
+++ smokeng/tests.py	2014-02-11 23:24:55 +0000
@@ -40,6 +40,7 @@
 
 from smokeng.management.commands.jenkins_pull_smokeng import Command
 from smokeng.config import get_internal_jenkins, primary_targets
+from smokeng import views
 
 from common.models import (
     Bug,
@@ -355,6 +356,62 @@
 
         self.assertEqual(self.result.smokeresultbug_set.count(), 1)
 
+    def test_crash_count_empty(self):
+        results = views._results_overview(SmokeResult.objects.all())
+        totals = views.get_totals(results, 'saucy')[0].values()
+        self.assertEqual(0, totals[0]['crash_count'])
+
+    def test_crash_count_simple(self):
+        Artifact.objects.create(
+            url='http://foo/clientlogs/notes-app-autopilot/test.crash',
+            name='test.crash',
+            jenkins_build=self.jenkins_build,
+        )
+        results = views._results_overview(SmokeResult.objects.all())
+        totals = views.get_totals(results, 'saucy')[0].values()
+        self.assertEqual(1, totals[0]['crash_count'])
+
+    def test_crash_count_complex(self):
+        Artifact.objects.create(
+            url='http://foo/clientlogs/notes-app-autopilot/test.crash',
+            name='test.crash',
+            jenkins_build=self.jenkins_build,
+        )
+        Artifact.objects.create(
+            url='http://foo/clientlogs/notes-app-autopilot/test-other.crash',
+            name='test-other.crash',
+            jenkins_build=self.jenkins_build,
+        )
+
+        results = views._results_overview(SmokeResult.objects.all())
+        totals = views.get_totals(results, 'saucy')[0].values()
+        self.assertEqual(2, totals[0]['crash_count'])
+
+        # now create a new run that has a different number of failures
+        # NOTE: our pull script and REST API reuse the existing SmokeResult
+        rerun_build = JenkinsBuild.objects.create(
+            job=self.jenkins_job,
+            build_number=2,
+            ran_at=datetime.datetime.now(),
+        )
+
+        self.result.jenkins_build = rerun_build
+        self.result.save()
+
+        results = views._results_overview(SmokeResult.objects.all())
+        totals = views.get_totals(results, 'saucy')[0].values()
+        self.assertEqual(0, totals[0]['crash_count'])
+
+        # now add a failure to the re-run
+        Artifact.objects.create(
+            url='http://foo/clientlogs/notes-app-autopilot/test-other.crash',
+            name='test-other.crash',
+            jenkins_build=rerun_build,
+        )
+        results = views._results_overview(SmokeResult.objects.all())
+        totals = views.get_totals(results, 'saucy')[0].values()
+        self.assertEqual(1, totals[0]['crash_count'])
+
 
 class TestSmokeMethods(TestCase):
 

=== modified file 'smokeng/views.py'
--- smokeng/views.py	2014-02-10 18:06:07 +0000
+++ smokeng/views.py	2014-02-11 23:24:55 +0000
@@ -101,11 +101,6 @@
         totals[image]['pass_rate_pct'] = \
             totals[image]['pass_rate'] * 100
 
-    for t in totals:
-        for crash in crashes:
-            if totals[t]['image_id'] == crash['image__id']:
-                totals[t]['crash_count'] += 1
-
     for bug in bugs.iterator():
         if bug.result.image.id in totals:
             if bug.bug.bug_no not in totals[bug.result.image.id]['bugs']:
@@ -132,7 +127,17 @@
         'jenkins_build__artifact__url',
     ).values(
         'image__id',
+        'jenkins_build__id',
+        'jenkins_build__artifact__url',
     )
+
+    multi = re.compile('.*clientlogs/(.*)/.*')
+    for c in crashes:
+        m = multi.match(c['jenkins_build__artifact__url'])
+        if m:
+            c['smokeresult'] = m.group(1)
+        else:
+            c['smokeresult'] = ''
     return crashes
 
 
@@ -148,6 +153,10 @@
     key = result['image__id']
 
     if key not in totals:
+        # we could have job-wide crashes or per-mega test crashes. figure
+        # out job wide crashes here, and per-mega get appended below:
+        job_crashes = len([x for x in crashes
+                          if x['image__id'] == key and not x['smokeresult']])
         totals[key] = dict(
             build_id=result['image__build_id'],
             rootfs_id=result['image__rootfs_id'],
@@ -161,7 +170,7 @@
             error_count=0,
             bugs=[],
             name=result['name'],
-            crash_count=0,
+            crash_count=job_crashes,
         )
 
         if 'image__variant' in result:
@@ -184,6 +193,52 @@
     totals[key]['error_count'] += result['error_count']
     totals[key]['fail_count'] += result['fail_count']
 
+    for c in crashes:
+        if c['jenkins_build__id'] == result['jenkins_build']:
+            if result['name'] == c['smokeresult']:
+                totals[key]['crash_count'] += 1
+
+
+def _results_overview(results):
+    return results.values(
+        'image__build_number',
+        'image__variant',
+        'image__arch',
+        'image__id',
+        'image__build_id',
+        'image__rootfs_id',
+        'image__device_specific_id',
+        'image__customization_id',
+        'image__release',
+        'name',
+        'ran_at',
+        'status',
+        'jenkins_build',
+    ).annotate(
+        pass_count=models.Sum('pass_count'),
+        fail_count=models.Sum('fail_count'),
+        error_count=models.Sum('error_count'),
+        total_count=models.Sum('total_count'),
+    ).values(
+        'image__build_id',
+        'image__rootfs_id',
+        'image__device_specific_id',
+        'image__customization_id',
+        'image__build_number',
+        'image__variant',
+        'image__arch',
+        'image__id',
+        'image__release',
+        'name',
+        'ran_at',
+        'status',
+        'jenkins_build',
+        'pass_count',
+        'fail_count',
+        'error_count',
+        'total_count',
+    )
+
 
 @BreadCrumb("{release} image testing", parent=index, needs=['release'])
 @require_GET
@@ -210,44 +265,7 @@
             targets = 'primary'
             results = results.filter(image__arch__in=primary_tgts)
 
-    results = results.values(
-        'image__build_number',
-        'image__variant',
-        'image__arch',
-        'image__id',
-        'image__build_id',
-        'image__rootfs_id',
-        'image__device_specific_id',
-        'image__customization_id',
-        'image__release',
-        'name',
-        'ran_at',
-        'status',
-        'jenkins_build',
-    ).annotate(
-        pass_count=models.Sum('pass_count'),
-        fail_count=models.Sum('fail_count'),
-        error_count=models.Sum('error_count'),
-        total_count=models.Sum('total_count'),
-    ).values(
-        'image__build_id',
-        'image__rootfs_id',
-        'image__device_specific_id',
-        'image__customization_id',
-        'image__build_number',
-        'image__variant',
-        'image__arch',
-        'image__id',
-        'image__release',
-        'name',
-        'ran_at',
-        'status',
-        'jenkins_build',
-        'pass_count',
-        'fail_count',
-        'error_count',
-        'total_count',
-    )
+    results = _results_overview(results)
 
     totals, bug_data = get_totals(results, release)
     table = SmokeImageTable(totals.values())