From f13d0430e852f66754b08039754d847ca79f8d17 Mon Sep 17 00:00:00 2001 From: Nate Armstrong Date: Thu, 15 Oct 2026 19:12:07 -0700 Subject: [PATCH] test(e2e): attach perf case metrics and limits as JUnit properties (#46774) * test(e2e): attach perf case metrics and limits as JUnit properties The perf case only printed its metrics table, so a green run left no numbers behind. Each observed metric and its limit now land as entries on the test's JUnit testcase, named perf_[_p] and perf_[_p]_limit, so project-releaser can collect them * test(e2e): compare perf properties exactly instead of through pytest.approx basedpyright flags pytest.approx as partially unknown in the e2e lint step, so the harness test now uses metric values that are exact in binary floating point --- tests/e2e/perf/setup/observe.py | 32 ++++++++- tests/e2e/perf/test_perf_cases.py | 15 ++++- tests/e2e_harness/perf/setup/test_observe.py | 68 ++++++++++++++++++++ tests/e2e_harness/pytest.ini | 2 +- 4 files changed, 112 insertions(+), 5 deletions(-) create mode 100644 tests/e2e_harness/perf/setup/test_observe.py diff --git a/tests/e2e/perf/setup/observe.py b/tests/e2e/perf/setup/observe.py index af8295e8ffd..a322e5a7443 100644 --- a/tests/e2e/perf/setup/observe.py +++ b/tests/e2e/perf/setup/observe.py @@ -8,6 +8,7 @@ import statistics from collections import Counter from collections.abc import Iterable from dataclasses import dataclass +from itertools import chain from typing import Final from e2e_http import NoBody, get_external, post_external, unwrap @@ -203,10 +204,19 @@ def observe(metric: Metric, percentile_rank: int | None, observation: Observatio @dataclass(frozen=True, slots=True) class Row: - label: str + metric: Metric + percentile: int | None observed: float | None limit: float | None + @property + def label(self) -> str: + return f"{self.metric} percentile {self.percentile}" if self.percentile is not None else self.metric + + @property + def property_name(self) -> str: + return f"perf_{self.metric}" if self.percentile is None else f"perf_{self.metric}_p{self.percentile}" + @property def over(self) -> bool: return self.limit is not None and self.observed is not None and self.observed > self.limit @@ -240,8 +250,14 @@ def evaluate(case: PerformanceTestCase, observation: Observation) -> tuple[list[ rows: list[Row] = [] for metric, rank in keys: limit = limits.get((metric, rank)) - label = f"{metric} percentile {rank}" if rank is not None else metric - rows.append(Row(label=label, observed=observe(metric, rank, observation), limit=limit.max if limit else None)) + rows.append( + Row( + metric=metric, + percentile=rank, + observed=observe(metric, rank, observation), + limit=limit.max if limit else None, + ) + ) exceeded: Final = [ f"{row.label}: observed {row.observed:.4f} > limit {row.limit:.4f}" if row.over and row.observed is not None and row.limit is not None @@ -252,6 +268,16 @@ def evaluate(case: PerformanceTestCase, observation: Observation) -> tuple[list[ return rows, exceeded +def _row_properties(row: Row) -> tuple[tuple[str, float], ...]: + observed: Final = ((row.property_name, row.observed),) if row.observed is not None else () + limit: Final = ((f"{row.property_name}_limit", row.limit),) if row.limit is not None else () + return observed + limit + + +def junit_properties(rows: Iterable[Row]) -> tuple[tuple[str, float], ...]: + return tuple(chain.from_iterable(map(_row_properties, rows))) + + def render(case: PerformanceTestCase, observation: Observation, rows: list[Row]) -> str: by_endpoint: Final = Counter((record.endpoint, record.stream) for record in observation.records) failures: Final = [record for record in observation.records if not record.ok][:10] diff --git a/tests/e2e/perf/test_perf_cases.py b/tests/e2e/perf/test_perf_cases.py index a69c6bd63cc..d077cb9f8c7 100644 --- a/tests/e2e/perf/test_perf_cases.py +++ b/tests/e2e/perf/test_perf_cases.py @@ -17,6 +17,7 @@ from cases import CASES from setup.observe import ( Observation, evaluate, + junit_properties, read_guardrail_calls, read_mock_requests, render, @@ -48,7 +49,13 @@ def _spend_logged_share(proxy: ProxyClient, call_ids: tuple[str, ...]) -> float @pytest.mark.parametrize("case", CASES, ids=[case.name for case in CASES]) -def test_perf_case(case: PerformanceTestCase, proxy: ProxyClient, idp: Keycloak, resources: ResourceManager) -> None: +def test_perf_case( + case: PerformanceTestCase, + proxy: ProxyClient, + idp: Keycloak, + resources: ResourceManager, + request: pytest.FixtureRequest, +) -> None: plan: Final = build_plan(case) mock_urls: Final = {provider: mock_url(provider) for provider in MOCK_PROVIDERS} for url in (*mock_urls.values(), guardrail_url()): @@ -94,6 +101,12 @@ def test_perf_case(case: PerformanceTestCase, proxy: ProxyClient, idp: Keycloak, gateway_cpu_seconds=cpu.seconds, ) rows, exceeded_limits = evaluate(case, observation) + item: Final = request.node # pyright: ignore[reportUnknownMemberType, reportUnknownVariableType] # pytest: untyped + assert isinstance(item, pytest.Item) + item.user_properties[:] = [ + entry for entry in item.user_properties if not entry[0].startswith("perf_") + ] # rebind-ok: a --reruns retry replaces the first attempt's numbers instead of appending a second set + item.user_properties.extend(junit_properties(rows)) print(render(case, observation, rows)) print( f" phases: setup {run_started - setup_started:.0f}s, locust {run_finished - run_started:.0f}s, " diff --git a/tests/e2e_harness/perf/setup/test_observe.py b/tests/e2e_harness/perf/setup/test_observe.py new file mode 100644 index 00000000000..998d7d9b87e --- /dev/null +++ b/tests/e2e_harness/perf/setup/test_observe.py @@ -0,0 +1,68 @@ +from __future__ import annotations + +from dataclasses import replace +from typing import Final + +from cases.small_bodies_5rps import CASE +from memory_readings import RssCapture +from setup.datastore import DbCounters +from setup.observe import Observation, evaluate, junit_properties +from setup.performance_test_case import Limit +from setup.wire import WireRecord + +NO_RSS: Final = RssCapture(readings=(), failures=()) + + +def served(seq: int, total_s: float) -> WireRecord: + return WireRecord( + seq=seq, + marker=f"perfreq-{seq:06d}-n", + caller="key-a1", + endpoint="chat_completions", + stream=False, + blocked=False, + started_at=0.0, + status=200, + header_s=total_s, + first_byte_s=total_s, + first_token_s=None, + total_s=total_s, + text="", + ok=True, + mock_delay_s=1.0, + call_id=None, + error=None, + ) + + +def test_junit_properties_carry_each_observed_metric_and_each_limit_including_unobserved_ones() -> None: + case: Final = replace( + CASE, + limits=( + Limit("gateway_added_seconds", percentile=50, max=0.625), + Limit("redis_calls_per_request", max=35), + ), + ) + observation: Final = Observation( + records=(served(0, 1.25), served(1, 1.5), served(2, 1.75)), + mock_requests=(), + guardrail_calls=(), + rss_before=NO_RSS, + rss_after=NO_RSS, + wall_seconds=3.0, + db_before=DbCounters(commits=100), + db_after=DbCounters(commits=130), + ) + rows, _ = evaluate(case, observation) + + properties: Final = junit_properties(rows) + + assert len({name for name, _ in properties}) == len(properties) + assert dict(properties) == { + "perf_unexpected_status_count": 0.0, + "perf_gateway_added_seconds_p50": 0.5, + "perf_gateway_added_seconds_p50_limit": 0.625, + "perf_gateway_added_seconds_p99": 0.75, + "perf_postgres_calls_per_request": 10.0, + "perf_redis_calls_per_request_limit": 35.0, + } diff --git a/tests/e2e_harness/pytest.ini b/tests/e2e_harness/pytest.ini index 54d2630acc6..a29166d31bd 100644 --- a/tests/e2e_harness/pytest.ini +++ b/tests/e2e_harness/pytest.ini @@ -3,7 +3,7 @@ # (`from e2e_http import ...`, `from batch_cleanup import ...`) exactly as the suites # do, so tests/e2e and each suite folder that owns a module under test go on the path. addopts = --strict-markers --strict-config -p no:cacheprovider -pythonpath = ../e2e ../e2e/batches ../e2e/guardrails ../e2e/logging +pythonpath = ../e2e ../e2e/batches ../e2e/guardrails ../e2e/logging ../e2e/perf markers = covers(cell_id, *, exercised_on=()): coverage-registry cell(s) a test covers; exercised here only to prove the collector and the JUnit properties read it cli_determinism: drives the real claude CLI for several seconds