fix link bug and change sorting on /changes page

codespeed/models.py

@@ -516,6 +516,7 @@ def get_changes_table(self, trend_depth=10, force_save=False):
 
                 currentlist.append({
                     'bench_name': bench.name,
+                    'bench_source': bench.source,
                     'bench_description': bench.description,
                     'result': result,
                     'std_dev': std_dev,

codespeed/templates/codespeed/changes_table.html

@@ -30,7 +30,7 @@
   </tr>
 </tfoot>
 <tbody>
-{% for row in units.rows|dictsort:"bench_name" %}
+{% for row in units.rows|dictsort:"bench_name"|dictsort:"bench_source" %}
   <tr data-change="{{ row.change|fix_infinity }}" data-trend="{{ row.trend|fix_infinity }}">
     <td title="{{ row.bench_description }}">{{ row.bench_name }}</td>
     <td>{{ row.result|floatformat:units.precission }}</td>

codespeed/tests/test_models.py

@@ -186,6 +186,12 @@ def test_bigger_bad_beats_smaller_bad(self):
         self.assertIn('b1', rep.summary)
         self.assertEqual('red', rep.colorcode)
 
+    def test_changes_table_row_has_bench_source(self):
+        s1 = self.make_result(15)
+        rep = self.make_report(s1)
+        tablelist = rep.get_changes_table(force_save=True)
+        self.assertEqual(tablelist[0]['rows'][0]['bench_source'], 'legacy')
+
     def test_multiple_quantities(self):
         b1 = self.make_bench('b1', quantity='Space', units='bytes')
         s1 = self.make_result(1.0)

codespeed/tests/test_views_data.py

@@ -2,11 +2,12 @@
 from django.test import TestCase
 from django.test import override_settings
 
-from codespeed.models import Project, Executable, Branch, Revision
+from codespeed.models import Project, Executable, Branch, Revision, Benchmark
 from codespeed.views import getbaselineexecutables
 from codespeed.views import getcomparisonexes
 from codespeed.views_data import get_sanitized_executable_name_for_timeline_view
 from codespeed.views_data import get_sanitized_executable_name_for_comparison_view
+from codespeed.views_data import parse_benchmark_ident
 
 
 class TestGetBaselineExecutables(TestCase):
@@ -278,3 +279,43 @@ def test_get_sanitized_executable_name_for_comparison_view(self):
         executable = Executable(name='b' * 25)
         name = get_sanitized_executable_name_for_comparison_view(executable)
         self.assertEqual(name, 'b' * 20 + '...')
+
+
+class TestParseBenchmarkIdent(TestCase):
+    """The /changes/ page links to /timeline/?ben=<benchmark.name>, i.e. the
+    bare name without a '.<source>' suffix. parse_benchmark_ident() must
+    resolve such short links back to the right source whenever the name is
+    unambiguous, even when the name itself contains a dot (as pyperformance
+    benchmark names sometimes do, e.g. 'base16_large.pyperf')."""
+
+    def test_full_ident_with_valid_source_suffix(self):
+        self.assertEqual(
+            parse_benchmark_ident('mybenchmark.pyperformance'),
+            ('mybenchmark', 'pyperformance'))
+        self.assertEqual(
+            parse_benchmark_ident('mybenchmark.legacy'),
+            ('mybenchmark', 'legacy'))
+
+    def test_bare_legacy_name_without_dot(self):
+        Benchmark.objects.create(name='ai', source='legacy')
+        self.assertEqual(parse_benchmark_ident('ai'), ('ai', 'legacy'))
+
+    def test_bare_pyperformance_name_containing_dot(self):
+        # Regression test: this name contains a dot but isn't a valid
+        # '<name>.<source>' pair, since 'pyperf' isn't a known source.
+        Benchmark.objects.create(
+            name='base16_large.pyperf', source='pyperformance')
+        self.assertEqual(
+            parse_benchmark_ident('base16_large.pyperf'),
+            ('base16_large.pyperf', 'pyperformance'))
+
+    def test_unknown_name_falls_back_to_legacy(self):
+        self.assertEqual(
+            parse_benchmark_ident('nosuchbenchmark'),
+            ('nosuchbenchmark', 'legacy'))
+
+    def test_ambiguous_name_across_sources_falls_back_to_legacy(self):
+        Benchmark.objects.create(name='float', source='legacy')
+        Benchmark.objects.create(name='float', source='pyperformance')
+        self.assertEqual(
+            parse_benchmark_ident('float'), ('float', 'legacy'))

codespeed/views_data.py

@@ -11,10 +11,20 @@
 
 
 def parse_benchmark_ident(ben):
-    """Split a '<name>.<source>' (or bare '<name>') into (name, source='legacy')."""
+    """Split a '<name>.<source>' (or bare '<name>') into (name, source).
+
+    If the suffix isn't a known source (e.g. a bare name that itself
+    contains a dot, like 'base16_large.pyperf'), look up the source by
+    name: if it uniquely identifies a benchmark, use that source, else
+    fall back to 'legacy'.
+    """
     name, _, suffix = ben.rpartition('.')
     if name and suffix in dict(Benchmark.S_TYPES):
         return name, suffix
+    sources = list(
+        Benchmark.objects.filter(name=ben).values_list('source', flat=True))
+    if len(sources) == 1:
+        return ben, sources[0]
     return ben, 'legacy'