Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 91 additions & 0 deletions tests/webapp/api/test_perfcompare_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@
PerfCompareMwuCache,
PerformanceDatum,
PerformanceDatumReplicate,
PerformanceFramework,
PerformanceSignature,
)
from treeherder.webapp.api import perfcompare_utils
from treeherder.webapp.api.performance_data import LIST_ALL_FRAMEWORKS

pytestmark = pytest.mark.perf

Expand Down Expand Up @@ -1664,3 +1667,91 @@ def test_mwu_cache_recalculates_after_data_change(
assert PerfCompareMwuCache.objects.count() == 2
cache_keys = list(PerfCompareMwuCache.objects.values_list("hash_key", flat=True))
assert cache_keys[0] != cache_keys[1]


def test_perfcompare_results_with_all_framework_param(
client,
create_perf_datum,
test_perf_signature,
test_repository,
try_repository,
eleven_jobs_stored,
test_perfcomp_push,
test_perfcomp_push_2,
test_linux_platform,
):
# Given two frameworks with performance data in different repositories
framework2 = PerformanceFramework.objects.create(name="test_talos_2", enabled=True)

perf_jobs = Job.objects.filter(pk__in=range(1, 11)).order_by("push__time").all()
test_perfcomp_push.time = FOUR_DAYS_AGO
test_perfcomp_push.repository = try_repository
test_perfcomp_push.save()
test_perfcomp_push_2.time = datetime.datetime.now()
test_perfcomp_push_2.save()

base_options = dict(
test="dhtml.html",
has_subtests=False,
extra_options="e10s fission stylo webrender",
measurement_unit="ms",
last_updated=datetime.datetime.now(),
)

base_sig = PerformanceSignature.objects.create(
repository=try_repository,
signature_hash=(20 * "n1"),
framework=framework2,
platform=test_linux_platform,
option_collection=test_perf_signature.option_collection,
suite="a11yr",
**base_options,
)
new_sig = PerformanceSignature.objects.create(
repository=test_repository,
signature_hash=(20 * "n2"),
framework=test_perf_signature.framework,
platform=test_linux_platform,
option_collection=test_perf_signature.option_collection,
suite="b11yr",
**base_options,
)

job = perf_jobs[0]
job.push = test_perfcomp_push
job.save()
perf_datum = PerformanceDatum.objects.create(
value=32.4,
push_timestamp=job.push.time,
job=job,
push=job.push,
repository=try_repository,
signature=base_sig,
)
perf_datum.push.time = job.push.time
perf_datum.push.save()
create_perf_datum(0, perf_jobs[1], test_perfcomp_push_2, new_sig, [40.2])

# When the framework parameter is omitted
query_params = (
f"?base_repository={try_repository.name}&new_repository={test_repository.name}"
f"&new_revision={test_perfcomp_push_2.revision}"
f"&interval=604800&no_subtests=true"
f"&framework={LIST_ALL_FRAMEWORKS}"
)
response = client.get(reverse("perfcompare-results") + query_params)

# Then results are returned across all frameworks
assert response.status_code == 200
results = response.json()
assert len(results) > 0

# And each result has a valid framework_id from its signature model
# And graph links are free of placeholder values
for result in results:
assert result["framework_id"] is not None
assert "None" not in result["graphs_link"]

# And results include data from both frameworks
framework_ids = {result["framework_id"] for result in results}
assert framework_ids == {framework2.id, test_perf_signature.framework_id}
39 changes: 38 additions & 1 deletion tests/webapp/api/test_performance_data_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
PerformanceFramework,
PerformanceSignature,
)
from treeherder.webapp.api.performance_data import PerformanceSummary
from treeherder.webapp.api.performance_data import (
LIST_ALL_FRAMEWORKS,
PerformanceSummary,
)

pytestmark = pytest.mark.perf

Expand Down Expand Up @@ -874,3 +877,37 @@ def test_alert_summary_tasks_get_failure(client, test_perf_alert_summary):
resp = client.get(reverse("performance-alertsummary-tasks"))
assert resp.status_code == 400
assert resp.json() == {"id": ["This field is required."]}


def test_perf_summary_with_all_framework_param(
client,
test_perf_signature,
test_perf_signature_same_hash_different_framework,
test_perf_data,
):
# Given two signatures in different frameworks with performance data
signature1 = test_perf_signature
signature2 = test_perf_signature_same_hash_different_framework

PerformanceDatum.objects.create(
repository=signature2.repository,
push=test_perf_data[0].push,
job=test_perf_data[0].job,
signature=signature2,
value=20.0,
push_timestamp=test_perf_data[0].push_timestamp,
)

# When the framework parameter is omitted from the summary request
query_params = (
f"?repository={signature1.repository.name}"
f"&interval=172800&no_subtests=true"
f"&revision={test_perf_data[0].push.revision}"
f"&framework={LIST_ALL_FRAMEWORKS}"
)
response = client.get(reverse("performance-summary") + query_params)

# Then results are returned for both frameworks
assert response.status_code == 200
framework_ids = {item["framework_id"] for item in response.json()}
assert sorted(framework_ids) == sorted({signature1.framework_id, signature2.framework_id})
15 changes: 12 additions & 3 deletions treeherder/webapp/api/performance_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@
logger = logging.getLogger(__name__)


# Flag to denote that we should return results for all frameworks
LIST_ALL_FRAMEWORKS = -1

@gopar gopar Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using -1 since PKs cannot be negative (unless we set it manually but i didn't see anything related to that here - please correct me if I'm wrong). And in the frontend we can map an "all" option to -1, so we don't have to update the serializers on the backend as well. This solves :sparky comment about using all string.



class PerformanceSignatureViewSet(viewsets.ViewSet):
def list(self, request, project):
repository = models.Repository.objects.get(name=project)
Expand Down Expand Up @@ -928,6 +932,8 @@ def list(self, request):
repository_name = query_params.validated_data["repository"]
interval = query_params.validated_data["interval"]
frameworks = query_params.validated_data["framework"]
if LIST_ALL_FRAMEWORKS in frameworks:
frameworks = []
parent_signature = query_params.validated_data["parent_signature"]
signature = query_params.validated_data["signature"]
no_subtests = query_params.validated_data["no_subtests"]
Expand Down Expand Up @@ -1208,7 +1214,7 @@ class _ComparisonData:
base: _RepoPerfData
new: _RepoPerfData
option_collection_map: dict
framework: int
framework: int | None
push_timestamp: int


Expand Down Expand Up @@ -1236,6 +1242,8 @@ def list(self, request):
new_repo_name = query_params.validated_data["new_repository"]
interval = query_params.validated_data["interval"]
framework = query_params.validated_data["framework"]
if framework == LIST_ALL_FRAMEWORKS:
framework = None
no_subtests = query_params.validated_data["no_subtests"]
base_parent_signature = query_params.validated_data["base_parent_signature"]
new_parent_signature = query_params.validated_data["new_parent_signature"]
Expand Down Expand Up @@ -1517,6 +1525,7 @@ def _build_common_result(comparison_inputs, header, platform):
base_sig_id = base_sig.get("id", None)
new_sig = comparison_inputs.new.signatures_map.get(sig_identifier, {})
new_sig_id = new_sig.get("id", None)
sig_framework_id = base_sig.get("framework_id") or new_sig.get("framework_id")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here:

  • When a framework is provided, base and new signatures are filtered to it, so they always share that framework, so or operation is benign.

  • When omitted (all frameworks), the comparison key doesn't include the framework, so the two sides can potentially differ and this matters for creating the graph links since it'll use one framework for both new/base.

I can try and create the graphs individually? Or another solution?


# Get signature-based properties
if base_sig:
Expand Down Expand Up @@ -1567,7 +1576,7 @@ def _build_common_result(comparison_inputs, header, platform):
"suite": suite,
"test": test,
"is_complete": is_complete,
"framework_id": comparison_inputs.framework,
"framework_id": sig_framework_id,
"option_name": option_name,
"extra_options": extra_options,
"base_repository_name": comparison_inputs.base.repo_name,
Expand All @@ -1583,7 +1592,7 @@ def _build_common_result(comparison_inputs, header, platform):
comparison_inputs.new.repo_name,
comparison_inputs.base.rev,
comparison_inputs.new.rev,
str(comparison_inputs.framework),
str(sig_framework_id),
comparison_inputs.push_timestamp,
str(sig_hash),
),
Expand Down