Merge pull request #220 from mwatts15/report-gen-refactor

Refactoring reports creation for clarity

codespeed/models.py

 # -*- coding: utf-8 -*-
 from __future__ import absolute_import, unicode_literals
 
+import logging
 import os
 import json
 
@@ -12,6 +13,8 @@ from django.utils.encoding import python_2_unicode_compatible
 
 from .commits.github import GITHUB_URL_RE
 
+logger = logging.getLogger(__name__)
+
 
 @python_2_unicode_compatible
 class Project(models.Model):
@@ -73,6 +76,31 @@ class Project(models.Model):
         super(Project, self).save(*args, **kwargs)
 
 
+class HistoricalValue(object):
+    def __init__(self, name=None, val=0, color='none'):
+        self.name = name
+        self.val = val
+        self.color = color
+
+    def update_if_less_important_than(self, val, color, name):
+        if self.is_less_important_than(val, color):
+            # Do update biggest total change
+            self.val = val
+            self.color = color
+            self.name = name
+
+    def is_less_important_than(self, val, color):
+        if color == "red" and self.color != "red":
+            return True
+        elif color == "red" and abs(val) > abs(self.val):
+            return True
+        elif (color == "green" and self.color != "red" and
+                abs(val) > abs(self.val)):
+            return True
+        else:
+            return False
+
+
 @python_2_unicode_compatible
 class Branch(models.Model):
     name = models.CharField(max_length=20)
@@ -225,11 +253,44 @@ class Report(models.Model):
 
     def save(self, *args, **kwargs):
         tablelist = self.get_changes_table(force_save=True)
-        max_change, max_change_ben, max_change_color = 0, None, "none"
-        max_trend, max_trend_ben, max_trend_color = 0, None, "none"
-        average_change, average_change_units, average_change_color = 0, None, "none"
-        average_trend, average_trend_units, average_trend_color = 0, None, "none"
+        self.reinitialize()
+        changes = self.aggregate_significant_changes(tablelist)
+        self.update_to_highest_priority_change(changes)
+
+        super(Report, self).save(*args, **kwargs)
+
+    def update_to_highest_priority_change(self, changes):
+        average_change = changes['average_change']
+        max_change = changes['max_change']
+        average_trend = changes['average_trend']
+        max_trend = changes['max_trend']
+
+        # Average change
+        if average_change.color != "none":
+            self.update_summary("Average {} {}", average_change)
+            self.colorcode = average_change.color
+
+        # Single benchmark change
+        elif max_change.color != "none":
+            self.update_summary("{} {}", max_change)
+            self.colorcode = max_change.color
+
+        # Average trend
+        elif average_trend.color != "none":
+            self.update_summary("Average {} trend {}", average_trend)
+            self.update_by_trend_color(average_trend.color)
+
+        # Single benchmark trend
+        elif max_trend.color != "none":
+            self.update_summary("{} trend {}", max_trend)
+            # use lighter colors for trend results:
+            self.update_by_trend_color(max_trend.color)
+
+    def reinitialize(self):
+        self.summary = ""
+        self.colorcode = "none"
 
+    def aggregate_significant_changes(self, tablelist):
         # Get default threshold values
         change_threshold = 3.0
         trend_threshold = 5.0
@@ -239,95 +300,54 @@ class Report(models.Model):
         if hasattr(settings, 'TREND_THRESHOLD') and settings.TREND_THRESHOLD:
             trend_threshold = settings.TREND_THRESHOLD
 
-        # Fetch big changes for each unit type and each benchmark
-        for units in tablelist:
-            # Total change
-            val = units['totals']['change']
+        max_change = HistoricalValue()
+        max_trend = HistoricalValue()
+        average_change = HistoricalValue()
+        average_trend = HistoricalValue()
+
+        # Fetch big changes for each quantity and each benchmark
+        for quantity in tablelist:
+            quantity_name = quantity['units_title'].lower()
+            less_is_better = quantity['lessisbetter']
+
+            val = quantity['totals']['change']
             if val == "-":
                 continue
-            color = self.getcolorcode(val, units['lessisbetter'],
-                                      change_threshold)
-            if self.is_big_change(val, color, average_change, average_change_color):
-                # Do update biggest total change
-                average_change = val
-                average_change_units = units['units_title']
-                average_change_color = color
-            # Total trend
-            val = units['totals']['trend']
+            color = self.getcolorcode(val, less_is_better, change_threshold)
+            average_change.update_if_less_important_than(val, color,
+                                                         quantity_name)
+
+            val = quantity['totals']['trend']
             if val != "-":
-                color = self.getcolorcode(val, units['lessisbetter'],
-                                          trend_threshold)
-                if self.is_big_change(val, color, average_trend, average_trend_color):
-                    # Do update biggest total trend change
-                    average_trend = val
-                    average_trend_units = units['units_title']
-                    average_trend_color = color
-            for row in units['rows']:
+                color = self.getcolorcode(val, less_is_better, trend_threshold)
+                average_trend.update_if_less_important_than(val, color,
+                                                            quantity_name)
+
+            for row in quantity['rows']:
+                benchmark_name = row['bench_name']
                 # Single change
                 val = row['change']
                 if val == "-":
                     continue
-                color = self.getcolorcode(val, units['lessisbetter'],
+                color = self.getcolorcode(val, less_is_better,
                                           change_threshold)
-                if self.is_big_change(val, color, max_change, max_change_color):
-                    # Do update biggest single change
-                    max_change = val
-                    max_change_ben = row['bench_name']
-                    max_change_color = color
+                max_change.update_if_less_important_than(val, color,
+                                                         benchmark_name)
                 # Single trend
                 val = row['trend']
                 if val == "-":
                     continue
-                color = self.getcolorcode(val, units['lessisbetter'], trend_threshold)
-                if self.is_big_change(val, color, max_trend, max_trend_color):
-                    # Do update biggest single trend change
-                    max_trend = val
-                    max_trend_ben = row['bench_name']
-                    max_trend_color = color
-        # Reinitialize
-        self.summary = ""
-        self.colorcode = "none"
-
-        # Save summary in order of priority
-        # (changes results before trends, averages before individual results)
-
-        # Average change
-        if average_change_color != "none":
-            self.summary = "Average %s %s" % (
-                average_change_units.lower(),
-                self.updown(average_change))
-            self.colorcode = average_change_color
-
-        # Single benchmark change
-        elif max_change_color != "none":
-            self.summary = "%s %s" % (
-                max_change_ben,
-                self.updown(max_change))
-            self.colorcode = max_change_color
-
-        # Average trend
-        elif average_trend_color != "none":
-            self.summary = "Average %s trend %s" % (
-                average_trend_units.lower(),
-                self.updown(average_trend))
-            # use lighter colors for trend results:
-            if average_trend_color == "red":
-                self.colorcode = "yellow"
-            elif average_trend_color == "green":
-                self.colorcode = "lightgreen"
-
-        # Single benchmark trend
-        elif max_trend_color != "none":
-            self.summary = "%s trend %s" % (
-                max_trend_ben,
-                self.updown(max_trend))
-            # use lighter colors for trend results:
-            if max_trend_color == "red":
-                self.colorcode = "yellow"
-            elif max_trend_color == "green":
-                self.colorcode = "lightgreen"
-
-        super(Report, self).save(*args, **kwargs)
+                color = self.getcolorcode(val, less_is_better, trend_threshold)
+                max_trend.update_if_less_important_than(val, color,
+                                                        benchmark_name)
+        return {'max_change': max_change,
+                'max_trend': max_trend,
+                'average_change': average_change,
+                'average_trend': average_trend}
+
+    def update_summary(self, format, hist_value):
+        self.summary = format.format(hist_value.name,
+                                     self.updown(hist_value.val))
 
     def updown(self, val):
         """Substitutes plus/minus with up/down"""
@@ -338,16 +358,12 @@ class Report(models.Model):
         else:
             return "%s %.1f%%" % (direction, aval)
 
-    def is_big_change(self, val, color, current_val, current_color):
-        if color == "red" and current_color != "red":
-            return True
-        elif color == "red" and abs(val) > abs(current_val):
-            return True
-        elif (color == "green" and current_color != "red" and
-                abs(val) > abs(current_val)):
-            return True
-        else:
-            return False
+    def update_by_trend_color(self, color):
+        # use lighter colors for trend results:
+        if color == "red":
+            self.colorcode = "yellow"
+        elif color == "green":
+            self.colorcode = "lightgreen"
 
     def getcolorcode(self, val, lessisbetter, threshold):
         if lessisbetter:
@@ -359,6 +375,20 @@ class Report(models.Model):
         else:
             return "none"
 
+    def get_last_revisions(self, depth):
+        lastrevisions = []
+        try:
+            lastrevisions = Revision.objects.filter(
+                branch=self.revision.branch
+            ).filter(
+                date__lte=self.revision.date
+            ).order_by('-date')[:depth + 1]
+            # Same as self.revision unless in a different branch
+        except Exception as e:
+            logger.warning("Exception while getting results: %s", e,
+                           exc_info=True)
+        return lastrevisions
+
     def get_changes_table(self, trend_depth=10, force_save=False):
         # Determine whether required trend value is the default one
         default_trend = 10
@@ -370,16 +400,10 @@ class Report(models.Model):
             return self._get_tablecache()
         # Otherwise generate a new changes table
         # Get latest revisions for this branch (which also sets the project)
-        try:
-            lastrevisions = Revision.objects.filter(
-                branch=self.revision.branch
-            ).filter(
-                date__lte=self.revision.date
-            ).order_by('-date')[:trend_depth + 1]
-            # Same as self.revision unless in a different branch
-            lastrevision = lastrevisions[0]
-        except:
+        lastrevisions = self.get_last_revisions(trend_depth)
+        if not lastrevisions:
             return []
+
         change_list = []
         pastrevisions = []
         if len(lastrevisions) > 1:
@@ -394,7 +418,7 @@ class Report(models.Model):
             pastrevisions = lastrevisions[trend_depth - 2:trend_depth + 1]
 
         result_list = Result.objects.filter(
-            revision=lastrevision
+            revision=lastrevisions[0]
         ).filter(
             environment=self.environment
         ).filter(
@@ -463,23 +487,23 @@ class Report(models.Model):
                 # Calculate trend:
                 # percentage change relative to average of 3 previous results
                 # Calculate past average
-                average = 0
-                averagecount = 0
+                result_sum = 0
+                num_past_results = 0
                 if len(pastrevisions):
                     for rev in pastrevisions:
-                        past_rev = Result.objects.filter(
+                        past_result = Result.objects.filter(
                             revision=rev
                         ).filter(
                             environment=self.environment
                         ).filter(
                             executable=self.executable
                         ).filter(benchmark=bench)
-                        if past_rev.count():
-                            average += past_rev[0].value
-                            averagecount += 1
+                        if past_result.count():
+                            result_sum += past_result[0].value
+                            num_past_results += 1
                 trend = "-"
-                if average:
-                    average = average / averagecount
+                if result_sum:
+                    average = result_sum / num_past_results
                     trend = (result - average) * 100 / average
                     totals['trend'].append(result / average)
 

codespeed/tests/test_models.py

@@ -2,9 +2,239 @@
 import os
 
 from django.conf import settings
-from django.test import TestCase
+from django.test import TestCase, override_settings
 
-from codespeed.models import Project
+from codespeed.models import (Project, Report, Revision, Branch, Environment,
+                              Benchmark, Executable, Result)
+from datetime import timedelta, datetime
+
+
+@override_settings(CHANGE_THRESHOLD=3.0, TREND_THRESHOLD=5.0)
+class TestReport(TestCase):
+
+    def setUp(self):
+        self.days = 0
+        self.starttime = datetime.now() + timedelta(days=-100)
+
+        Project(repo_type='G', name='pro',
+                repo_path='/home/foo/codespeed').save()
+        self.pro = Project.objects.get(name='pro')
+
+        Branch(project=self.pro, name='branch').save()
+        self.b = Branch.objects.get(name='branch')
+
+        Environment(name='Walden Pond').save()
+        Executable(name='walden', project=self.pro).save()
+        Benchmark(name='TestBench').save()
+
+        self.env = Environment.objects.get(name='Walden Pond')
+        self.exe = Executable.objects.get(name='walden')
+        self.bench = Benchmark.objects.get(name='TestBench')
+
+    def test_average_change_bad(self):
+        self.make_result(12)
+        s2 = self.make_result(15)
+        rep = self.make_report(s2)
+        self.assertEqual(rep.colorcode, 'red')
+
+    def test_average_change_good(self):
+        self.make_result(15)
+        s2 = self.make_result(12)
+        rep = self.make_report(s2)
+        self.assertEqual(rep.colorcode, 'green')
+
+    def test_within_threshold_none(self):
+        self.make_result(15)
+        s2 = self.make_result(15.2)
+        rep = self.make_report(s2)
+        self.assertEqual(rep.colorcode, 'none')
+
+    def test_initial_revision_none(self):
+        s2 = self.make_result(15)
+        rep = self.make_report(s2)
+        self.assertEqual(rep.colorcode, 'none')
+
+    def test_bench_change_good(self):
+        b1 = self.make_bench('b1')
+
+        s1 = self.make_result(15)
+        self.make_result(15, rev=s1, benchmark=b1)
+
+        s2 = self.make_result(14.54)
+        self.make_result(15, rev=s2, benchmark=b1)
+
+        rep = self.make_report(s2)
+        self.assertEqual(rep.colorcode, 'green')
+        self.assertIn(self.bench.name, rep.summary)
+
+    def test_bench_change_bad(self):
+        b1 = self.make_bench('b1')
+
+        s1 = self.make_result(15)
+        self.make_result(15, rev=s1, benchmark=b1)
+
+        s2 = self.make_result(15.46)
+        self.make_result(15, rev=s2, benchmark=b1)
+
+        rep = self.make_report(s2)
+        self.assertEqual(rep.colorcode, 'red')
+        self.assertIn(self.bench.name, rep.summary)
+
+    # NOTE: Don't need to test with multiple projects since the calculation of
+    # urgency doesn't take projects into account
+
+    def test_average_change_beats_bench_change(self):
+        b1 = self.make_bench('b1')
+
+        s1 = self.make_result(15)
+        self.make_result(15, rev=s1, benchmark=b1)
+
+        s2 = self.make_result(14)
+        self.make_result(15, rev=s2, benchmark=b1)
+
+        rep = self.make_report(s2)
+        self.assertIn('Average', rep.summary)
+
+    def test_good_benchmark_change_beats_bad_average_trend(self):
+        changes = self.make_bad_trend()
+        b1 = self.make_bench('b1')
+        for x in changes:
+            s1 = self.make_result(x)
+            if x != changes[-1]:
+                self.make_result(x, rev=s1, benchmark=b1)
+        self.make_result(changes[-2] * .97, rev=s1, benchmark=b1)
+        rep = self.make_report(s1)
+        self.assertEquals('green', rep.colorcode)
+        self.assertIn('b1', rep.summary)
+
+    def test_good_average_change_beats_bad_average_trend(self):
+        changes = self.make_bad_trend()
+        b1 = self.make_bench('b1')
+        for x in changes:
+            s1 = self.make_result(x)
+            if x != changes[-1]:
+                self.make_result(x, rev=s1, benchmark=b1)
+        self.make_result(changes[-2] * .92, rev=s1, benchmark=b1)
+        rep = self.make_report(s1)
+        self.assertEquals('green', rep.colorcode)
+        self.assertIn('Average', rep.summary)
+
+    def test_good_change_beats_good_trend(self):
+        changes = self.make_good_trend()
+        b1 = self.make_bench('b1')
+        for x in changes:
+            s1 = self.make_result(x)
+            if x != changes[-1]:
+                self.make_result(x, rev=s1, benchmark=b1)
+        self.make_result(changes[-2] * .95, rev=s1, benchmark=b1)
+        rep = self.make_report(s1)
+        self.assertIn('b1', rep.summary)
+        self.assertNotIn('trend', rep.summary)
+
+    def test_bad_trend_beats_good_trend(self):
+        good_changes = self.make_good_trend()
+        bad_changes = self.make_bad_trend()
+
+        b1 = self.make_bench('b1')
+        for i in range(len(good_changes)):
+            s1 = self.make_result(good_changes[i])
+            self.make_result(bad_changes[i], rev=s1, benchmark=b1)
+
+        rep = self.make_report(s1)
+        self.assertIn('trend', rep.summary)
+        self.assertIn('b1', rep.summary)
+        self.assertIn('yellow', rep.colorcode)
+
+    def test_bad_change_beats_good_trend(self):
+        changes = self.make_good_trend()
+        b1 = self.make_bench('b1')
+        for x in changes:
+            s1 = self.make_result(x)
+            if x != changes[-1]:
+                self.make_result(x, rev=s1, benchmark=b1)
+        self.make_result(changes[-2] * 1.05, rev=s1, benchmark=b1)
+        rep = self.make_report(s1)
+        self.assertIn('b1', rep.summary)
+        self.assertNotIn('trend', rep.summary)
+        self.assertEquals('red', rep.colorcode)
+
+    def test_bad_beats_good_change(self):
+        b1 = self.make_bench('b1')
+
+        s1 = self.make_result(12)
+        self.make_result(12, rev=s1, benchmark=b1)
+
+        s2 = self.make_result(15)
+        self.make_result(9, rev=s2, benchmark=b1)
+
+        rep = self.make_report(s2)
+        self.assertEqual(rep.colorcode, 'red')
+
+    def test_bigger_bad_beats_smaller_bad(self):
+        b1 = self.make_bench('b1')
+        b2 = self.make_bench('b2')
+
+        s1 = self.make_result(1.0)
+        self.make_result(1.0, rev=s1, benchmark=b1)
+        self.make_result(1.0, rev=s1, benchmark=b2)
+
+        s2 = self.make_result(1.0)
+        self.make_result(1.04, rev=s2, benchmark=b1)
+        self.make_result(1.03, rev=s2, benchmark=b2)
+
+        rep = self.make_report(s2)
+        self.assertIn('b1', rep.summary)
+        self.assertEquals('red', rep.colorcode)
+
+    def test_multiple_quantities(self):
+        b1 = self.make_bench('b1', quantity='Space', units='bytes')
+        s1 = self.make_result(1.0)
+        self.make_result(1.0, rev=s1, benchmark=b1)
+
+        s2 = self.make_result(1.4)
+        self.make_result(1.5, rev=s2, benchmark=b1)
+
+        rep = self.make_report(s2)
+        self.assertRegexpMatches(rep.summary, '[sS]pace')
+        self.assertEquals('red', rep.colorcode)
+
+    def make_result(self, value, rev=None, benchmark=None):
+        from uuid import uuid4
+
+        if not benchmark:
+            benchmark = self.bench
+
+        if not rev:
+            commitdate = self.starttime + timedelta(days=self.days)
+            cid = str(uuid4())
+            Revision(commitid=cid, date=commitdate, branch=self.b,
+                     project=self.pro).save()
+            rev = Revision.objects.get(commitid=cid)
+
+        Result(value=value, revision=rev, executable=self.exe,
+               environment=self.env, benchmark=benchmark).save()
+        self.days += 1
+        return rev
+
+    def make_report(self, revision):
+        Report(revision=revision, environment=self.env,
+               executable=self.exe).save()
+        return Report.objects.get(revision=revision)
+
+    def make_bench(self, name, quantity='Time', units='seconds'):
+        Benchmark(name=name, units_title=quantity, units=units).save()
+        return Benchmark.objects.get(name=name)
+
+    def make_bad_trend(self):
+        return self.make_trend(1)
+
+    def make_good_trend(self):
+        return self.make_trend(-1)
+
+    def make_trend(self, direction):
+        return [1 + direction * x * 1.25 *
+                settings.TREND_THRESHOLD / 100 / settings.TREND
+                for x in range(settings.TREND)]
 
 
 class TestProject(TestCase):