From efd9a8be99569511134642720fee2fb2238c4ca9 Mon Sep 17 00:00:00 2001 From: mattip Date: Tue, 25 Aug 2026 15:59:06 +0300 Subject: [PATCH] fix link bug and change sorting on /changes page --- codespeed/models.py | 1 + .../templates/codespeed/changes_table.html | 2 +- codespeed/tests/test_models.py | 6 +++ codespeed/tests/test_views_data.py | 43 ++++++++++++++++++- codespeed/views_data.py | 12 +++++- 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/codespeed/models.py b/codespeed/models.py index d9b4cb50..45585f22 100644 --- a/codespeed/models.py +++ b/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, diff --git a/codespeed/templates/codespeed/changes_table.html b/codespeed/templates/codespeed/changes_table.html index 2b3d7e9d..4600f3a0 100644 --- a/codespeed/templates/codespeed/changes_table.html +++ b/codespeed/templates/codespeed/changes_table.html @@ -30,7 +30,7 @@ -{% for row in units.rows|dictsort:"bench_name" %} +{% for row in units.rows|dictsort:"bench_name"|dictsort:"bench_source" %} {{ row.bench_name }} {{ row.result|floatformat:units.precission }} diff --git a/codespeed/tests/test_models.py b/codespeed/tests/test_models.py index eb1fc58d..b231c908 100644 --- a/codespeed/tests/test_models.py +++ b/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) diff --git a/codespeed/tests/test_views_data.py b/codespeed/tests/test_views_data.py index 8d320d87..66112bbb 100644 --- a/codespeed/tests/test_views_data.py +++ b/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=, i.e. the + bare name without a '.' 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 + # '.' 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')) diff --git a/codespeed/views_data.py b/codespeed/views_data.py index 74b9ed42..1e72fb64 100644 --- a/codespeed/views_data.py +++ b/codespeed/views_data.py @@ -11,10 +11,20 @@ def parse_benchmark_ident(ben): - """Split a '.' (or bare '') into (name, source='legacy').""" + """Split a '.' (or bare '') 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'