From 1dc7ba2ce11204cdca8a0f0027334078621b6fee Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Wed, 27 May 2026 17:50:07 -0700 Subject: [PATCH 1/4] initial commit --- python/CHANGELOG.md | 1 + .../guides/pytest_plugin/configuration.md | 3 + .../guides/pytest_plugin/running_modes.md | 82 +++++ python/lib/sift_client/_internal/urls.py | 55 +++ .../_tests/pytest_plugin/conftest.py | 2 +- .../pytest_plugin/test_terminal_output.py | 197 ++++++++++ python/lib/sift_client/_tests/test_urls.py | 76 ++++ python/lib/sift_client/client.py | 27 ++ python/lib/sift_client/pytest_plugin.py | 346 ++++++++++++++++++ .../sift_client/transport/base_connection.py | 6 + .../util/test_results/context_manager.py | 44 +++ 11 files changed, 838 insertions(+), 1 deletion(-) create mode 100644 python/lib/sift_client/_internal/urls.py create mode 100644 python/lib/sift_client/_tests/pytest_plugin/test_terminal_output.py create mode 100644 python/lib/sift_client/_tests/test_urls.py diff --git a/python/CHANGELOG.md b/python/CHANGELOG.md index ef087d52b..e65718d54 100644 --- a/python/CHANGELOG.md +++ b/python/CHANGELOG.md @@ -16,6 +16,7 @@ Highlights: - **Pass/fail mapping.** Every pytest outcome (pass, assertion failure, exception, skip, xfail, hard exit) maps to a `TestStatus` and propagates to parent steps and the report. `step.measure(...)` returns a pass/fail boolean without raising, so all measurements land in the report even when one fails; `step.fail_if_measurements_failed()` fails the test at the end without adding assertion noise to `error_info`. - **Assertion messages as error info.** Assertion failure messages are reported as the step's error info. - **Git metadata.** Repo, branch, and commit are captured on the report automatically. +- **Terminal output.** The plugin prints a session header with the SDK version and active mode, and an end-of-run `Sift report` panel showing the test case, outcome, step and measurement breakdowns (color-coded), test system/operator, plus a link to the report (online), the saved log and upload command (offline), or a disabled note. Both suppress under `-q`. `SiftClient.app_url` exposes the web-app origin; set `sift_report_url_base` for on-prem or custom deployments. `--sift-open-report` opens the report in a browser at session end. See the [Pytest Plugin guide](https://github.com/sift-stack/sift/blob/main/python/docs/guides/pytest_plugin/index.md) and the runnable quickstart example for full configuration. diff --git a/python/docs/guides/pytest_plugin/configuration.md b/python/docs/guides/pytest_plugin/configuration.md index 6ed78f931..3b3151111 100644 --- a/python/docs/guides/pytest_plugin/configuration.md +++ b/python/docs/guides/pytest_plugin/configuration.md @@ -132,6 +132,8 @@ def sift_client() -> SiftClient: | `--sift-disabled` | off | Skip Sift entirely. Nothing contacts the API and no log file is written; `step.measure(...)` still evaluates bounds and returns a real pass/fail boolean. Also honored via `SIFT_DISABLED=1`. Supersedes every other flag (disabled wins over offline). | | `--sift-log-file=` | temp file | Where the JSONL log of create/update calls goes. With a log file set, the plugin spawns an `import-test-result-log --incremental` worker that polls the file and replays entries against Sift while the run is in flight. Pass `false` to disable the file entirely; create/update calls then go straight to the API synchronously during tests. Incompatible with `--sift-offline` since offline mode needs the log file as its sole sink. | | `--no-sift-git-metadata` | git metadata on | Skip capturing git repo/branch/commit on the report's metadata. | +| `--sift-report-url-base=` | derived from REST URI | Web-app origin used to build the clickable report link in the terminal footer (e.g. `https://app.siftstack.com`). Set this for on-prem or custom deployments whose API host can't be mapped to a frontend automatically. Also honored via the `SIFT_APP_URL` environment variable. When unset, the link is derived from the REST URI for known Sift hosts. | +| `--sift-open-report` | off | Open the resulting report in a browser at session end. Online mode only; a no-op when the report URL can't be resolved. Intended for local development. | These can be passed permanently via `addopts`: @@ -158,6 +160,7 @@ CLI flags, when passed, override the ini values. | `sift_module_step` | bool (default `true`) | _(ini-only)_. Opens a parent step for each test module (file). | | `sift_class_step` | bool (default `true`) | _(ini-only)_. Opens a parent step for each test class, including nested classes. | | `sift_parametrize_nesting` | bool (default `true`) | _(ini-only)_. Clusters parametrized tests under shared parents (`test_x`, `axis=value`) instead of flat leaves (`test_x[value]`). | +| `sift_open_report` | bool (default `false`) | `--sift-open-report` | ```toml title="pyproject.toml" [tool.pytest.ini_options] diff --git a/python/docs/guides/pytest_plugin/running_modes.md b/python/docs/guides/pytest_plugin/running_modes.md index e69688cf1..9289428e4 100644 --- a/python/docs/guides/pytest_plugin/running_modes.md +++ b/python/docs/guides/pytest_plugin/running_modes.md @@ -25,6 +25,88 @@ pytest --sift-log-file=./sift-results.jsonl Pass both flags and disabled wins: it skips Sift entirely and supersedes every other setting. +## Terminal output + +Each run prints a header with the SDK version and active mode, and an end-of-run +`Sift report` panel summarizing the outcome. Both are suppressed under `-q`. The +panel is color-coded when the terminal supports it (green pass, red +failure/error, yellow skip, cyan link) and plain text otherwise (`--color=no`, +captured output, CI logs). + +The section title carries the report name (truncated if long). The `Steps` row +tallies every step in the report by final status, so it counts substeps and the +package/module/class/parametrize grouping steps too — its totals are expected to +exceed pytest's own test count. The `Measurements` row tallies recorded +measurements (`step.measure(...)`) and is omitted when there are none. The +`Test case` and `System` rows echo the report's test case, test system, and +operator. + +**Online** shows the report metadata, step and measurement breakdowns, and a +clickable link. The web host is derived from the REST URI for known Sift hosts; +for on-prem or custom deployments set `--sift-report-url-base` +(ini: `sift_report_url_base`, env: `SIFT_APP_URL`). Add `--sift-open-report` to +open the report in a browser at session end. + +```text +============================= test session starts ============================== +platform linux -- Python 3.11.8, pytest-8.3.2, pluggy-1.5.0 +Sift: sift-stack-py 0.17.0 — online mode +collected 12 items + +tests/test_battery.py ........ [ 66%] +tests/test_thermal.py .... [100%] + +================ Sift report · pytest tests/ 2026-05-27T22:44:23Z ============== + Test case pytest tests/ + Status PASSED online · sift-stack-py 0.17.0 + Steps 14 passed + Measurements 42 passed + System ci-runner-7 · cibot + Log file /tmp/sift-a1b2c3.jsonl + Report https://app.siftstack.com/test-results/0193f1a2-7c44-7e5b-9b1a-2f6c0d9e84aa +============================== 12 passed in 3.45s ============================== +``` + +If the background uploader doesn't finish, the panel still links the report and +flags that it may be incomplete: + +```text +================ Sift report · pytest tests/ 2026-05-27T22:44:23Z ============== + Test case pytest tests/ + Status FAILED online · sift-stack-py 0.17.0 + Steps 11 passed · 2 failed · 1 error + Measurements 40 passed · 3 failed + System ci-runner-7 · cibot + Log file /tmp/sift-a1b2c3.jsonl + Report https://app.siftstack.com/test-results/0193f1a2-7c44-7e5b-9b1a-2f6c0d9e84aa + may be incomplete — finish with: import-test-result-log /tmp/sift-a1b2c3.jsonl +``` + +When the web host can't be resolved and no override is set, the `Report` row +shows the report id instead of a link. + +**Offline** shows the metadata and breakdowns, then the upload command under a +small rule (the log path is part of the command): + +```text +================ Sift report · pytest tests/ 2026-05-27T22:44:23Z ============== + Test case pytest tests/ + Status PASSED offline · not uploaded + Steps 14 passed + Measurements 42 passed + System ci-runner-7 · cibot + Log file ./run.jsonl +------------------------------ to upload to Sift ------------------------------- + >> import-test-result-log ./run.jsonl +``` + +**Disabled** notes that no report was created: + +```text +===================================== Sift ===================================== +Sift disabled — no test report created. +``` + ## Online mode (default) `report_context` resolves `client_has_connection` at session start. The default diff --git a/python/lib/sift_client/_internal/urls.py b/python/lib/sift_client/_internal/urls.py new file mode 100644 index 000000000..a87f3b53c --- /dev/null +++ b/python/lib/sift_client/_internal/urls.py @@ -0,0 +1,55 @@ +"""Helpers for turning Sift API endpoints into web-app (frontend) URLs. + +The Sift frontend can be hosted on several domains and the backend exposes no +field for its own URL, so the frontend origin is derived client-side from the +API host. This table mirrors the canonical mapping used by the Grafana +datasource (sift-stack/sift-grafana-datasource, +``src/components/sharelink/getFrontendHostnameDefaults.ts``). Hosts outside the +table (on-prem and custom deployments) require an explicit override. +""" + +from __future__ import annotations + +from urllib.parse import urlparse + +# API host (host[:port], no scheme) -> frontend origin (with scheme). +_API_HOST_TO_FRONTEND_ORIGIN: dict[str, str] = { + "api.siftstack.com": "https://app.siftstack.com", + "gov.api.siftstack.com": "https://gov.siftstack.com", +} + + +def _origin(url: str) -> str: + """Normalize a URL or bare host into a ``scheme://host[:port]`` origin. + + Bare hosts (no scheme) are assumed to be ``https``. + """ + candidate = url if "://" in url else f"https://{url}" + parsed = urlparse(candidate) + return f"{parsed.scheme}://{parsed.netloc}".rstrip("/") + + +def _host(url: str) -> str: + """Extract ``host[:port]`` from a URL or bare host string.""" + candidate = url if "://" in url else f"https://{url}" + return urlparse(candidate).netloc + + +def frontend_origin_for_api(api_base_url: str, override: str | None = None) -> str | None: + """Return the Sift web-app origin for a given API base URL. + + Args: + api_base_url: The REST API base URL (e.g. ``https://api.siftstack.com``). + override: An explicit frontend origin (host or full URL) to use instead + of the derived value. Set this for on-prem or custom deployments + whose API host isn't in the built-in mapping. + + Returns: + The frontend origin (e.g. ``https://app.siftstack.com``), or ``None`` + when no override is given and the API host isn't recognized. + """ + if override: + return _origin(override) + if not api_base_url: + return None + return _API_HOST_TO_FRONTEND_ORIGIN.get(_host(api_base_url)) diff --git a/python/lib/sift_client/_tests/pytest_plugin/conftest.py b/python/lib/sift_client/_tests/pytest_plugin/conftest.py index 7afee768d..ba775e04b 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/conftest.py +++ b/python/lib/sift_client/_tests/pytest_plugin/conftest.py @@ -29,7 +29,7 @@ import pytest -_SIFT_ENV_VARS = ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI", "SIFT_DISABLED") +_SIFT_ENV_VARS = ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI", "SIFT_DISABLED", "SIFT_APP_URL") @pytest.fixture diff --git a/python/lib/sift_client/_tests/pytest_plugin/test_terminal_output.py b/python/lib/sift_client/_tests/pytest_plugin/test_terminal_output.py new file mode 100644 index 000000000..4985fd9fb --- /dev/null +++ b/python/lib/sift_client/_tests/pytest_plugin/test_terminal_output.py @@ -0,0 +1,197 @@ +"""Tests for the plugin's terminal output (session header + report footer). + +Driven through inner pytester sessions. Online output is exercised by the +``SiftClient.app_url`` unit tests (``_tests/test_urls.py``) since a live link +needs a real backend; here we cover the deterministic disabled/offline footers +and the ``-q`` suppression both share. +""" + +from __future__ import annotations + +from collections import Counter +from types import SimpleNamespace +from typing import TYPE_CHECKING, Callable + +from sift_client._internal.low_level_wrappers._test_results_log import LogTracking +from sift_client.pytest_plugin import ( + _measurement_segments, + _resolve_real_report_id, + _step_count_segments, +) +from sift_client.sift_types.test_report import TestStatus + +if TYPE_CHECKING: + from pathlib import Path + + import pytest + + +class TestStepCountSegments: + def test_lists_nonzero_statuses_in_order_with_color(self) -> None: + counts = Counter( + {TestStatus.PASSED: 4, TestStatus.FAILED: 2, TestStatus.SKIPPED: 1} + ) + assert _step_count_segments(counts) == [ + ("4 passed", {"green": True}), + ("2 failed", {"red": True}), + ("1 skipped", {"yellow": True}), + ] + + def test_error_and_aborted_are_red(self) -> None: + counts = Counter({TestStatus.ERROR: 1, TestStatus.ABORTED: 1}) + assert _step_count_segments(counts) == [ + ("1 error", {"red": True}), + ("1 aborted", {"red": True}), + ] + + def test_empty_is_empty(self) -> None: + assert _step_count_segments(Counter()) == [] + + +class TestMeasurementSegments: + def test_passed_green_failed_red(self) -> None: + assert _measurement_segments(Counter({True: 2, False: 1})) == [ + ("2 passed", {"green": True}), + ("1 failed", {"red": True}), + ] + + def test_empty_is_empty(self) -> None: + assert _measurement_segments(Counter()) == [] + + +class TestResolveRealReportId: + """``_resolve_real_report_id`` maps the footer to the real server report id.""" + + def test_synchronous_online_uses_report_id_directly(self) -> None: + # No log file, non-simulated report (``--sift-log-file=false`` path). + context = SimpleNamespace( + report=SimpleNamespace(id_="real-123", is_simulated=False), + log_file=None, + ) + assert _resolve_real_report_id(context) == "real-123" + + def test_incremental_resolves_via_sidecar(self, tmp_path: Path) -> None: + log_file = tmp_path / "run.jsonl" + log_file.write_text("") + LogTracking(id_map={"sim-1": "real-1"}).save(log_file) + context = SimpleNamespace( + report=SimpleNamespace(id_="sim-1", is_simulated=True), + log_file=log_file, + ) + assert _resolve_real_report_id(context) == "real-1" + + def test_empty_report_id_returns_none(self) -> None: + # An unset/empty id must not produce a ``/test-results/`` link. + context = SimpleNamespace( + report=SimpleNamespace(id_="", is_simulated=False), + log_file=None, + ) + assert _resolve_real_report_id(context) is None + + def test_incremental_unmapped_returns_none(self, tmp_path: Path) -> None: + # Worker died before mapping the report: no sidecar entry. + log_file = tmp_path / "run.jsonl" + log_file.write_text("") + context = SimpleNamespace( + report=SimpleNamespace(id_="sim-1", is_simulated=True), + log_file=log_file, + ) + assert _resolve_real_report_id(context) is None + + +class TestHeader: + def test_header_shows_version_and_mode( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + """The session header reports the SDK version and the active mode.""" + write_plugin_conftest() + pytester.makepyfile("def test_runs(step): step.measure(name='v', value=1.0)") + result = pytester.runpytest_subprocess("--sift-disabled") + result.assert_outcomes(passed=1) + result.stdout.fnmatch_lines(["*sift-stack-py*disabled mode*"]) + + def test_header_suppressed_under_quiet( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + """``-q`` suppresses the header, matching pytest's own platform header.""" + write_plugin_conftest() + pytester.makepyfile("def test_runs(step): step.measure(name='v', value=1.0)") + result = pytester.runpytest_subprocess("--sift-disabled", "-q") + result.assert_outcomes(passed=1) + result.stdout.no_fnmatch_line("*sift-stack-py*") + + +class TestDisabledFooter: + def test_footer_notes_no_report( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + write_plugin_conftest() + pytester.makepyfile("def test_runs(step): step.measure(name='v', value=1.0)") + result = pytester.runpytest_subprocess("--sift-disabled") + result.assert_outcomes(passed=1) + result.stdout.fnmatch_lines(["*Sift disabled*no test report created*"]) + + def test_footer_suppressed_under_quiet( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + write_plugin_conftest() + pytester.makepyfile("def test_runs(step): step.measure(name='v', value=1.0)") + result = pytester.runpytest_subprocess("--sift-disabled", "-q") + result.assert_outcomes(passed=1) + result.stdout.no_fnmatch_line("*Sift disabled*") + + +class TestOfflineFooter: + def test_footer_shows_log_path_and_replay_command( + self, + pytester: pytest.Pytester, + tmp_path: Path, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + """Offline footer points at the saved log file and the replay command.""" + log_path = tmp_path / "run.jsonl" + write_plugin_conftest() + pytester.makepyfile("def test_runs(step): step.measure(name='v', value=1.0)") + result = pytester.runpytest_subprocess("--sift-offline", f"--sift-log-file={log_path}") + result.assert_outcomes(passed=1) + result.stdout.fnmatch_lines( + [ + "*Test case*", + "*Status*offline*not uploaded*", + "*Steps*passed*", + "*Measurements*1 passed*", + "*System*", + f"*Log file*{log_path}", + "*to upload to Sift*", + f"*import-test-result-log {log_path}", + ] + ) + + def test_sift_open_report_flag_is_accepted_offline( + self, + pytester: pytest.Pytester, + tmp_path: Path, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + """``--sift-open-report`` is a no-op offline (no resolvable URL) and never errors.""" + log_path = tmp_path / "run.jsonl" + write_plugin_conftest() + pytester.makepyfile("def test_runs(step): step.measure(name='v', value=1.0)") + result = pytester.runpytest_subprocess( + "--sift-offline", f"--sift-log-file={log_path}", "--sift-open-report" + ) + result.assert_outcomes(passed=1) diff --git a/python/lib/sift_client/_tests/test_urls.py b/python/lib/sift_client/_tests/test_urls.py new file mode 100644 index 000000000..d3ac5dc75 --- /dev/null +++ b/python/lib/sift_client/_tests/test_urls.py @@ -0,0 +1,76 @@ +"""Tests for web-app URL derivation (``_internal/urls.py`` and ``SiftClient.app_url``).""" + +from __future__ import annotations + +import pytest + +from sift_client import SiftClient, SiftConnectionConfig +from sift_client._internal.urls import frontend_origin_for_api + + +class TestFrontendOriginForApi: + @pytest.mark.parametrize( + ("api_base_url", "expected"), + [ + ("https://api.siftstack.com", "https://app.siftstack.com"), + ("https://gov.api.siftstack.com", "https://gov.siftstack.com"), + ("http://localhost:8080", "http://localhost:3000"), + ("http://host.docker.internal:8080", "http://localhost:3000"), + # Bare host (no scheme) resolves the same as the full URL. + ("api.siftstack.com", "https://app.siftstack.com"), + ], + ) + def test_known_hosts(self, api_base_url: str, expected: str) -> None: + assert frontend_origin_for_api(api_base_url) == expected + + def test_unknown_host_returns_none(self) -> None: + assert frontend_origin_for_api("https://api.acme.example.com") is None + + def test_empty_returns_none(self) -> None: + assert frontend_origin_for_api("") is None + + def test_override_wins_over_derivation(self) -> None: + # Override applies even for a known host. + assert ( + frontend_origin_for_api("https://api.siftstack.com", override="https://app.acme.test") + == "https://app.acme.test" + ) + + def test_override_normalizes_bare_host(self) -> None: + assert ( + frontend_origin_for_api("https://api.acme.example.com", override="sift.acme.test") + == "https://sift.acme.test" + ) + + +class TestSiftClientAppUrl: + def _client(self, rest_url: str, app_url: str | None = None) -> SiftClient: + return SiftClient( + connection_config=SiftConnectionConfig( + api_key="k", + grpc_url="grpc-api.siftstack.com:443", + rest_url=rest_url, + ), + app_url=app_url, + ) + + def test_derives_from_known_rest_host(self) -> None: + assert self._client("https://api.siftstack.com").app_url == "https://app.siftstack.com" + + def test_unknown_host_without_override_is_none(self) -> None: + assert self._client("https://api.acme.example.com").app_url is None + + def test_override_used_for_unknown_host(self) -> None: + client = self._client("https://api.acme.example.com", app_url="https://sift.acme.test") + assert client.app_url == "https://sift.acme.test" + + def test_override_from_connection_config(self) -> None: + client = SiftClient( + connection_config=SiftConnectionConfig( + api_key="k", + grpc_url="grpc-api.siftstack.com:443", + rest_url="https://api.acme.example.com", + app_url="https://sift.acme.test", + ) + ) + assert client.app_url == "https://sift.acme.test" diff --git a/python/lib/sift_client/client.py b/python/lib/sift_client/client.py index ff574adba..d77aff6c0 100644 --- a/python/lib/sift_client/client.py +++ b/python/lib/sift_client/client.py @@ -1,5 +1,6 @@ from __future__ import annotations +from sift_client._internal.urls import frontend_origin_for_api from sift_client.resources import ( AssetsAPI, AssetsAPIAsync, @@ -124,6 +125,7 @@ def __init__( grpc_url: str | None = None, rest_url: str | None = None, connection_config: SiftConnectionConfig | None = None, + app_url: str | None = None, ): """Initialize the SiftClient with specific connection parameters or a connection_config. @@ -132,6 +134,10 @@ def __init__( grpc_url: The Sift gRPC API URL. rest_url: The Sift REST API URL. connection_config: A SiftConnectionConfig object to configure the connection behavior of the SiftClient. + app_url: The Sift web-app origin (e.g. ``https://app.siftstack.com``). + Set this for on-prem or custom deployments whose API host can't be + mapped to a frontend automatically; see the ``app_url`` property. + A value here takes precedence over ``connection_config.app_url``. """ if not (api_key and grpc_url and rest_url) and not connection_config: raise ValueError( @@ -152,6 +158,12 @@ def __init__( WithGrpcClient.__init__(self, grpc_client=grpc_client) WithRestClient.__init__(self, rest_client=rest_client) + # Explicit web-app origin override; falls back to the connection config's + # value, then to host-based derivation in the ``app_url`` property. + self._app_url: str | None = app_url or ( + connection_config.app_url if connection_config else None + ) + # When set, test-results writes return synthesized responses without # contacting Sift. Read by `TestResultsAPIAsync._simulate`. Used by the # pytest plugin's ``--sift-disabled`` mode. @@ -198,3 +210,18 @@ def grpc_client(self) -> GrpcClient: def rest_client(self) -> RestClient: """The REST client used by the SiftClient for making REST API calls.""" return self._rest_client + + @property + def app_url(self) -> str | None: + """The Sift web-app origin for this client, or None if it can't be determined. + + Uses the explicit override passed at construction when set, otherwise + derives the origin from the REST host for known Sift deployments (e.g. + ``https://api.siftstack.com`` -> ``https://app.siftstack.com``). Returns + None for unrecognized hosts with no override. + + # TODO: Add a ``WithAppPage`` mixin on BaseType so resources (TestReport, + # Run, ...) can expose their own web-app link from ``_client.app_url`` plus + # a per-type path, instead of callers assembling paths by hand. + """ + return frontend_origin_for_api(self.rest_client.base_url, override=self._app_url) diff --git a/python/lib/sift_client/pytest_plugin.py b/python/lib/sift_client/pytest_plugin.py index 09aca5e33..0ede078ed 100644 --- a/python/lib/sift_client/pytest_plugin.py +++ b/python/lib/sift_client/pytest_plugin.py @@ -15,6 +15,7 @@ from sift_client.sift_types.test_report import ErrorInfo, TestStatus from sift_client.util.test_results import ReportContext from sift_client.util.test_results.context_manager import ( + _quiet_fork_stderr, format_assertion_message, format_truncated_traceback, ) @@ -42,6 +43,12 @@ class SiftPytestStepDrainError(RuntimeError): REPORT_CONTEXT: Any = None +# Set at session end with the resolved (real) report id/URL when online and +# uploaded. Read from a project's conftest in a later hook (e.g. +# ``pytest_unconfigure``) to post the link, write a file, etc. +SIFT_REPORT_ID_STASH_KEY = pytest.StashKey[str]() +SIFT_REPORT_URL_STASH_KEY = pytest.StashKey[str]() + _STASH_MISSING = object() _PARAMETRIZE_PATH_KEY = pytest.StashKey[Tuple[str, ...]]() @@ -297,6 +304,33 @@ class _Option: "this ini value.", ) +_REPORT_URL_BASE = _Option( + cli_flag="--sift-report-url-base", + ini_name="sift_report_url_base", + cli_help="Sift web-app origin used to build the clickable report link in the " + "terminal footer (e.g. https://app.siftstack.com). Set this for on-prem or " + "custom deployments whose API host can't be mapped to a frontend " + "automatically. Also honored via the SIFT_APP_URL env var. When unset, the " + "link is derived from the REST URI for known Sift hosts.", + ini_help="Default for --sift-report-url-base. The Sift web-app origin used to " + "build the report link in the terminal footer. Also honored via the " + "SIFT_APP_URL env var. When unset, the link is derived from the REST URI for " + "known Sift hosts.", +) + +_OPEN = _Option( + cli_flag="--sift-open-report", + ini_name="sift_open_report", + action="store_true", + cli_help="Open the resulting Sift test report in a browser at session end. " + "Online mode only; no-op when the report URL can't be resolved. Intended for " + "local development.", + ini_help="When true, open the report in a browser at session end (online only). " + "Defaults to false.", + ini_type="bool", + ini_default=False, +) + _AUTOUSE = _Option( ini_name="sift_autouse", ini_help="Default for the Sift autouse fixtures (report_context, step, " @@ -350,6 +384,8 @@ class _Option: _DISABLED, _GRPC_URI, _REST_URI, + _REPORT_URL_BASE, + _OPEN, _AUTOUSE, _PACKAGE_STEP, _MODULE_STEP, @@ -445,6 +481,311 @@ def _is_disabled(pytestconfig: pytest.Config | None) -> bool: return os.getenv("SIFT_DISABLED", "").lower() in ("1", "true", "yes") +def _sdk_version() -> str: + """Return the installed ``sift_stack_py`` version, or ``"unknown"``.""" + from importlib.metadata import PackageNotFoundError, version + + try: + return version("sift_stack_py") + except PackageNotFoundError: + return "unknown" + + +def _mode_label(config: pytest.Config) -> str: + """Resolve the active mode for the terminal header: disabled > offline > online.""" + if _is_disabled(config): + return "disabled" + if _is_offline(config): + return "offline" + return "online" + + +def pytest_report_header(config: pytest.Config) -> str | None: + """Emit a session-start header with the SDK version and active mode. + + Suppressed under ``-q`` (negative verbosity), matching how pytest hides its + own platform/plugin header. + """ + if config.get_verbosity() < 0: + return None + return f"Sift: sift-stack-py {_sdk_version()} — {_mode_label(config)} mode" + + +def _resolve_real_report_id(context: Any) -> str | None: + """Resolve the real server-side report id for the online footer link. + + In synchronous online mode (``--sift-log-file=false``) the report is created + directly against the API, so ``report.id_`` is already the real id. In the + default incremental mode the report is created through the simulate path + (a client-side UUID) and the background worker maps it to the real id on + replay, recording it in the ``.tracking`` sidecar's ``id_map``. By the + time this footer runs the session-scoped report context has torn down and + the worker has drained, so the sidecar is final. + + Returns ``None`` when the worker never mapped the report (e.g. it died before + replaying the create), meaning no real report exists to link. + """ + report = context.report + if not report.id_: + # No id was ever assigned (unset/empty); nothing to link. + return None + sim_id = str(report.id_) + if not getattr(report, "is_simulated", False): + return sim_id + log_file = getattr(context, "log_file", None) + if log_file is None: + return None + from sift_client._internal.low_level_wrappers._test_results_log import LogTracking + + return LogTracking.load(log_file).id_map.get(sim_id) + + +_LABEL_WIDTH = 13 + + +def _sift_kv(terminalreporter: Any, label: str, value: str, **value_markup: bool) -> None: + """Write an indented ``label value`` row, bolding the label. + + ``value_markup`` (e.g. ``green=True``, ``cyan=True``) styles only the value. + Color is dropped automatically when the terminal has no markup (not a TTY or + ``--color=no``), so captured/CI output stays plain text. + """ + terminalreporter.write(" ") + terminalreporter.write(f"{label:<{_LABEL_WIDTH}}", bold=True) + terminalreporter.write_line(value, **value_markup) + + +# Step-count breakdown order and labels for the footer's "Steps" row. +_STEP_COUNT_ORDER: tuple[tuple[TestStatus, str], ...] = ( + (TestStatus.PASSED, "passed"), + (TestStatus.FAILED, "failed"), + (TestStatus.ERROR, "error"), + (TestStatus.ABORTED, "aborted"), + (TestStatus.SKIPPED, "skipped"), + (TestStatus.IN_PROGRESS, "in progress"), +) + + +# Per-status color for the footer's step breakdown: green pass, red +# failure/error/abort, yellow skip; in-progress (and anything else) stays plain. +_STEP_STATUS_MARKUP: dict[TestStatus, dict[str, bool]] = { + TestStatus.PASSED: {"green": True}, + TestStatus.FAILED: {"red": True}, + TestStatus.ERROR: {"red": True}, + TestStatus.ABORTED: {"red": True}, + TestStatus.SKIPPED: {"yellow": True}, +} + + +def _step_count_segments(counts: Any) -> list[tuple[str, dict[str, bool]]]: + """Build ``(text, markup)`` segments for a step tally, non-zero only.""" + return [ + (f"{counts.get(status, 0)} {label}", _STEP_STATUS_MARKUP.get(status, {})) + for status, label in _STEP_COUNT_ORDER + if counts.get(status, 0) + ] + + +def _measurement_segments(counts: Any) -> list[tuple[str, dict[str, bool]]]: + """Build ``(text, markup)`` segments for a measurement tally, non-zero only.""" + segments: list[tuple[str, dict[str, bool]]] = [] + if counts.get(True, 0): + segments.append((f"{counts[True]} passed", {"green": True})) + if counts.get(False, 0): + segments.append((f"{counts[False]} failed", {"red": True})) + return segments + + +def _write_count_row( + terminalreporter: Any, label: str, segments: list[tuple[str, dict[str, bool]]] +) -> None: + """Write a ``label a · b · c`` row, applying each segment's color markup.""" + terminalreporter.write(" ") + terminalreporter.write(f"{label:<{_LABEL_WIDTH}}", bold=True) + for index, (text, markup) in enumerate(segments): + if index: + terminalreporter.write(" · ") + terminalreporter.write(text, **markup) + terminalreporter.write_line("") + + +def _report_panel_title(report: Any, terminalreporter: Any) -> str: + """``Sift report · `` for the section rule, truncated to the terminal width. + + The report name embeds a timestamp (and, for invocation-based runs, the + pytest args), so a long name is truncated with an ellipsis to keep the + separator line from wrapping. + """ + base = "Sift report" + name = getattr(report, "name", None) + if not name: + return base + title = f"{base} · {name}" + fullwidth = getattr(getattr(terminalreporter, "_tw", None), "fullwidth", 80) + # Reserve room for the separator characters and spaces write_sep adds. + limit = max(len(base), fullwidth - 8) + if len(title) > limit: + title = title[: limit - 1] + "…" + return title + + +def _maybe_open_report(url: str) -> None: + """Best-effort open the report URL in a browser (for ``--sift-open-report``). + + Skipped on CI or non-interactive sessions so a committed ``sift_open_report`` + setting can't spawn a browser on a headless agent; the flag is meant for + local development. + """ + import sys + import webbrowser + + if os.environ.get("CI") or not sys.stdout.isatty(): + return + try: + # webbrowser.open forks/execs the platform opener while the gRPC client's + # background threads are live; redirect fd 2 across the fork to swallow + # gRPC's prefork notice (same treatment as the plugin's other fork sites). + with _quiet_fork_stderr(): + webbrowser.open(url) + except Exception: + # Headless / no browser available: opening is a convenience, never fatal. + pass + + +def pytest_terminal_summary(terminalreporter: Any, exitstatus: int, config: pytest.Config) -> None: + """Emit a session-end Sift report summary, adapting per mode. + + The printed panel is suppressed under ``-q``, but programmatic side effects + (stashing the report ref for ``conftest.py``, ``--sift-open-report``) still run so + other plugins and CI steps can consume the result. The panel shows the + outcome (green/red), step and measurement tallies, and a per-mode action: a + report link (online), the upload command (offline), or a disabled note. + """ + quiet = config.get_verbosity() < 0 + + if _is_disabled(config): + if not quiet: + terminalreporter.write_sep("=", "Sift", cyan=True, bold=True) + terminalreporter.write_line("Sift disabled — no test report created.") + return + + context = REPORT_CONTEXT + if context is None: + # No gated test ran, so no report context was created. Nothing to show. + return + + log_file = getattr(context, "log_file", None) + offline = _is_offline(config) + + # Resolve the report link first so stashing and --sift-open-report run even under + # -q (programmatic consumers don't care about verbosity). Truthiness, not + # ``is not None``: a resolved-but-empty id (degenerate sidecar mapping, unset + # proto field) must fall through to the "not uploaded" path, not produce a + # ``/test-results/`` link. + report_id = None if offline else _resolve_real_report_id(context) + report_url = ( + f"{context.client.app_url}/test-results/{report_id}" + if report_id and context.client.app_url + else None + ) + if report_id: + config.stash[SIFT_REPORT_ID_STASH_KEY] = report_id + if report_url is not None: + config.stash[SIFT_REPORT_URL_STASH_KEY] = report_url + if _option_or_ini(config, _OPEN): + _maybe_open_report(report_url) + + if quiet: + return + + failed = bool(getattr(context, "any_failures", False)) + status_word, status_markup = ( + ("FAILED", {"red": True, "bold": True}) + if failed + else ("PASSED", {"green": True, "bold": True}) + ) + # Offline results live only in the local log until replayed, so the status + # row calls that out instead of repeating the version (already in the header). + status_context = ( + f"{_mode_label(config)} · not uploaded" + if offline + else f"{_mode_label(config)} · sift-stack-py {_sdk_version()}" + ) + + report = context.report + + terminalreporter.write_sep( + "=", _report_panel_title(report, terminalreporter), cyan=True, bold=True + ) + + # Identity row: the test case (test path or pytest invocation). + if report.test_case: + _sift_kv(terminalreporter, "Test case", str(report.test_case)) + + # Status row: colored outcome, then compact mode context. + terminalreporter.write(" ") + terminalreporter.write(f"{'Status':<{_LABEL_WIDTH}}", bold=True) + terminalreporter.write(status_word, **status_markup) + terminalreporter.write_line(f" {status_context}") + + # Step + measurement tallies (green pass, red failure, yellow skip). + _write_count_row( + terminalreporter, + "Steps", + _step_count_segments(context.step_status_counts) or [("no steps", {})], + ) + measurement_segments = _measurement_segments(context.measurement_counts) + if measurement_segments: + _write_count_row(terminalreporter, "Measurements", measurement_segments) + + # Provenance row: test system and operator. + system = " · ".join( + part for part in (report.test_system_name, report.system_operator) if part + ) + if system: + _sift_kv(terminalreporter, "System", system) + + # Local log file (write-through backup online, sole sink offline). + if log_file is not None: + _sift_kv(terminalreporter, "Log file", str(log_file)) + + if offline: + if log_file is not None: + terminalreporter.write_sep("-", "to upload to Sift") + terminalreporter.write_line(f" >> import-test-result-log {log_file}", cyan=True) + return + + if not report_id: + # Incremental upload never mapped the report (the worker died before + # replaying the create), so there's no real report to link. + _sift_kv( + terminalreporter, + "Report", + f"not uploaded — replay with: import-test-result-log {log_file}", + yellow=True, + ) + elif report_url is not None: + _sift_kv(terminalreporter, "Report", report_url, cyan=True) + else: + _sift_kv( + terminalreporter, + "Report", + f"id {report_id} (set sift_report_url_base for a clickable link)", + ) + + if ( + report_id + and getattr(context, "replay_incomplete", False) + and log_file is not None + ): + _sift_kv( + terminalreporter, + "", + f"may be incomplete — finish with: import-test-result-log {log_file}", + yellow=True, + ) + + def _sift_enabled_for(node: pytest.Item | pytest.Collector, default: bool) -> bool: """Resolve the Sift gate for a node: sift_exclude > sift_include > default. @@ -806,6 +1147,10 @@ def sift_client(pytestconfig: pytest.Config) -> SiftClient: ) for env in missing: resolved[env] = _OFFLINE_DEFAULTS[env] + # Web-app origin for the report link: the sift_report_url_base CLI/ini option + # wins, then the SIFT_APP_URL env var, else host-based derivation in + # SiftClient.app_url. + report_url_base = _option_or_ini(pytestconfig, _REPORT_URL_BASE) or os.getenv("SIFT_APP_URL") # `or ""` is unreachable in practice since the `missing` check above guarantees # non-None values return SiftClient( @@ -813,6 +1158,7 @@ def sift_client(pytestconfig: pytest.Config) -> SiftClient: api_key=resolved.get("SIFT_API_KEY") or "", grpc_url=resolved.get("SIFT_GRPC_URI") or "", rest_url=resolved.get("SIFT_REST_URI") or "", + app_url=report_url_base or None, ) ) diff --git a/python/lib/sift_client/transport/base_connection.py b/python/lib/sift_client/transport/base_connection.py index 02f0e096e..6586412fe 100644 --- a/python/lib/sift_client/transport/base_connection.py +++ b/python/lib/sift_client/transport/base_connection.py @@ -24,6 +24,7 @@ def __init__( api_key: str, use_ssl: bool = True, cert_via_openssl: bool = False, + app_url: str | None = None, ): """Initialize the connection configuration. @@ -33,12 +34,17 @@ def __init__( api_key: The API key for authentication. use_ssl: Whether to use SSL/TLS for secure connections. cert_via_openssl: Whether to use OpenSSL for certificate validation. + app_url: The Sift web-app origin (e.g. ``https://app.siftstack.com``). + Set this for on-prem or custom deployments whose API host can't be + mapped to a frontend automatically. When unset, the web-app URL is + derived from ``rest_url`` for known hosts. """ self.api_key = api_key self.grpc_url = grpc_url self.rest_url = rest_url self.use_ssl = use_ssl self.cert_via_openssl = cert_via_openssl + self.app_url = app_url def get_grpc_config(self): """Create and return a GrpcConfig with the current settings. diff --git a/python/lib/sift_client/util/test_results/context_manager.py b/python/lib/sift_client/util/test_results/context_manager.py index 48a89b2d9..fa7e8a044 100644 --- a/python/lib/sift_client/util/test_results/context_manager.py +++ b/python/lib/sift_client/util/test_results/context_manager.py @@ -8,6 +8,7 @@ import tempfile import traceback import warnings +from collections import Counter from contextlib import AbstractContextManager, contextmanager from datetime import datetime, timezone from pathlib import Path @@ -19,6 +20,7 @@ from sift_client.sift_types.test_report import ( ErrorInfo, NumericBounds, + TestMeasurement, TestMeasurementCreate, TestReport, TestReportCreate, @@ -140,6 +142,19 @@ class ReportContext(AbstractContextManager): step_number_at_depth: dict[int, int] open_step_results: dict[str, bool] any_failures: bool + # Every step created in this report (including hierarchy/parametrize + # parents), retained after close so end-of-run summaries can tally final + # statuses. ``update`` mutates step instances in place, so these references + # reflect late status changes (e.g. a teardown-phase failure). + created_steps: list[TestStep] + # Every measurement recorded in this report, retained for end-of-run + # summaries. Appended in ``NewStep.measure``. A measurement's ``passed`` is + # fixed at creation, so the retained references stay accurate. + created_measurements: list[TestMeasurement] + # Set True in ``__exit__`` when the background replay worker timed out or + # exited non-zero, so callers (e.g. the pytest plugin footer) can flag that + # the uploaded report may be missing entries. + replay_incomplete: bool = False _import_proc: subprocess.Popen | None = None # Seconds to wait for the import worker subprocess to finish uploading # the JSONL backlog at session end before killing it. Tests substitute @@ -184,6 +199,9 @@ def __init__( self.step_number_at_depth = {} self.open_step_results = {} self.any_failures = False + self.created_steps = [] + self.created_measurements = [] + self.replay_incomplete = False if log_file is True: tmp = tempfile.NamedTemporaryFile(suffix=".jsonl", delete=False) @@ -279,6 +297,7 @@ def __exit__(self, exc_type, exc_value, traceback): except subprocess.TimeoutExpired: self._import_proc.kill() self._import_proc.wait() + self.replay_incomplete = True warnings.warn( f"Sift import worker did not exit in " f"{self._import_proc_timeout}s; killing it. " @@ -289,6 +308,7 @@ def __exit__(self, exc_type, exc_value, traceback): log_replay_instructions(self.log_file) return True # Ensures the session is marked as passed in pytest if self._import_proc.returncode != 0: + self.replay_incomplete = True stderr_text = ( stderr_bytes.decode("utf-8", errors="replace").strip() if stderr_bytes else "" ) @@ -311,6 +331,23 @@ def is_simulated(self) -> bool: """ return self.report.is_simulated + @property + def step_status_counts(self) -> Counter[TestStatus]: + """Tally of every created step by its current status. + + Includes hierarchy/parametrize parent steps. Read at the end of a run for + summaries; reflects late status changes since steps are mutated in place. + """ + return Counter(step.status for step in self.created_steps) + + @property + def measurement_counts(self) -> Counter[bool]: + """Tally of recorded measurements keyed by ``passed`` (True/False). + + Read at the end of a run for summaries. + """ + return Counter(m.passed for m in self.created_measurements) + def new_step( self, name: str, @@ -378,6 +415,8 @@ def create_step( ) self.step_stack.append(step) self.open_step_results[step.step_path] = True + # Retained for end-of-run tallies; never popped (unlike step_stack). + self.created_steps.append(step) return step @@ -388,6 +427,10 @@ def record_step_outcome(self, outcome: bool, step: TestStep): self.open_step_results[step.step_path] = False self.any_failures = True + def record_measurement(self, measurement: TestMeasurement) -> None: + """Retain a recorded measurement for end-of-run summaries.""" + self.created_measurements.append(measurement) + def mark_step_failed_after_close(self, step: TestStep): """Mark a step's parent as failed after the step has already been popped from the stack. @@ -662,6 +705,7 @@ def measure( create, log_file=self.report_context.log_file ) self.report_context.record_step_outcome(measurement.passed, self.current_step) + self.report_context.record_measurement(measurement) if not measurement.passed: self._failed_measurement_count += 1 From f11c738f84cbb1baeee840b6a3392f8a25936e23 Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Thu, 28 May 2026 09:49:35 -0700 Subject: [PATCH 2/4] clean up tests --- python/lib/sift_client/_tests/test_urls.py | 2 -- .../lib/sift_client/sift_types/test_report.py | 32 +++++++++++++++++++ .../util/test_results/context_manager.py | 16 +++++++--- 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/python/lib/sift_client/_tests/test_urls.py b/python/lib/sift_client/_tests/test_urls.py index d3ac5dc75..be9febd52 100644 --- a/python/lib/sift_client/_tests/test_urls.py +++ b/python/lib/sift_client/_tests/test_urls.py @@ -14,8 +14,6 @@ class TestFrontendOriginForApi: [ ("https://api.siftstack.com", "https://app.siftstack.com"), ("https://gov.api.siftstack.com", "https://gov.siftstack.com"), - ("http://localhost:8080", "http://localhost:3000"), - ("http://host.docker.internal:8080", "http://localhost:3000"), # Bare host (no scheme) resolves the same as the full URL. ("api.siftstack.com", "https://app.siftstack.com"), ], diff --git a/python/lib/sift_client/sift_types/test_report.py b/python/lib/sift_client/sift_types/test_report.py index c4abfc548..dd786b02d 100644 --- a/python/lib/sift_client/sift_types/test_report.py +++ b/python/lib/sift_client/sift_types/test_report.py @@ -410,6 +410,38 @@ class TestMeasurement(BaseType[TestMeasurementProto, "TestMeasurement"], Simulat # Set by the low-level wrapper when this instance came from the simulate path _simulated: bool = False + def __str__(self) -> str: + """Human-readable form: ``[STATUS] name = value [unit] (bounds)``. + + Used for failure messages, logs, and the REPL. The string omits whichever + parts aren't set (no unit, no bounds), and falls back to ``?`` if no + value type is populated. The status prefix reflects ``self.passed``. + """ + status = "PASSED" if self.passed else "FAILED" + if self.numeric_value is not None: + value = f"{self.numeric_value}" + if self.unit: + value += f" {self.unit}" + elif self.string_value is not None: + value = repr(self.string_value) + elif self.boolean_value is not None: + value = str(self.boolean_value).lower() + else: + value = "?" + bounds = "" + nb = self.numeric_bounds + if nb is not None: + parts: list[str] = [] + if nb.min is not None: + parts.append(f"min {nb.min}") + if nb.max is not None: + parts.append(f"max {nb.max}") + if parts: + bounds = f" ({', '.join(parts)})" + elif self.string_expected_value: + bounds = f" (expected {self.string_expected_value!r})" + return f"[{status}] {self.name} = {value}{bounds}" + @classmethod def _from_proto( cls, proto: TestMeasurementProto, sift_client: SiftClient | None = None diff --git a/python/lib/sift_client/util/test_results/context_manager.py b/python/lib/sift_client/util/test_results/context_manager.py index fa7e8a044..41066b247 100644 --- a/python/lib/sift_client/util/test_results/context_manager.py +++ b/python/lib/sift_client/util/test_results/context_manager.py @@ -509,6 +509,9 @@ def __init__( # substep / ``report_outcome`` failures are intentionally not folded # in here (see ``measurements_passed`` vs ``passed``). self._failed_measurement_count = 0 + # Out-of-bounds measurements recorded on this step, retained so + # ``fail_if_measurements_failed`` can name them in the failure message. + self._failed_measurements: list[TestMeasurement] = [] def __enter__(self): """Enter the context manager to create a new step. @@ -530,9 +533,7 @@ def measurements_passed(self) -> bool: """ return self._failed_measurement_count == 0 - def fail_if_measurements_failed( - self, message: str = "one or more measurements out of bounds" - ) -> None: + def fail_if_measurements_failed(self, message: str = "measurements out of bounds") -> None: """Fail the pytest test if any measurement on this step was out of bounds. Use instead of ``assert step.measurements_passed``: it fails via @@ -540,12 +541,18 @@ def fail_if_measurements_failed( assertion message to ``error_info``. No-op when every measurement passed. Call once at the end of the test so every measurement is still recorded before the failure fires. + + The failure message names each out-of-bounds measurement with its + recorded value and bounds. ``message`` is used as the header line. """ if self.measurements_passed: return import pytest - pytest.fail(message, pytrace=False) + failed = self._failed_measurements + header = f"{message} ({len(failed)}):" if failed else message + body = [f" - {m}" for m in failed] + pytest.fail("\n".join([header, *body]), pytrace=False) def update_step_from_result( self, @@ -708,6 +715,7 @@ def measure( self.report_context.record_measurement(measurement) if not measurement.passed: self._failed_measurement_count += 1 + self._failed_measurements.append(measurement) return measurement.passed From 81f964b80e77d3af4bd22a917e8b0ebc9891772c Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Thu, 28 May 2026 09:50:56 -0700 Subject: [PATCH 3/4] lint --- .../_tests/pytest_plugin/test_terminal_output.py | 4 +--- python/lib/sift_client/pytest_plugin.py | 10 ++-------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/python/lib/sift_client/_tests/pytest_plugin/test_terminal_output.py b/python/lib/sift_client/_tests/pytest_plugin/test_terminal_output.py index 4985fd9fb..76550cc22 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/test_terminal_output.py +++ b/python/lib/sift_client/_tests/pytest_plugin/test_terminal_output.py @@ -28,9 +28,7 @@ class TestStepCountSegments: def test_lists_nonzero_statuses_in_order_with_color(self) -> None: - counts = Counter( - {TestStatus.PASSED: 4, TestStatus.FAILED: 2, TestStatus.SKIPPED: 1} - ) + counts = Counter({TestStatus.PASSED: 4, TestStatus.FAILED: 2, TestStatus.SKIPPED: 1}) assert _step_count_segments(counts) == [ ("4 passed", {"green": True}), ("2 failed", {"red": True}), diff --git a/python/lib/sift_client/pytest_plugin.py b/python/lib/sift_client/pytest_plugin.py index 0ede078ed..cf85b3abb 100644 --- a/python/lib/sift_client/pytest_plugin.py +++ b/python/lib/sift_client/pytest_plugin.py @@ -739,9 +739,7 @@ def pytest_terminal_summary(terminalreporter: Any, exitstatus: int, config: pyte _write_count_row(terminalreporter, "Measurements", measurement_segments) # Provenance row: test system and operator. - system = " · ".join( - part for part in (report.test_system_name, report.system_operator) if part - ) + system = " · ".join(part for part in (report.test_system_name, report.system_operator) if part) if system: _sift_kv(terminalreporter, "System", system) @@ -773,11 +771,7 @@ def pytest_terminal_summary(terminalreporter: Any, exitstatus: int, config: pyte f"id {report_id} (set sift_report_url_base for a clickable link)", ) - if ( - report_id - and getattr(context, "replay_incomplete", False) - and log_file is not None - ): + if report_id and getattr(context, "replay_incomplete", False) and log_file is not None: _sift_kv( terminalreporter, "", From 198ae2d57a26f4bebd3c4015690652aeb5fbea56 Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Thu, 28 May 2026 14:52:15 -0700 Subject: [PATCH 4/4] reuse url util --- .../_internal/grpc_transport/transport.py | 25 ++++--------------- python/lib/sift_client/_internal/rest.py | 4 +-- python/lib/sift_client/_internal/urls.py | 8 +++--- 3 files changed, 11 insertions(+), 26 deletions(-) diff --git a/python/lib/sift_client/_internal/grpc_transport/transport.py b/python/lib/sift_client/_internal/grpc_transport/transport.py index 7e0bc5425..e088befa0 100644 --- a/python/lib/sift_client/_internal/grpc_transport/transport.py +++ b/python/lib/sift_client/_internal/grpc_transport/transport.py @@ -8,7 +8,6 @@ from importlib.metadata import PackageNotFoundError, version from typing import TYPE_CHECKING, Any, TypedDict, cast -from urllib.parse import ParseResult, urlparse import grpc import grpc.aio as grpc_aio @@ -21,6 +20,7 @@ Metadata, MetadataInterceptor, ) +from sift_client._internal.urls import parse_host if TYPE_CHECKING: from sift_client._internal.grpc_transport._async_interceptors.base import ClientAsyncInterceptor @@ -78,7 +78,7 @@ def use_sift_channel( credentials = get_ssl_credentials(cert_via_openssl) options = _compute_channel_options(config) - api_uri = _clean_uri(config["uri"], use_ssl) + api_uri = parse_host(config["uri"]) channel = grpc.secure_channel(api_uri, credentials, options) interceptors = _compute_sift_interceptors(config, metadata) return grpc.intercept_channel(channel, *interceptors) @@ -98,7 +98,7 @@ def use_sift_async_channel( return _use_insecure_sift_async_channel(config, metadata) return grpc_aio.secure_channel( - target=_clean_uri(config["uri"], use_ssl), + target=parse_host(config["uri"]), credentials=get_ssl_credentials(cert_via_openssl), options=_compute_channel_options(config), interceptors=_compute_sift_async_interceptors(config, metadata), @@ -112,7 +112,7 @@ def _use_insecure_sift_channel( FOR DEVELOPMENT PURPOSES ONLY """ options = _compute_channel_options(config) - api_uri = _clean_uri(config["uri"], False) + api_uri = parse_host(config["uri"]) channel = grpc.insecure_channel(api_uri, options) interceptors = _compute_sift_interceptors(config, metadata) return grpc.intercept_channel(channel, *interceptors) @@ -125,7 +125,7 @@ def _use_insecure_sift_async_channel( FOR DEVELOPMENT PURPOSES ONLY """ return grpc_aio.insecure_channel( - target=_clean_uri(config["uri"], False), + target=parse_host(config["uri"]), options=_compute_channel_options(config), interceptors=_compute_sift_async_interceptors(config, metadata), ) @@ -205,21 +205,6 @@ def _metadata_async_interceptor( return MetadataAsyncInterceptor(md) -def _clean_uri(uri: str, use_ssl: bool) -> str: - """ - This will automatically transform the URI to an acceptable form regardless of whether or not - users included the scheme in the URL or included trailing slashes. - """ - - if "http://" in uri or "https://" in uri: - parsed: ParseResult = urlparse(uri) - return parsed.netloc - - full_uri = f"https://{uri}" if use_ssl else f"http://{uri}" - parsed_res: ParseResult = urlparse(full_uri) - return parsed_res.netloc - - def _compute_user_agent() -> str: try: return f"sift_stack_py/{version('sift_stack_py')}" diff --git a/python/lib/sift_client/_internal/rest.py b/python/lib/sift_client/_internal/rest.py index ee0239b79..6a9d1c9d1 100644 --- a/python/lib/sift_client/_internal/rest.py +++ b/python/lib/sift_client/_internal/rest.py @@ -6,7 +6,7 @@ from typing_extensions import NotRequired from urllib3.util import Retry -from sift_client._internal.grpc_transport.transport import _clean_uri +from sift_client._internal.urls import parse_host _DEFAULT_REST_RETRY = Retry(total=3, status_forcelist=[500, 502, 503, 504], backoff_factor=1) @@ -33,7 +33,7 @@ class SiftRestConfig(TypedDict): def compute_uri(restconf: SiftRestConfig) -> str: uri = restconf["uri"] use_ssl = restconf.get("use_ssl", True) - clean_uri = _clean_uri(uri, use_ssl) + clean_uri = parse_host(uri) if use_ssl: return f"https://{clean_uri}" diff --git a/python/lib/sift_client/_internal/urls.py b/python/lib/sift_client/_internal/urls.py index a87f3b53c..99dd1816f 100644 --- a/python/lib/sift_client/_internal/urls.py +++ b/python/lib/sift_client/_internal/urls.py @@ -19,7 +19,7 @@ } -def _origin(url: str) -> str: +def parse_origin(url: str) -> str: """Normalize a URL or bare host into a ``scheme://host[:port]`` origin. Bare hosts (no scheme) are assumed to be ``https``. @@ -29,7 +29,7 @@ def _origin(url: str) -> str: return f"{parsed.scheme}://{parsed.netloc}".rstrip("/") -def _host(url: str) -> str: +def parse_host(url: str) -> str: """Extract ``host[:port]`` from a URL or bare host string.""" candidate = url if "://" in url else f"https://{url}" return urlparse(candidate).netloc @@ -49,7 +49,7 @@ def frontend_origin_for_api(api_base_url: str, override: str | None = None) -> s when no override is given and the API host isn't recognized. """ if override: - return _origin(override) + return parse_origin(override) if not api_base_url: return None - return _API_HOST_TO_FRONTEND_ORIGIN.get(_host(api_base_url)) + return _API_HOST_TO_FRONTEND_ORIGIN.get(parse_host(api_base_url))