From 04fa43c07905bd8f95c34d26d3070855bc33d628 Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Thu, 14 May 2026 15:43:04 -0700 Subject: [PATCH 01/14] enable registering plugin --- python/docs/examples/pytest_plugin.md | 56 +++++++----- python/lib/sift_client/_tests/conftest.py | 4 - .../lib/sift_client/_tests/util/conftest.py | 8 -- .../pytest_util.py => pytest_plugin.py} | 28 +++++- .../sift_client/util/test_results/__init__.py | 85 ++++++------------- 5 files changed, 85 insertions(+), 96 deletions(-) rename python/lib/sift_client/{util/test_results/pytest_util.py => pytest_plugin.py} (89%) diff --git a/python/docs/examples/pytest_plugin.md b/python/docs/examples/pytest_plugin.md index cf56dd75e..ac50d0003 100644 --- a/python/docs/examples/pytest_plugin.md +++ b/python/docs/examples/pytest_plugin.md @@ -9,11 +9,13 @@ This page walks through wiring the plugin into a project, the fixtures and hooks it provides, and the patterns you'll use day-to-day. !!! info "Where the plugin lives" - The plugin is part of `sift_client.util.test_results`. It is **not** - registered as a `pytest11` entry point. Projects opt in with a - `from sift_client.util.test_results import *` in their `conftest.py`. - That import is what wires up the fixtures, the CLI options, and the - `pytest_runtest_makereport` hook. + The plugin lives at `sift_client.pytest_plugin`. It is + **not** registered as a `pytest11` entry point. Projects opt in with a + `pytest_plugins` declaration in their top-level `conftest.py`. Pytest + then loads the module as a real plugin: the fixtures, CLI options, and + `pytest_runtest_makereport` hook all register through standard pytest + machinery, so `pytest --trace-config` lists it and + `pytest -p no:sift_client.pytest_plugin` disables it. ## Install @@ -33,9 +35,26 @@ The `SIFT_GRPC_URI` and `SIFT_REST_URI` are the gRPC and REST endpoints for your ## Wire the plugin into `conftest.py` -Two things are required: a session-scoped `sift_client` fixture (the plugin's -`report_context` fixture resolves it by name), and a star-import that registers -the plugin's fixtures into the conftest's namespace. +A single `pytest_plugins` declaration in your top-level `conftest.py` is all +that's required. The plugin ships a default `sift_client` fixture that reads +`SIFT_API_KEY`, `SIFT_GRPC_URI`, and `SIFT_REST_URI` from the environment. + +```python title="conftest.py" +from dotenv import load_dotenv + +load_dotenv() + +pytest_plugins = ["sift_client.pytest_plugin"] +``` + +That's the whole setup. Every test in the session will now create a step on a +single shared `TestReport`. + +### Customizing the `SiftClient` + +To construct the client differently (custom TLS, timeouts, alternate +credentials, etc.), override the `sift_client` fixture in your conftest. The +plugin's default falls away in favor of your definition. ```python title="conftest.py" import os @@ -45,30 +64,23 @@ from dotenv import load_dotenv from sift_client import SiftClient, SiftConnectionConfig -# Star-import wires fixtures + hooks + CLI options into pytest collection. -from sift_client.util.test_results import * - load_dotenv() +pytest_plugins = ["sift_client.pytest_plugin"] + @pytest.fixture(scope="session") def sift_client() -> SiftClient: - grpc_url = os.getenv("SIFT_GRPC_URI") - rest_url = os.getenv("SIFT_REST_URI") - api_key = os.getenv("SIFT_API_KEY") - return SiftClient( connection_config=SiftConnectionConfig( - api_key=api_key, - grpc_url=grpc_url, - rest_url=rest_url, + api_key=os.getenv("SIFT_API_KEY"), + grpc_url=os.getenv("SIFT_GRPC_URI"), + rest_url=os.getenv("SIFT_REST_URI"), + use_ssl=False, ) ) ``` -That's the whole setup. Every test in the session will now create a step on a -single shared `TestReport`. - ## Plugin provided fixtures | Name | Kind | Scope | Purpose | @@ -585,7 +597,7 @@ automatic skip. ```python title="conftest.py" import pytest -from sift_client.util.test_results import * +pytest_plugins = ["sift_client.pytest_plugin"] @pytest.fixture(autouse=True) diff --git a/python/lib/sift_client/_tests/conftest.py b/python/lib/sift_client/_tests/conftest.py index 5683182e5..79b079d39 100644 --- a/python/lib/sift_client/_tests/conftest.py +++ b/python/lib/sift_client/_tests/conftest.py @@ -78,10 +78,6 @@ def ci_pytest_tag(sift_client): return tag -# Import the Sift test results fixtures the way we recommend to users. -from sift_client.util.test_results import * # noqa: F403 - - def pytest_configure(config: pytest.Config) -> None: """Enable the Sift connection-check mode for the fixtures used in this test suite since we run w/ mock client in non-integration tests.""" config.option.sift_test_results_check_connection = True diff --git a/python/lib/sift_client/_tests/util/conftest.py b/python/lib/sift_client/_tests/util/conftest.py index 45279cca6..086d2f690 100644 --- a/python/lib/sift_client/_tests/util/conftest.py +++ b/python/lib/sift_client/_tests/util/conftest.py @@ -1,14 +1,6 @@ import pytest -def pytest_addoption(parser: pytest.Parser) -> None: - existing_options = [opt.names() for opt in parser._anonymous.options] - # Flatten the list of lists into a single list of strings - flat_options = [item for sublist in existing_options for item in sublist] - if not any("--sift-test-results-log-file" in name for name in flat_options): - parser.addoption("--sift-test-results-log-file", action="store_true", default=False) - - def pytest_configure(config: pytest.Config) -> None: """Configure the pytest configuration to disable the Sift test results log file.""" config.option.sift_test_results_log_file = False diff --git a/python/lib/sift_client/util/test_results/pytest_util.py b/python/lib/sift_client/pytest_plugin.py similarity index 89% rename from python/lib/sift_client/util/test_results/pytest_util.py rename to python/lib/sift_client/pytest_plugin.py index a96a47fb3..15ebf4193 100644 --- a/python/lib/sift_client/util/test_results/pytest_util.py +++ b/python/lib/sift_client/pytest_plugin.py @@ -1,16 +1,17 @@ from __future__ import annotations +import os from datetime import datetime, timezone from pathlib import Path from typing import TYPE_CHECKING, Any, Generator import pytest +from sift_client import SiftClient, SiftConnectionConfig from sift_client.sift_types.test_report import TestStatus from sift_client.util.test_results import ReportContext if TYPE_CHECKING: - from sift_client.client import SiftClient from sift_client.util.test_results.context_manager import NewStep REPORT_CONTEXT: ReportContext | None = None @@ -18,7 +19,8 @@ def pytest_addoption(parser: pytest.Parser) -> None: """Register Sift-specific command-line options.""" - parser.addoption( + group = parser.getgroup("sift", description="Sift test results") + group.addoption( "--sift-test-results-log-file", default=None, help="Path to write the Sift test result log file. " @@ -26,7 +28,7 @@ def pytest_addoption(parser: pytest.Parser) -> None: "False, 'false', or 'none' to disable logging, " "or a file path to write to a specific location.", ) - parser.addoption( + group.addoption( "--no-sift-test-results-git-metadata", action="store_false", dest="sift_test_results_git_metadata", @@ -34,7 +36,7 @@ def pytest_addoption(parser: pytest.Parser) -> None: help="Exclude git metadata from the Sift test results. " "Git metadata (repo, branch, commit) is included by default.", ) - parser.addoption( + group.addoption( "--sift-test-results-check-connection", action="store_true", default=False, @@ -116,6 +118,24 @@ def _has_sift_connection(request: pytest.FixtureRequest) -> bool: return bool(request.getfixturevalue("client_has_connection")) +@pytest.fixture(scope="session") +def sift_client() -> SiftClient: + """Default ``SiftClient`` resolved from environment variables. + + Reads ``SIFT_API_KEY``, ``SIFT_GRPC_URI``, and ``SIFT_REST_URI``. Projects + that need custom construction (TLS toggles, custom timeouts, etc.) can + override this fixture by defining their own ``sift_client`` in their + ``conftest.py``; pytest fixture resolution prefers the local definition. + """ + return SiftClient( + connection_config=SiftConnectionConfig( + api_key=os.getenv("SIFT_API_KEY"), + grpc_url=os.getenv("SIFT_GRPC_URI"), + rest_url=os.getenv("SIFT_REST_URI"), + ) + ) + + @pytest.fixture(scope="session", autouse=True) def report_context( sift_client: SiftClient, request: pytest.FixtureRequest, pytestconfig: pytest.Config diff --git a/python/lib/sift_client/util/test_results/__init__.py b/python/lib/sift_client/util/test_results/__init__.py index e7a82866c..93a25169a 100644 --- a/python/lib/sift_client/util/test_results/__init__.py +++ b/python/lib/sift_client/util/test_results/__init__.py @@ -49,78 +49,47 @@ def main(self): cleanup() ``` -## Pytest Fixtures +## Pytest Plugin -The report context and steps can also be accessed in pytest by importing the `report_context` and `step` fixtures. - -### How to use: -- These fixtures are set to autouse and will automatically create a report and steps for each test function. - - If you want each module(file) to be marked as a step w/ each test as a substep, import the `module_substep` fixture as well. -- The `report_context` fixture requires a fixture `sift_client` returning an `SiftClient` instance to be passed in. - -Note: FedRAMP users: report_context will log test results to a temp file to avoid API calls during test execution. If this is a shared environment, you can disable logging by passing ``--sift-test-results-log-file=false``. - -#### Configuration - -Import the `pytest_addoption` function to add configuration options for Test Results to the commandline or add the options to your pyproject.toml file (https://docs.pytest.org/en/stable/reference/customize.html#configuration). If ommitted, will use the default values described below. - -- Git metadata: Include git metadata (repo, branch, commit) in the test results. Default is True. You can disable it by passing `--no-sift-test-results-git-metadata`. -- Log file: Write test results to a file. This happens automatically but you can configure specify a specific log file by passing `--sift-test-results-log-file=` or disable logging by passing `--sift-test-results-log-file=false`. -- Check connection: Pass `--sift-test-results-check-connection` (off by default) to make the `report_context`, `step`, and `module_substep` fixtures no-op when the Sift client has no connection to the server. Requires a `client_has_connection` fixture to be available. - -###### Example at top of your test file or in your conftest.py file: +The pytest plugin lives at `sift_client.pytest_plugin`. Opt in +from your `conftest.py`: ```python -import pytest +# conftest.py +pytest_plugins = ["sift_client.pytest_plugin"] +``` -@pytest.fixture(scope="session") -def sift_client() -> SiftClient: - grpc_url = os.getenv("SIFT_GRPC_URI", "localhost:50051") - rest_url = os.getenv("SIFT_REST_URI", "localhost:8080") - api_key = os.getenv("SIFT_API_KEY", "") +The plugin ships an autouse session-scoped `report_context` fixture (one +`TestReport` per session), an autouse function-scoped `step` fixture, and an +optional `module_substep` fixture. It also registers a default `sift_client` +fixture that reads `SIFT_API_KEY`, `SIFT_GRPC_URI`, and `SIFT_REST_URI` from +the environment. Override it by defining your own `sift_client` fixture in +your conftest. - client = SiftClient(api_key=api_key, grpc_url=grpc_url, rest_url=rest_url) +Note: FedRAMP users: `report_context` will log test results to a temp file to +avoid API calls during test execution. If this is a shared environment, you +can disable logging by passing `--sift-test-results-log-file=false`. - return client +#### Configuration -from sift_client.util.test_results import * -``` +CLI options registered by the plugin: -###### Then in your test file: +- `--sift-test-results-log-file`: Path to write the JSONL log file. `true` + (default) auto-creates a temp file; `false`/`none` disables logging; a path + writes to that location. +- `--no-sift-test-results-git-metadata`: Exclude git metadata (repo, branch, + commit) from the test report. Included by default. +- `--sift-test-results-check-connection`: Make `report_context`, `step`, and + `module_substep` no-op when the client has no connection. Requires a + `client_has_connection` fixture (the plugin ships a default). -```python -# Because step was already imported and set autouse=True, this test will automatically get a step created for it. -def test_no_includes(): - assert condition, "Example failure" - -# Passing the fixtures to the test function allows you to take measurements or create substeps. -def test_example(report_context, step): - # This will add a measurement to the current step for this function - step.measure(name="Example Measurement", value=test_string_value, bounds="expected_string_value") - - with report_context.new_step(name="Example Step") as substep: - example_measurement = tlm.read(channel_name) - substep.measure(name="Substep Measurement", value=example_measurement, bounds=(min=74.9, max=75.1)) -``` +To disable the plugin for a single run: +`pytest -p no:sift_client.pytest_plugin`. """ from .context_manager import NewStep, ReportContext -from .pytest_util import ( - client_has_connection, - module_substep, - pytest_addoption, - pytest_runtest_makereport, - report_context, - step, -) __all__ = [ "NewStep", "ReportContext", - "client_has_connection", - "module_substep", - "pytest_addoption", - "pytest_runtest_makereport", - "report_context", - "step", ] From 8820e7fa6043aea3439725fdc64a25d8172f7274 Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Thu, 14 May 2026 15:56:31 -0700 Subject: [PATCH 02/14] fix githook for worktree case --- .githooks/pre-push-python/extras.sh | 3 +++ .githooks/pre-push-python/fmt-lint.sh | 3 +++ .githooks/pre-push-python/stubs.sh | 3 +++ 3 files changed, 9 insertions(+) diff --git a/.githooks/pre-push-python/extras.sh b/.githooks/pre-push-python/extras.sh index 56cb00620..515c5614e 100755 --- a/.githooks/pre-push-python/extras.sh +++ b/.githooks/pre-push-python/extras.sh @@ -1,5 +1,8 @@ # ensure generated pyproject.toml extras are up-to-date +# Clear git env vars set by the parent hook so git commands resolve the work tree normally +unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_PREFIX + # Store the root directory of the repository REPO_ROOT="$(git rev-parse --show-toplevel)" PYTHON_DIR="$REPO_ROOT/python" diff --git a/.githooks/pre-push-python/fmt-lint.sh b/.githooks/pre-push-python/fmt-lint.sh index f112c7f09..f5261fbdb 100644 --- a/.githooks/pre-push-python/fmt-lint.sh +++ b/.githooks/pre-push-python/fmt-lint.sh @@ -2,6 +2,9 @@ set -e +# Clear git env vars set by the parent hook so git commands resolve the work tree normally +unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_PREFIX + # Store the root directory of the repository REPO_ROOT="$(git rev-parse --show-toplevel)" PYTHON_DIR="$REPO_ROOT/python" diff --git a/.githooks/pre-push-python/stubs.sh b/.githooks/pre-push-python/stubs.sh index 9ee86fb42..8e72fd426 100644 --- a/.githooks/pre-push-python/stubs.sh +++ b/.githooks/pre-push-python/stubs.sh @@ -1,5 +1,8 @@ # ensure generated python stubs are up-to-date, from sync clients +# Clear git env vars set by the parent hook so git commands resolve the work tree normally +unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_PREFIX + # Store the root directory of the repository REPO_ROOT="$(git rev-parse --show-toplevel)" PYTHON_DIR="$REPO_ROOT/python" From 848d8284b808f3636961b29267cbc013714cfdba Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Fri, 15 May 2026 13:06:55 -0700 Subject: [PATCH 03/14] fix mypy --- python/lib/sift_client/pytest_plugin.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/python/lib/sift_client/pytest_plugin.py b/python/lib/sift_client/pytest_plugin.py index 15ebf4193..701ee3e47 100644 --- a/python/lib/sift_client/pytest_plugin.py +++ b/python/lib/sift_client/pytest_plugin.py @@ -118,6 +118,16 @@ def _has_sift_connection(request: pytest.FixtureRequest) -> bool: return bool(request.getfixturevalue("client_has_connection")) +def _required_env(name: str) -> str: + value = os.getenv(name) + if not value: + raise pytest.UsageError( + f"{name} must be set to use the default sift_client fixture; " + f"set the environment variable or override the sift_client fixture in conftest.py." + ) + return value + + @pytest.fixture(scope="session") def sift_client() -> SiftClient: """Default ``SiftClient`` resolved from environment variables. @@ -129,9 +139,9 @@ def sift_client() -> SiftClient: """ return SiftClient( connection_config=SiftConnectionConfig( - api_key=os.getenv("SIFT_API_KEY"), - grpc_url=os.getenv("SIFT_GRPC_URI"), - rest_url=os.getenv("SIFT_REST_URI"), + api_key=_required_env("SIFT_API_KEY"), + grpc_url=_required_env("SIFT_GRPC_URI"), + rest_url=_required_env("SIFT_REST_URI"), ) ) From 4c21cbcee5481de204e88b3fbadf608d8db8a71e Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Sun, 17 May 2026 14:43:39 -0700 Subject: [PATCH 04/14] clean up --- python/docs/examples/pytest_plugin.md | 72 ++++++- python/lib/sift_client/pytest_plugin.py | 193 ++++++++++++------ .../sift_client/util/test_results/__init__.py | 17 ++ 3 files changed, 218 insertions(+), 64 deletions(-) diff --git a/python/docs/examples/pytest_plugin.md b/python/docs/examples/pytest_plugin.md index ac50d0003..a3bbc7573 100644 --- a/python/docs/examples/pytest_plugin.md +++ b/python/docs/examples/pytest_plugin.md @@ -98,17 +98,81 @@ def sift_client() -> SiftClient: | `--no-sift-test-results-git-metadata` | git metadata on | Skip capturing git repo/branch/commit on the report's metadata. | | `--sift-test-results-check-connection` | off | Make `report_context`, `step`, and `module_substep` no-op (yield `None`) when `client_has_connection` is `False`. Lets the same suite run locally without a Sift backend. | -These can be set permanently in `pytest.ini`: +These can be passed permanently via `addopts`: ```ini title="pytest.ini" [pytest] addopts = --sift-test-results-check-connection ``` +Or set the matching ini key directly (recommended for stable per-project +configuration). Each CLI flag has a corresponding key under +`[tool.pytest.ini_options]` in `pyproject.toml` or `[pytest]` in `pytest.ini`. +CLI flags, when passed, override the ini values. + +| Ini key | Type | Equivalent CLI flag | +|---|---|---| +| `sift_test_results_log_file` | string (`true` / `false` / `none` / path) | `--sift-test-results-log-file=` | +| `sift_test_results_git_metadata` | bool (default `true`) | `--no-sift-test-results-git-metadata` (sets to `false`) | +| `sift_test_results_check_connection` | bool (default `false`) | `--sift-test-results-check-connection` | + +The default `sift_client` fixture reads its two URIs from environment first +and falls back to ini keys when the env vars are unset. `SIFT_API_KEY` is +intentionally env-only — keep it out of source control and supply it through +`pytest-dotenv` (see [API key handling](#api-key-handling) below). The env +var wins when both are set, so secrets injected into a CI environment +continue to override values committed to `pyproject.toml`. There are no CLI +flags for credentials. + +| Ini key | Environment variable | Notes | +|---|---|---| +| _(none)_ | `SIFT_API_KEY` | Env-only. Use `.env` + `pytest-dotenv` locally; inject from your secret store in CI. | +| `sift_grpc_uri` | `SIFT_GRPC_URI` | Stable per-org gRPC endpoint; safe to commit. | +| `sift_rest_uri` | `SIFT_REST_URI` | Stable per-org REST endpoint; safe to commit. | + +```toml title="pyproject.toml" +[tool.pytest.ini_options] +sift_test_results_check_connection = true +sift_test_results_log_file = "false" +sift_test_results_git_metadata = false +sift_grpc_uri = "your-org.sift.example:443" +sift_rest_uri = "https://your-org.sift.example" +``` + +```ini title="pytest.ini" +[pytest] +sift_test_results_check_connection = true +sift_test_results_log_file = false +sift_test_results_git_metadata = false +sift_grpc_uri = your-org.sift.example:443 +sift_rest_uri = https://your-org.sift.example +``` + +#### API key handling + +`SIFT_API_KEY` is deliberately read from the process environment only. The +recommended workflow uses the +[`pytest-dotenv`](https://pypi.org/project/pytest-dotenv/) plugin (already a +dependency of `sift-stack-py`), which loads variables from a `.env` file +into `os.environ` before tests run. + +1. Add `.env` to `.gitignore`. +2. Drop your key into `.env` at the project root: + + ```bash title=".env" + SIFT_API_KEY=sk-...your-key... + ``` + +3. In CI, set `SIFT_API_KEY` directly via your provider's secret manager + instead of committing a `.env` file. + +`pytest-dotenv` picks the file up automatically; no `pytest_configure` +glue is needed. + !!! warning "FedRAMP / shared environments" - Pass `--sift-test-results-log-file=false` to skip the temp file + worker - pipeline. Create/update calls then run inline against the API instead of - being deferred through a subprocess. + Pass `--sift-test-results-log-file=false` (or set the ini key to `"false"`) + to skip the temp file + worker pipeline. Create/update calls then run + inline against the API instead of being deferred through a subprocess. ### Report metadata captured automatically diff --git a/python/lib/sift_client/pytest_plugin.py b/python/lib/sift_client/pytest_plugin.py index 701ee3e47..69cd02b88 100644 --- a/python/lib/sift_client/pytest_plugin.py +++ b/python/lib/sift_client/pytest_plugin.py @@ -16,9 +16,21 @@ REPORT_CONTEXT: ReportContext | None = None +# Ini keys that mirror the registered CLI options. Centralized so renames stay in sync. +_INI_LOG_FILE = "sift_test_results_log_file" +_INI_GIT_METADATA = "sift_test_results_git_metadata" +_INI_CHECK_CONNECTION = "sift_test_results_check_connection" +_INI_GRPC_URI = "sift_grpc_uri" +_INI_REST_URI = "sift_rest_uri" + def pytest_addoption(parser: pytest.Parser) -> None: - """Register Sift-specific command-line options.""" + """Register Sift-specific command-line options and ini keys. + + Each option can be set on the command line or under ``[tool.pytest.ini_options]`` + in ``pyproject.toml`` (or ``[pytest]`` in ``pytest.ini``). CLI values take + precedence over ini values, which take precedence over the built-in default. + """ group = parser.getgroup("sift", description="Sift test results") group.addoption( "--sift-test-results-log-file", @@ -28,30 +40,83 @@ def pytest_addoption(parser: pytest.Parser) -> None: "False, 'false', or 'none' to disable logging, " "or a file path to write to a specific location.", ) + parser.addini( + _INI_LOG_FILE, + help="Default value for --sift-test-results-log-file. Same values " + "accepted as the CLI flag (path, 'true', 'false', 'none').", + default=None, + ) group.addoption( "--no-sift-test-results-git-metadata", action="store_false", dest="sift_test_results_git_metadata", - default=True, + default=None, help="Exclude git metadata from the Sift test results. " "Git metadata (repo, branch, commit) is included by default.", ) + parser.addini( + _INI_GIT_METADATA, + help="Include git repo/branch/commit in the report (true/false). " + "Defaults to true. The --no-sift-test-results-git-metadata CLI flag " + "overrides this when passed.", + type="bool", + default=True, + ) group.addoption( "--sift-test-results-check-connection", action="store_true", - default=False, + default=None, help="Skip the sift test-result fixtures (report_context, step, module_substep) " "when the Sift client has no connection to the server. Requires a " "`client_has_connection` fixture to be available in the test session.", ) + parser.addini( + _INI_CHECK_CONNECTION, + help="When true, skip the sift test-result fixtures if the client has " + "no connection (same effect as --sift-test-results-check-connection). " + "Defaults to false.", + type="bool", + default=False, + ) + parser.addini( + _INI_GRPC_URI, + help="Sift gRPC endpoint URI. The default `sift_client` fixture " + "prefers the SIFT_GRPC_URI environment variable and falls back to " + "this ini value.", + default=None, + ) + parser.addini( + _INI_REST_URI, + help="Sift REST endpoint URI. The default `sift_client` fixture " + "prefers the SIFT_REST_URI environment variable and falls back to " + "this ini value.", + default=None, + ) + + +def _option_or_ini(pytestconfig: pytest.Config | None, name: str) -> Any: + """Resolve a Sift plugin setting from CLI > ini > None. + + The ``addoption`` registrations use ``default=None`` so we can tell whether + the CLI was actually used. When the CLI didn't set a value, fall back to + the matching ``addini`` key. + """ + if pytestconfig is None: + return None + cli = pytestconfig.getoption(name, default=None) + if cli is not None: + return cli + try: + return pytestconfig.getini(name) + except (KeyError, ValueError): + return None def _resolve_log_file(pytestconfig: pytest.Config | None) -> str | Path | bool | None: - """Determine log_file value from --sift-test-results-log-file option.""" - raw = None - if pytestconfig is not None: - raw = pytestconfig.getoption("--sift-test-results-log-file", default=None) - if raw is None: + """Determine log_file value from CLI flag or ini key.""" + raw = _option_or_ini(pytestconfig, _INI_LOG_FILE) + if not raw: + # None, empty string from ini, or False — treat as "use temp file default". return True lower = str(raw).lower() if lower in ("true", "1"): @@ -63,11 +128,11 @@ def _resolve_log_file(pytestconfig: pytest.Config | None) -> str | Path | bool | @pytest.hookimpl(tryfirst=True, hookwrapper=True) def pytest_runtest_makereport(item: pytest.Item, call: pytest.CallInfo[Any]): - """You should import this hook to capture any AssertionErrors that occur during the test. If not included, any assert failures in a test will not automatically fail the step.""" + """Capture pytest outcomes so assertion failures and skips land on the Sift step.""" outcome = yield report = outcome.get_result() if report.outcome == "skipped": - # Skipped steps won't invoke the method/fixtures at all, so we need to manually record a step. + # Skipped tests bypass the autouse `step` fixture, so we record the step manually here. if REPORT_CONTEXT: with REPORT_CONTEXT.new_step(name=item.name) as new_step: new_step.current_step.update({"status": TestStatus.SKIPPED}) @@ -88,11 +153,8 @@ def _report_context_impl( base_name = "pytest " + " ".join(args) if args else "pytest" test_case = base_name log_file = _resolve_log_file(pytestconfig) - include_git_metadata = ( - bool(pytestconfig.getoption("sift_test_results_git_metadata", default=True)) - if pytestconfig - else True - ) + git_metadata = _option_or_ini(pytestconfig, _INI_GIT_METADATA) + include_git_metadata = True if git_metadata is None else bool(git_metadata) with ReportContext( sift_client, name=f"{base_name} {datetime.now(timezone.utc).isoformat()}", @@ -100,17 +162,14 @@ def _report_context_impl( log_file=log_file, include_git_metadata=include_git_metadata, ) as context: - # Set a global so we can access this in pytest hooks. global REPORT_CONTEXT REPORT_CONTEXT = context yield context def _check_connection_enabled(pytestconfig: pytest.Config | None) -> bool: - """Return True when the caller opted into `--sift-test-results-check-connection`.""" - if pytestconfig is None: - return False - return bool(pytestconfig.getoption("sift_test_results_check_connection", default=False)) + """Return True when the caller opted into the check-connection mode via CLI or ini.""" + return bool(_option_or_ini(pytestconfig, _INI_CHECK_CONNECTION)) def _has_sift_connection(request: pytest.FixtureRequest) -> bool: @@ -118,30 +177,60 @@ def _has_sift_connection(request: pytest.FixtureRequest) -> bool: return bool(request.getfixturevalue("client_has_connection")) -def _required_env(name: str) -> str: - value = os.getenv(name) - if not value: - raise pytest.UsageError( - f"{name} must be set to use the default sift_client fixture; " - f"set the environment variable or override the sift_client fixture in conftest.py." - ) - return value +_CREDENTIAL_KEYS: tuple[tuple[str, str | None], ...] = ( + ("SIFT_API_KEY", None), # env-only; never read from ini to keep secrets out of source control. + ("SIFT_GRPC_URI", _INI_GRPC_URI), + ("SIFT_REST_URI", _INI_REST_URI), +) -@pytest.fixture(scope="session") -def sift_client() -> SiftClient: - """Default ``SiftClient`` resolved from environment variables. +def _resolve_credential( + pytestconfig: pytest.Config | None, env_name: str, ini_name: str | None +) -> str | None: + """Resolve a Sift credential: env var first, then ini key (if registered), else None.""" + env_value = os.getenv(env_name) + if env_value: + return env_value + if ini_name is None or pytestconfig is None: + return None + return pytestconfig.getini(ini_name) or None - Reads ``SIFT_API_KEY``, ``SIFT_GRPC_URI``, and ``SIFT_REST_URI``. Projects - that need custom construction (TLS toggles, custom timeouts, etc.) can - override this fixture by defining their own ``sift_client`` in their - ``conftest.py``; pytest fixture resolution prefers the local definition. + +@pytest.fixture(scope="session") +def sift_client(pytestconfig: pytest.Config) -> SiftClient: + """Default ``SiftClient`` resolved from environment variables and ini keys. + + Each credential is read from its environment variable first. The URIs + (``SIFT_GRPC_URI``, ``SIFT_REST_URI``) additionally fall back to the + ``sift_grpc_uri`` / ``sift_rest_uri`` ini keys, since they are stable + per-org values that are safe to commit. ``SIFT_API_KEY`` is intentionally + env-only — use ``pytest-dotenv`` (already a project dependency) to load + it from a ``.env`` file kept out of version control. + + Projects that need custom construction (TLS toggles, custom timeouts, + etc.) can override this fixture by defining their own ``sift_client`` + in their ``conftest.py``; pytest fixture resolution prefers the local + definition. """ + resolved = { + env: _resolve_credential(pytestconfig, env, ini) for env, ini in _CREDENTIAL_KEYS + } + missing = [env for env, value in resolved.items() if not value] + if missing: + raise pytest.UsageError( + "Sift credentials missing: " + + ", ".join(missing) + + ". Set the environment variable(s) — pytest-dotenv loads them " + "from a `.env` file automatically — or set the URIs via " + "`sift_grpc_uri` / `sift_rest_uri` under `[tool.pytest.ini_options]` " + "in pyproject.toml, or override the sift_client fixture in your " + "conftest.py." + ) return SiftClient( connection_config=SiftConnectionConfig( - api_key=_required_env("SIFT_API_KEY"), - grpc_url=_required_env("SIFT_GRPC_URI"), - rest_url=_required_env("SIFT_REST_URI"), + api_key=resolved["SIFT_API_KEY"], + grpc_url=resolved["SIFT_GRPC_URI"], + rest_url=resolved["SIFT_REST_URI"], ) ) @@ -186,16 +275,9 @@ def _step_impl( def step( report_context: ReportContext | None, request: pytest.FixtureRequest, - pytestconfig: pytest.Config, ) -> Generator[NewStep | None, None, None]: - """Create an outer step for the function. - - No-ops when ``--sift-test-results-check-connection`` is set and the client - has no connection (or when the session-scoped ``report_context`` resolved to None). - """ - if report_context is None or ( - _check_connection_enabled(pytestconfig) and not _has_sift_connection(request) - ): + """Create an outer step for the function. No-ops when ``report_context`` is None.""" + if report_context is None: yield None return yield from _step_impl(report_context, request) @@ -205,16 +287,9 @@ def step( def module_substep( report_context: ReportContext | None, request: pytest.FixtureRequest, - pytestconfig: pytest.Config, ) -> Generator[NewStep | None, None, None]: - """Create a step per module. - - No-ops when ``--sift-test-results-check-connection`` is set and the client - has no connection (or when the session-scoped ``report_context`` resolved to None). - """ - if report_context is None or ( - _check_connection_enabled(pytestconfig) and not _has_sift_connection(request) - ): + """Create a step per module. No-ops when ``report_context`` is None.""" + if report_context is None: yield None return yield from _step_impl(report_context, request) @@ -227,10 +302,8 @@ def client_has_connection(sift_client): Can be used to skip tests that require a connection to the Sift server, and is consulted by the Sift fixtures when ``--sift-test-results-check-connection`` is set. """ - has_connection = False try: sift_client.ping.ping() - has_connection = True + return True except Exception: - has_connection = False - return has_connection + return False diff --git a/python/lib/sift_client/util/test_results/__init__.py b/python/lib/sift_client/util/test_results/__init__.py index 93a25169a..a80396ded 100644 --- a/python/lib/sift_client/util/test_results/__init__.py +++ b/python/lib/sift_client/util/test_results/__init__.py @@ -83,6 +83,23 @@ def main(self): `module_substep` no-op when the client has no connection. Requires a `client_has_connection` fixture (the plugin ships a default). +Each option has a matching ini key for per-project configuration under +``[tool.pytest.ini_options]`` in ``pyproject.toml`` (or ``[pytest]`` in +``pytest.ini``). CLI flags override ini values. The default ``sift_client`` +fixture also reads ``sift_grpc_uri`` and ``sift_rest_uri`` as fallbacks when +the corresponding env vars are unset (env vars win when both are set). +``SIFT_API_KEY`` is env-only — load it from a ``.env`` file via the +``pytest-dotenv`` plugin or inject it via your CI secret manager. + +```toml +[tool.pytest.ini_options] +sift_test_results_log_file = "false" +sift_test_results_check_connection = true +sift_test_results_git_metadata = false +sift_grpc_uri = "your-org.sift.example:443" +sift_rest_uri = "https://your-org.sift.example" +``` + To disable the plugin for a single run: `pytest -p no:sift_client.pytest_plugin`. """ From 2805b0e9e2a106ca611da3d7ef9cfec9cb39e4f3 Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Sun, 17 May 2026 15:07:38 -0700 Subject: [PATCH 05/14] add pytest plugin tests --- .../sift_client/_tests/test_pytest_plugin.py | 237 ++++++++++++++++++ 1 file changed, 237 insertions(+) create mode 100644 python/lib/sift_client/_tests/test_pytest_plugin.py diff --git a/python/lib/sift_client/_tests/test_pytest_plugin.py b/python/lib/sift_client/_tests/test_pytest_plugin.py new file mode 100644 index 000000000..fcb054a69 --- /dev/null +++ b/python/lib/sift_client/_tests/test_pytest_plugin.py @@ -0,0 +1,237 @@ +"""Pytester-based tests for the Sift pytest plugin's configuration surface.""" + +from __future__ import annotations + +import textwrap +from pathlib import Path + +import pytest + +pytest_plugins = ["pytester"] + + +def _probe_conftest(pytester: pytest.Pytester, probe_body: str) -> None: + """Write a conftest that loads the plugin and runs ``probe_body`` in pytest_configure. + + ``probe_body`` is python source that runs at config time with ``config`` in scope; + use ``print(...)`` calls and capture them via ``result.stdout.fnmatch_lines``. + """ + pytester.makeconftest( + 'pytest_plugins = ["sift_client.pytest_plugin"]\n\n' + "def pytest_configure(config):\n" + + textwrap.indent(textwrap.dedent(probe_body), " ") + ) + + +def _plugin_conftest(pytester: pytest.Pytester) -> None: + pytester.makeconftest('pytest_plugins = ["sift_client.pytest_plugin"]') + + +class TestIniConfiguration: + """`addini` keys configure the plugin via pyproject.toml / pytest.ini.""" + + def test_ini_log_file_none(self, pytester: pytest.Pytester) -> None: + _probe_conftest( + pytester, + """ + from sift_client.pytest_plugin import _resolve_log_file + print("RESOLVED:", _resolve_log_file(config)) + """, + ) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_test_results_log_file = "none" + """ + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest("-s", "--co") + result.stdout.fnmatch_lines(["RESOLVED: None"]) + + def test_ini_log_file_path(self, pytester: pytest.Pytester, tmp_path: Path) -> None: + log_path = tmp_path / "sift-run.jsonl" + _probe_conftest( + pytester, + """ + from sift_client.pytest_plugin import _resolve_log_file + print("RESOLVED:", _resolve_log_file(config)) + """, + ) + pytester.makepyprojecttoml( + f""" + [tool.pytest.ini_options] + sift_test_results_log_file = "{log_path}" + """ + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest("-s", "--co") + result.stdout.fnmatch_lines([f"RESOLVED: {log_path}"]) + + def test_ini_check_connection_true(self, pytester: pytest.Pytester) -> None: + _probe_conftest( + pytester, + """ + from sift_client.pytest_plugin import _check_connection_enabled + print("CHECK:", _check_connection_enabled(config)) + """, + ) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_test_results_check_connection = true + """ + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest("-s", "--co") + result.stdout.fnmatch_lines(["CHECK: True"]) + + def test_ini_git_metadata_false(self, pytester: pytest.Pytester) -> None: + _probe_conftest( + pytester, + """ + print("INI_GIT:", config.getini("sift_test_results_git_metadata")) + """, + ) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_test_results_git_metadata = false + """ + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest("-s", "--co") + result.stdout.fnmatch_lines(["INI_GIT: False"]) + + def test_cli_overrides_ini(self, pytester: pytest.Pytester, tmp_path: Path) -> None: + """A CLI flag takes precedence over the matching ini key.""" + cli_path = tmp_path / "cli-wins.jsonl" + _probe_conftest( + pytester, + """ + from sift_client.pytest_plugin import _resolve_log_file + print("RESOLVED:", _resolve_log_file(config)) + """, + ) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_test_results_log_file = "none" + """ + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest( + "-s", "--co", f"--sift-test-results-log-file={cli_path}" + ) + result.stdout.fnmatch_lines([f"RESOLVED: {cli_path}"]) + + def test_uris_from_ini( + self, pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch + ) -> None: + """The default sift_client fixture reads URI credentials from ini when env vars are unset.""" + monkeypatch.setenv("SIFT_API_KEY", "env-key") + monkeypatch.delenv("SIFT_GRPC_URI", raising=False) + monkeypatch.delenv("SIFT_REST_URI", raising=False) + _plugin_conftest(pytester) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_grpc_uri = "ini-grpc:1234" + sift_rest_uri = "https://ini-rest" + sift_test_results_check_connection = true + sift_test_results_log_file = "false" + """ + ) + pytester.makepyfile( + """ + def test_credentials_loaded(sift_client): + cfg = sift_client.grpc_client._config + assert cfg.api_key == "env-key" + assert "ini-grpc:1234" in cfg.uri + """ + ) + result = pytester.runpytest() + result.assert_outcomes(passed=1) + + def test_env_var_overrides_ini_uri( + self, pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch + ) -> None: + """When both env var and ini set a URI, the env var wins.""" + monkeypatch.setenv("SIFT_API_KEY", "env-key") + monkeypatch.setenv("SIFT_GRPC_URI", "env-grpc:9999") + monkeypatch.delenv("SIFT_REST_URI", raising=False) + _plugin_conftest(pytester) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_grpc_uri = "ini-grpc:1234" + sift_rest_uri = "https://ini-rest" + sift_test_results_check_connection = true + sift_test_results_log_file = "false" + """ + ) + pytester.makepyfile( + """ + def test_env_wins(sift_client): + assert "env-grpc:9999" in sift_client.grpc_client._config.uri + """ + ) + result = pytester.runpytest() + result.assert_outcomes(passed=1) + + def test_api_key_ignored_from_ini( + self, pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch + ) -> None: + """`sift_api_key` is not registered as an ini key; the fixture refuses to use it.""" + for name in ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI"): + monkeypatch.delenv(name, raising=False) + _plugin_conftest(pytester) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_api_key = "should-be-ignored" + sift_grpc_uri = "ini-grpc:1234" + sift_rest_uri = "https://ini-rest" + """ + ) + pytester.makepyfile("def test_should_not_run(): pass") + result = pytester.runpytest() + assert result.ret != 0 + combined = "\n".join(result.outlines + result.errlines) + assert "SIFT_API_KEY" in combined, combined + + def test_missing_credentials_named_in_error( + self, pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A missing credential aborts with all missing names listed.""" + for name in ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI"): + monkeypatch.delenv(name, raising=False) + _plugin_conftest(pytester) + pytester.makepyfile("def test_should_not_run(): pass") + result = pytester.runpytest() + assert result.ret != 0 + combined = "\n".join(result.outlines + result.errlines) + for name in ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI"): + assert name in combined, combined + + def test_defaults_when_neither_set(self, pytester: pytest.Pytester) -> None: + _probe_conftest( + pytester, + """ + from sift_client.pytest_plugin import ( + _check_connection_enabled, + _resolve_log_file, + ) + print("RESOLVED:", _resolve_log_file(config)) + print("CHECK:", _check_connection_enabled(config)) + print("INI_GIT:", config.getini("sift_test_results_git_metadata")) + """, + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest("-s", "--co") + result.stdout.fnmatch_lines( + [ + "RESOLVED: True", + "CHECK: False", + "INI_GIT: True", + ] + ) From 8b0f23c2ba9f5811e162f94400cdf2084b0cdb91 Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Sun, 17 May 2026 15:28:03 -0700 Subject: [PATCH 06/14] format --- python/lib/sift_client/_tests/test_pytest_plugin.py | 7 ++----- python/lib/sift_client/pytest_plugin.py | 4 +--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/python/lib/sift_client/_tests/test_pytest_plugin.py b/python/lib/sift_client/_tests/test_pytest_plugin.py index fcb054a69..bb16bbede 100644 --- a/python/lib/sift_client/_tests/test_pytest_plugin.py +++ b/python/lib/sift_client/_tests/test_pytest_plugin.py @@ -18,8 +18,7 @@ def _probe_conftest(pytester: pytest.Pytester, probe_body: str) -> None: """ pytester.makeconftest( 'pytest_plugins = ["sift_client.pytest_plugin"]\n\n' - "def pytest_configure(config):\n" - + textwrap.indent(textwrap.dedent(probe_body), " ") + "def pytest_configure(config):\n" + textwrap.indent(textwrap.dedent(probe_body), " ") ) @@ -119,9 +118,7 @@ def test_cli_overrides_ini(self, pytester: pytest.Pytester, tmp_path: Path) -> N """ ) pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest( - "-s", "--co", f"--sift-test-results-log-file={cli_path}" - ) + result = pytester.runpytest("-s", "--co", f"--sift-test-results-log-file={cli_path}") result.stdout.fnmatch_lines([f"RESOLVED: {cli_path}"]) def test_uris_from_ini( diff --git a/python/lib/sift_client/pytest_plugin.py b/python/lib/sift_client/pytest_plugin.py index 69cd02b88..a22918fa0 100644 --- a/python/lib/sift_client/pytest_plugin.py +++ b/python/lib/sift_client/pytest_plugin.py @@ -212,9 +212,7 @@ def sift_client(pytestconfig: pytest.Config) -> SiftClient: in their ``conftest.py``; pytest fixture resolution prefers the local definition. """ - resolved = { - env: _resolve_credential(pytestconfig, env, ini) for env, ini in _CREDENTIAL_KEYS - } + resolved = {env: _resolve_credential(pytestconfig, env, ini) for env, ini in _CREDENTIAL_KEYS} missing = [env for env, value in resolved.items() if not value] if missing: raise pytest.UsageError( From 214d986d1eb279a095c58b5741f09fa887c52777 Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Sun, 17 May 2026 17:45:07 -0700 Subject: [PATCH 07/14] reorganize tests --- .../_tests/pytest_plugin/__init__.py | 0 .../_tests/pytest_plugin/conftest.py | 48 ++++ .../pytest_plugin/test_configuration.py | 154 ++++++++++++ .../_tests/pytest_plugin/test_credentials.py | 117 +++++++++ .../sift_client/_tests/test_pytest_plugin.py | 234 ------------------ 5 files changed, 319 insertions(+), 234 deletions(-) create mode 100644 python/lib/sift_client/_tests/pytest_plugin/__init__.py create mode 100644 python/lib/sift_client/_tests/pytest_plugin/conftest.py create mode 100644 python/lib/sift_client/_tests/pytest_plugin/test_configuration.py create mode 100644 python/lib/sift_client/_tests/pytest_plugin/test_credentials.py delete mode 100644 python/lib/sift_client/_tests/test_pytest_plugin.py diff --git a/python/lib/sift_client/_tests/pytest_plugin/__init__.py b/python/lib/sift_client/_tests/pytest_plugin/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/python/lib/sift_client/_tests/pytest_plugin/conftest.py b/python/lib/sift_client/_tests/pytest_plugin/conftest.py new file mode 100644 index 000000000..8ed6e9673 --- /dev/null +++ b/python/lib/sift_client/_tests/pytest_plugin/conftest.py @@ -0,0 +1,48 @@ +"""Shared helpers for the pytest-plugin test suite. + +The tests in this directory drive inner pytester sessions to exercise the +plugin's behavior in isolation. The fixtures below produce the boilerplate +conftests those inner sessions need: + +- ``write_plugin_conftest``: minimal conftest that loads the plugin +- ``write_probe_conftest``: conftest that loads the plugin and runs a probe + block inside ``pytest_configure``, useful for inspecting internal state + without running tests against a real backend +""" + +from __future__ import annotations + +import textwrap +from typing import Callable + +import pytest + +pytest_plugins = ["pytester"] + + +@pytest.fixture +def write_plugin_conftest(pytester: pytest.Pytester) -> Callable[[], None]: + """Return a callable that writes a minimal conftest loading the plugin.""" + + def _write() -> None: + pytester.makeconftest('pytest_plugins = ["sift_client.pytest_plugin"]') + + return _write + + +@pytest.fixture +def write_probe_conftest(pytester: pytest.Pytester) -> Callable[[str], None]: + """Return a callable that writes a conftest running ``probe_body`` in ``pytest_configure``. + + ``probe_body`` is python source that runs at config time with ``config`` + in scope; use ``print(...)`` calls and capture them with + ``result.stdout.fnmatch_lines``. + """ + + def _write(probe_body: str) -> None: + pytester.makeconftest( + 'pytest_plugins = ["sift_client.pytest_plugin"]\n\n' + "def pytest_configure(config):\n" + textwrap.indent(textwrap.dedent(probe_body), " ") + ) + + return _write diff --git a/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py b/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py new file mode 100644 index 000000000..272ba37c5 --- /dev/null +++ b/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py @@ -0,0 +1,154 @@ +"""Tests for the plugin's CLI/ini configuration surface. + +Covers flag parsing, ini-key resolution, CLI-over-ini precedence, and the +defaults that apply when nothing is set. Credentials are tested in +``test_credentials.py``. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Callable + +if TYPE_CHECKING: + from pathlib import Path + + import pytest + + +class TestIniConfiguration: + """`addini` keys configure the plugin via pyproject.toml / pytest.ini.""" + + def test_ini_log_file_none( + self, + pytester: pytest.Pytester, + write_probe_conftest: Callable[[str], None], + ) -> None: + write_probe_conftest( + """ + from sift_client.pytest_plugin import _resolve_log_file + print("RESOLVED:", _resolve_log_file(config)) + """, + ) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_test_results_log_file = "none" + """ + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest("-s", "--co") + result.stdout.fnmatch_lines(["RESOLVED: None"]) + + def test_ini_log_file_path( + self, + pytester: pytest.Pytester, + tmp_path: Path, + write_probe_conftest: Callable[[str], None], + ) -> None: + log_path = tmp_path / "sift-run.jsonl" + write_probe_conftest( + """ + from sift_client.pytest_plugin import _resolve_log_file + print("RESOLVED:", _resolve_log_file(config)) + """, + ) + pytester.makepyprojecttoml( + f""" + [tool.pytest.ini_options] + sift_test_results_log_file = "{log_path}" + """ + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest("-s", "--co") + result.stdout.fnmatch_lines([f"RESOLVED: {log_path}"]) + + def test_ini_check_connection_true( + self, + pytester: pytest.Pytester, + write_probe_conftest: Callable[[str], None], + ) -> None: + write_probe_conftest( + """ + from sift_client.pytest_plugin import _check_connection_enabled + print("CHECK:", _check_connection_enabled(config)) + """, + ) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_test_results_check_connection = true + """ + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest("-s", "--co") + result.stdout.fnmatch_lines(["CHECK: True"]) + + def test_ini_git_metadata_false( + self, + pytester: pytest.Pytester, + write_probe_conftest: Callable[[str], None], + ) -> None: + write_probe_conftest( + """ + print("INI_GIT:", config.getini("sift_test_results_git_metadata")) + """, + ) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_test_results_git_metadata = false + """ + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest("-s", "--co") + result.stdout.fnmatch_lines(["INI_GIT: False"]) + + def test_cli_overrides_ini( + self, + pytester: pytest.Pytester, + tmp_path: Path, + write_probe_conftest: Callable[[str], None], + ) -> None: + """A CLI flag takes precedence over the matching ini key.""" + cli_path = tmp_path / "cli-wins.jsonl" + write_probe_conftest( + """ + from sift_client.pytest_plugin import _resolve_log_file + print("RESOLVED:", _resolve_log_file(config)) + """, + ) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_test_results_log_file = "none" + """ + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest("-s", "--co", f"--sift-test-results-log-file={cli_path}") + result.stdout.fnmatch_lines([f"RESOLVED: {cli_path}"]) + + def test_defaults_when_neither_set( + self, + pytester: pytest.Pytester, + write_probe_conftest: Callable[[str], None], + ) -> None: + write_probe_conftest( + """ + from sift_client.pytest_plugin import ( + _check_connection_enabled, + _resolve_log_file, + ) + print("RESOLVED:", _resolve_log_file(config)) + print("CHECK:", _check_connection_enabled(config)) + print("INI_GIT:", config.getini("sift_test_results_git_metadata")) + """, + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest("-s", "--co") + result.stdout.fnmatch_lines( + [ + "RESOLVED: True", + "CHECK: False", + "INI_GIT: True", + ] + ) diff --git a/python/lib/sift_client/_tests/pytest_plugin/test_credentials.py b/python/lib/sift_client/_tests/pytest_plugin/test_credentials.py new file mode 100644 index 000000000..e84f8f15e --- /dev/null +++ b/python/lib/sift_client/_tests/pytest_plugin/test_credentials.py @@ -0,0 +1,117 @@ +"""Tests for the default ``sift_client`` fixture's credential resolution. + +Covers the env-var-then-ini fallback for URIs, the env-only handling of +``SIFT_API_KEY``, and the error path that names missing credentials. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Callable + +if TYPE_CHECKING: + import pytest + + +class TestCredentials: + """The default ``sift_client`` fixture's resolution of env vars and ini keys.""" + + def test_uris_from_ini( + self, + pytester: pytest.Pytester, + monkeypatch: pytest.MonkeyPatch, + write_plugin_conftest: Callable[[], None], + ) -> None: + """The default sift_client fixture reads URI credentials from ini when env vars are unset.""" + monkeypatch.setenv("SIFT_API_KEY", "env-key") + monkeypatch.delenv("SIFT_GRPC_URI", raising=False) + monkeypatch.delenv("SIFT_REST_URI", raising=False) + write_plugin_conftest() + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_grpc_uri = "ini-grpc:1234" + sift_rest_uri = "https://ini-rest" + sift_test_results_check_connection = true + sift_test_results_log_file = "false" + """ + ) + pytester.makepyfile( + """ + def test_credentials_loaded(sift_client): + cfg = sift_client.grpc_client._config + assert cfg.api_key == "env-key" + assert "ini-grpc:1234" in cfg.uri + """ + ) + result = pytester.runpytest() + result.assert_outcomes(passed=1) + + def test_env_var_overrides_ini_uri( + self, + pytester: pytest.Pytester, + monkeypatch: pytest.MonkeyPatch, + write_plugin_conftest: Callable[[], None], + ) -> None: + """When both env var and ini set a URI, the env var wins.""" + monkeypatch.setenv("SIFT_API_KEY", "env-key") + monkeypatch.setenv("SIFT_GRPC_URI", "env-grpc:9999") + monkeypatch.delenv("SIFT_REST_URI", raising=False) + write_plugin_conftest() + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_grpc_uri = "ini-grpc:1234" + sift_rest_uri = "https://ini-rest" + sift_test_results_check_connection = true + sift_test_results_log_file = "false" + """ + ) + pytester.makepyfile( + """ + def test_env_wins(sift_client): + assert "env-grpc:9999" in sift_client.grpc_client._config.uri + """ + ) + result = pytester.runpytest() + result.assert_outcomes(passed=1) + + def test_api_key_ignored_from_ini( + self, + pytester: pytest.Pytester, + monkeypatch: pytest.MonkeyPatch, + write_plugin_conftest: Callable[[], None], + ) -> None: + """`sift_api_key` is not registered as an ini key; the fixture refuses to use it.""" + for name in ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI"): + monkeypatch.delenv(name, raising=False) + write_plugin_conftest() + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_api_key = "should-be-ignored" + sift_grpc_uri = "ini-grpc:1234" + sift_rest_uri = "https://ini-rest" + """ + ) + pytester.makepyfile("def test_should_not_run(): pass") + result = pytester.runpytest() + assert result.ret != 0 + combined = "\n".join(result.outlines + result.errlines) + assert "SIFT_API_KEY" in combined, combined + + def test_missing_credentials_named_in_error( + self, + pytester: pytest.Pytester, + monkeypatch: pytest.MonkeyPatch, + write_plugin_conftest: Callable[[], None], + ) -> None: + """A missing credential aborts with all missing names listed.""" + for name in ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI"): + monkeypatch.delenv(name, raising=False) + write_plugin_conftest() + pytester.makepyfile("def test_should_not_run(): pass") + result = pytester.runpytest() + assert result.ret != 0 + combined = "\n".join(result.outlines + result.errlines) + for name in ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI"): + assert name in combined, combined diff --git a/python/lib/sift_client/_tests/test_pytest_plugin.py b/python/lib/sift_client/_tests/test_pytest_plugin.py deleted file mode 100644 index bb16bbede..000000000 --- a/python/lib/sift_client/_tests/test_pytest_plugin.py +++ /dev/null @@ -1,234 +0,0 @@ -"""Pytester-based tests for the Sift pytest plugin's configuration surface.""" - -from __future__ import annotations - -import textwrap -from pathlib import Path - -import pytest - -pytest_plugins = ["pytester"] - - -def _probe_conftest(pytester: pytest.Pytester, probe_body: str) -> None: - """Write a conftest that loads the plugin and runs ``probe_body`` in pytest_configure. - - ``probe_body`` is python source that runs at config time with ``config`` in scope; - use ``print(...)`` calls and capture them via ``result.stdout.fnmatch_lines``. - """ - pytester.makeconftest( - 'pytest_plugins = ["sift_client.pytest_plugin"]\n\n' - "def pytest_configure(config):\n" + textwrap.indent(textwrap.dedent(probe_body), " ") - ) - - -def _plugin_conftest(pytester: pytest.Pytester) -> None: - pytester.makeconftest('pytest_plugins = ["sift_client.pytest_plugin"]') - - -class TestIniConfiguration: - """`addini` keys configure the plugin via pyproject.toml / pytest.ini.""" - - def test_ini_log_file_none(self, pytester: pytest.Pytester) -> None: - _probe_conftest( - pytester, - """ - from sift_client.pytest_plugin import _resolve_log_file - print("RESOLVED:", _resolve_log_file(config)) - """, - ) - pytester.makepyprojecttoml( - """ - [tool.pytest.ini_options] - sift_test_results_log_file = "none" - """ - ) - pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co") - result.stdout.fnmatch_lines(["RESOLVED: None"]) - - def test_ini_log_file_path(self, pytester: pytest.Pytester, tmp_path: Path) -> None: - log_path = tmp_path / "sift-run.jsonl" - _probe_conftest( - pytester, - """ - from sift_client.pytest_plugin import _resolve_log_file - print("RESOLVED:", _resolve_log_file(config)) - """, - ) - pytester.makepyprojecttoml( - f""" - [tool.pytest.ini_options] - sift_test_results_log_file = "{log_path}" - """ - ) - pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co") - result.stdout.fnmatch_lines([f"RESOLVED: {log_path}"]) - - def test_ini_check_connection_true(self, pytester: pytest.Pytester) -> None: - _probe_conftest( - pytester, - """ - from sift_client.pytest_plugin import _check_connection_enabled - print("CHECK:", _check_connection_enabled(config)) - """, - ) - pytester.makepyprojecttoml( - """ - [tool.pytest.ini_options] - sift_test_results_check_connection = true - """ - ) - pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co") - result.stdout.fnmatch_lines(["CHECK: True"]) - - def test_ini_git_metadata_false(self, pytester: pytest.Pytester) -> None: - _probe_conftest( - pytester, - """ - print("INI_GIT:", config.getini("sift_test_results_git_metadata")) - """, - ) - pytester.makepyprojecttoml( - """ - [tool.pytest.ini_options] - sift_test_results_git_metadata = false - """ - ) - pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co") - result.stdout.fnmatch_lines(["INI_GIT: False"]) - - def test_cli_overrides_ini(self, pytester: pytest.Pytester, tmp_path: Path) -> None: - """A CLI flag takes precedence over the matching ini key.""" - cli_path = tmp_path / "cli-wins.jsonl" - _probe_conftest( - pytester, - """ - from sift_client.pytest_plugin import _resolve_log_file - print("RESOLVED:", _resolve_log_file(config)) - """, - ) - pytester.makepyprojecttoml( - """ - [tool.pytest.ini_options] - sift_test_results_log_file = "none" - """ - ) - pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co", f"--sift-test-results-log-file={cli_path}") - result.stdout.fnmatch_lines([f"RESOLVED: {cli_path}"]) - - def test_uris_from_ini( - self, pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch - ) -> None: - """The default sift_client fixture reads URI credentials from ini when env vars are unset.""" - monkeypatch.setenv("SIFT_API_KEY", "env-key") - monkeypatch.delenv("SIFT_GRPC_URI", raising=False) - monkeypatch.delenv("SIFT_REST_URI", raising=False) - _plugin_conftest(pytester) - pytester.makepyprojecttoml( - """ - [tool.pytest.ini_options] - sift_grpc_uri = "ini-grpc:1234" - sift_rest_uri = "https://ini-rest" - sift_test_results_check_connection = true - sift_test_results_log_file = "false" - """ - ) - pytester.makepyfile( - """ - def test_credentials_loaded(sift_client): - cfg = sift_client.grpc_client._config - assert cfg.api_key == "env-key" - assert "ini-grpc:1234" in cfg.uri - """ - ) - result = pytester.runpytest() - result.assert_outcomes(passed=1) - - def test_env_var_overrides_ini_uri( - self, pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch - ) -> None: - """When both env var and ini set a URI, the env var wins.""" - monkeypatch.setenv("SIFT_API_KEY", "env-key") - monkeypatch.setenv("SIFT_GRPC_URI", "env-grpc:9999") - monkeypatch.delenv("SIFT_REST_URI", raising=False) - _plugin_conftest(pytester) - pytester.makepyprojecttoml( - """ - [tool.pytest.ini_options] - sift_grpc_uri = "ini-grpc:1234" - sift_rest_uri = "https://ini-rest" - sift_test_results_check_connection = true - sift_test_results_log_file = "false" - """ - ) - pytester.makepyfile( - """ - def test_env_wins(sift_client): - assert "env-grpc:9999" in sift_client.grpc_client._config.uri - """ - ) - result = pytester.runpytest() - result.assert_outcomes(passed=1) - - def test_api_key_ignored_from_ini( - self, pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch - ) -> None: - """`sift_api_key` is not registered as an ini key; the fixture refuses to use it.""" - for name in ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI"): - monkeypatch.delenv(name, raising=False) - _plugin_conftest(pytester) - pytester.makepyprojecttoml( - """ - [tool.pytest.ini_options] - sift_api_key = "should-be-ignored" - sift_grpc_uri = "ini-grpc:1234" - sift_rest_uri = "https://ini-rest" - """ - ) - pytester.makepyfile("def test_should_not_run(): pass") - result = pytester.runpytest() - assert result.ret != 0 - combined = "\n".join(result.outlines + result.errlines) - assert "SIFT_API_KEY" in combined, combined - - def test_missing_credentials_named_in_error( - self, pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch - ) -> None: - """A missing credential aborts with all missing names listed.""" - for name in ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI"): - monkeypatch.delenv(name, raising=False) - _plugin_conftest(pytester) - pytester.makepyfile("def test_should_not_run(): pass") - result = pytester.runpytest() - assert result.ret != 0 - combined = "\n".join(result.outlines + result.errlines) - for name in ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI"): - assert name in combined, combined - - def test_defaults_when_neither_set(self, pytester: pytest.Pytester) -> None: - _probe_conftest( - pytester, - """ - from sift_client.pytest_plugin import ( - _check_connection_enabled, - _resolve_log_file, - ) - print("RESOLVED:", _resolve_log_file(config)) - print("CHECK:", _check_connection_enabled(config)) - print("INI_GIT:", config.getini("sift_test_results_git_metadata")) - """, - ) - pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co") - result.stdout.fnmatch_lines( - [ - "RESOLVED: True", - "CHECK: False", - "INI_GIT: True", - ] - ) From 3c23ed2c8dda27b34710f9c87fe78eb66e056922 Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Mon, 18 May 2026 10:20:55 -0700 Subject: [PATCH 08/14] mypy --- python/lib/sift_client/pytest_plugin.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/python/lib/sift_client/pytest_plugin.py b/python/lib/sift_client/pytest_plugin.py index a22918fa0..4ab6f7657 100644 --- a/python/lib/sift_client/pytest_plugin.py +++ b/python/lib/sift_client/pytest_plugin.py @@ -224,11 +224,13 @@ def sift_client(pytestconfig: pytest.Config) -> SiftClient: "in pyproject.toml, or override the sift_client fixture in your " "conftest.py." ) + # `or ""` is unreachable in practice since the `missing` check above guarantees + # non-None values return SiftClient( connection_config=SiftConnectionConfig( - api_key=resolved["SIFT_API_KEY"], - grpc_url=resolved["SIFT_GRPC_URI"], - rest_url=resolved["SIFT_REST_URI"], + api_key=resolved.get("SIFT_API_KEY") or "", + grpc_url=resolved.get("SIFT_GRPC_URI") or "", + rest_url=resolved.get("SIFT_REST_URI") or "", ) ) From ef5991f245788202bfed9423d67229ccf814344d Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Mon, 18 May 2026 10:29:21 -0700 Subject: [PATCH 09/14] linting --- python/lib/sift_client/pytest_plugin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/lib/sift_client/pytest_plugin.py b/python/lib/sift_client/pytest_plugin.py index 4ab6f7657..96594a0c7 100644 --- a/python/lib/sift_client/pytest_plugin.py +++ b/python/lib/sift_client/pytest_plugin.py @@ -193,7 +193,8 @@ def _resolve_credential( return env_value if ini_name is None or pytestconfig is None: return None - return pytestconfig.getini(ini_name) or None + ini_value = pytestconfig.getini(ini_name) + return ini_value if isinstance(ini_value, str) and ini_value else None @pytest.fixture(scope="session") From 909665a0247a2076b16300ee47dc344f00e595cf Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Mon, 18 May 2026 15:02:29 -0700 Subject: [PATCH 10/14] clean up based on feedback --- .../pytest_plugin/test_configuration.py | 36 ++++ python/lib/sift_client/pytest_plugin.py | 189 ++++++++++-------- 2 files changed, 144 insertions(+), 81 deletions(-) diff --git a/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py b/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py index 272ba37c5..b324a3de5 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py +++ b/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py @@ -127,6 +127,42 @@ def test_cli_overrides_ini( result = pytester.runpytest("-s", "--co", f"--sift-test-results-log-file={cli_path}") result.stdout.fnmatch_lines([f"RESOLVED: {cli_path}"]) + def test_cli_check_connection_flag( + self, + pytester: pytest.Pytester, + write_probe_conftest: Callable[[str], None], + ) -> None: + """The ``--sift-test-results-check-connection`` CLI flag flips the resolver to True.""" + write_probe_conftest( + """ + from sift_client.pytest_plugin import _check_connection_enabled + print("CHECK:", _check_connection_enabled(config)) + """, + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest("-s", "--co", "--sift-test-results-check-connection") + result.stdout.fnmatch_lines(["CHECK: True"]) + + def test_cli_no_git_metadata_flag( + self, + pytester: pytest.Pytester, + write_probe_conftest: Callable[[str], None], + ) -> None: + """The ``--no-sift-test-results-git-metadata`` CLI flag flips git_metadata to False. + + Guards the negation flag's ``dest`` binding: the flag name doesn't match + the ini key, so a broken ``dest`` would silently fall back to the ini + default and pass every other test in this file. + """ + write_probe_conftest( + """ + print("CLI_GIT:", config.getoption("sift_test_results_git_metadata")) + """, + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest("-s", "--co", "--no-sift-test-results-git-metadata") + result.stdout.fnmatch_lines(["CLI_GIT: False"]) + def test_defaults_when_neither_set( self, pytester: pytest.Pytester, diff --git a/python/lib/sift_client/pytest_plugin.py b/python/lib/sift_client/pytest_plugin.py index 96594a0c7..48175f7f9 100644 --- a/python/lib/sift_client/pytest_plugin.py +++ b/python/lib/sift_client/pytest_plugin.py @@ -1,6 +1,7 @@ from __future__ import annotations import os +from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path from typing import TYPE_CHECKING, Any, Generator @@ -16,12 +17,83 @@ REPORT_CONTEXT: ReportContext | None = None -# Ini keys that mirror the registered CLI options. Centralized so renames stay in sync. -_INI_LOG_FILE = "sift_test_results_log_file" -_INI_GIT_METADATA = "sift_test_results_git_metadata" -_INI_CHECK_CONNECTION = "sift_test_results_check_connection" -_INI_GRPC_URI = "sift_grpc_uri" -_INI_REST_URI = "sift_rest_uri" +@dataclass(frozen=True) +class _Option: + """A single Sift plugin setting, registered as a CLI flag and/or an ini key. + + ``ini_name`` is used as both the ini key and the CLI ``dest``, so a value + set either way lands on the same config slot. ``cli_flag=None`` makes the + option ini-only (e.g. the URI fallbacks). + """ + + ini_name: str + ini_help: str + cli_flag: str | None = None + cli_help: str | None = None + action: str | None = None + ini_type: str | None = None + ini_default: Any = None + + +_LOG_FILE = _Option( + cli_flag="--sift-test-results-log-file", + ini_name="sift_test_results_log_file", + cli_help="Path to write the Sift test result log file. " + "Use 'true' (default) to auto-create a temp file, " + "False, 'false', or 'none' to disable logging, " + "or a file path to write to a specific location.", + ini_help="Default value for --sift-test-results-log-file. Same values " + "accepted as the CLI flag (path, 'true', 'false', 'none').", +) + +_GIT_METADATA = _Option( + cli_flag="--no-sift-test-results-git-metadata", + ini_name="sift_test_results_git_metadata", + action="store_false", + cli_help="Exclude git metadata from the Sift test results. " + "Git metadata (repo, branch, commit) is included by default.", + ini_help="Include git repo/branch/commit in the report (true/false). " + "Defaults to true. The --no-sift-test-results-git-metadata CLI flag " + "overrides this when passed.", + ini_type="bool", + ini_default=True, +) + +_CHECK_CONNECTION = _Option( + cli_flag="--sift-test-results-check-connection", + ini_name="sift_test_results_check_connection", + action="store_true", + cli_help="Skip the sift test-result fixtures (report_context, step, module_substep) " + "when the Sift client has no connection to the server. Requires a " + "`client_has_connection` fixture to be available in the test session.", + ini_help="When true, skip the sift test-result fixtures if the client has " + "no connection (same effect as --sift-test-results-check-connection). " + "Defaults to false.", + ini_type="bool", + ini_default=False, +) + +_GRPC_URI = _Option( + ini_name="sift_grpc_uri", + ini_help="Sift gRPC endpoint URI. The default `sift_client` fixture " + "prefers the SIFT_GRPC_URI environment variable and falls back to " + "this ini value.", +) + +_REST_URI = _Option( + ini_name="sift_rest_uri", + ini_help="Sift REST endpoint URI. The default `sift_client` fixture " + "prefers the SIFT_REST_URI environment variable and falls back to " + "this ini value.", +) + +_OPTIONS: tuple[_Option, ...] = ( + _LOG_FILE, + _GIT_METADATA, + _CHECK_CONNECTION, + _GRPC_URI, + _REST_URI, +) def pytest_addoption(parser: pytest.Parser) -> None: @@ -32,69 +104,24 @@ def pytest_addoption(parser: pytest.Parser) -> None: precedence over ini values, which take precedence over the built-in default. """ group = parser.getgroup("sift", description="Sift test results") - group.addoption( - "--sift-test-results-log-file", - default=None, - help="Path to write the Sift test result log file. " - "Use 'true' (default) to auto-create a temp file, " - "False, 'false', or 'none' to disable logging, " - "or a file path to write to a specific location.", - ) - parser.addini( - _INI_LOG_FILE, - help="Default value for --sift-test-results-log-file. Same values " - "accepted as the CLI flag (path, 'true', 'false', 'none').", - default=None, - ) - group.addoption( - "--no-sift-test-results-git-metadata", - action="store_false", - dest="sift_test_results_git_metadata", - default=None, - help="Exclude git metadata from the Sift test results. " - "Git metadata (repo, branch, commit) is included by default.", - ) - parser.addini( - _INI_GIT_METADATA, - help="Include git repo/branch/commit in the report (true/false). " - "Defaults to true. The --no-sift-test-results-git-metadata CLI flag " - "overrides this when passed.", - type="bool", - default=True, - ) - group.addoption( - "--sift-test-results-check-connection", - action="store_true", - default=None, - help="Skip the sift test-result fixtures (report_context, step, module_substep) " - "when the Sift client has no connection to the server. Requires a " - "`client_has_connection` fixture to be available in the test session.", - ) - parser.addini( - _INI_CHECK_CONNECTION, - help="When true, skip the sift test-result fixtures if the client has " - "no connection (same effect as --sift-test-results-check-connection). " - "Defaults to false.", - type="bool", - default=False, - ) - parser.addini( - _INI_GRPC_URI, - help="Sift gRPC endpoint URI. The default `sift_client` fixture " - "prefers the SIFT_GRPC_URI environment variable and falls back to " - "this ini value.", - default=None, - ) - parser.addini( - _INI_REST_URI, - help="Sift REST endpoint URI. The default `sift_client` fixture " - "prefers the SIFT_REST_URI environment variable and falls back to " - "this ini value.", - default=None, - ) - - -def _option_or_ini(pytestconfig: pytest.Config | None, name: str) -> Any: + for opt in _OPTIONS: + if opt.cli_flag is not None: + cli_kwargs: dict[str, Any] = { + "dest": opt.ini_name, + "default": None, + "help": opt.cli_help, + } + if opt.action is not None: + cli_kwargs["action"] = opt.action + group.addoption(opt.cli_flag, **cli_kwargs) + + ini_kwargs: dict[str, Any] = {"help": opt.ini_help, "default": opt.ini_default} + if opt.ini_type is not None: + ini_kwargs["type"] = opt.ini_type + parser.addini(opt.ini_name, **ini_kwargs) + + +def _option_or_ini(pytestconfig: pytest.Config | None, opt: _Option) -> Any: """Resolve a Sift plugin setting from CLI > ini > None. The ``addoption`` registrations use ``default=None`` so we can tell whether @@ -103,18 +130,18 @@ def _option_or_ini(pytestconfig: pytest.Config | None, name: str) -> Any: """ if pytestconfig is None: return None - cli = pytestconfig.getoption(name, default=None) + cli = pytestconfig.getoption(opt.ini_name, default=None) if cli is not None: return cli try: - return pytestconfig.getini(name) + return pytestconfig.getini(opt.ini_name) except (KeyError, ValueError): return None def _resolve_log_file(pytestconfig: pytest.Config | None) -> str | Path | bool | None: """Determine log_file value from CLI flag or ini key.""" - raw = _option_or_ini(pytestconfig, _INI_LOG_FILE) + raw = _option_or_ini(pytestconfig, _LOG_FILE) if not raw: # None, empty string from ini, or False — treat as "use temp file default". return True @@ -153,7 +180,7 @@ def _report_context_impl( base_name = "pytest " + " ".join(args) if args else "pytest" test_case = base_name log_file = _resolve_log_file(pytestconfig) - git_metadata = _option_or_ini(pytestconfig, _INI_GIT_METADATA) + git_metadata = _option_or_ini(pytestconfig, _GIT_METADATA) include_git_metadata = True if git_metadata is None else bool(git_metadata) with ReportContext( sift_client, @@ -169,7 +196,7 @@ def _report_context_impl( def _check_connection_enabled(pytestconfig: pytest.Config | None) -> bool: """Return True when the caller opted into the check-connection mode via CLI or ini.""" - return bool(_option_or_ini(pytestconfig, _INI_CHECK_CONNECTION)) + return bool(_option_or_ini(pytestconfig, _CHECK_CONNECTION)) def _has_sift_connection(request: pytest.FixtureRequest) -> bool: @@ -177,23 +204,23 @@ def _has_sift_connection(request: pytest.FixtureRequest) -> bool: return bool(request.getfixturevalue("client_has_connection")) -_CREDENTIAL_KEYS: tuple[tuple[str, str | None], ...] = ( +_CREDENTIAL_KEYS: tuple[tuple[str, _Option | None], ...] = ( ("SIFT_API_KEY", None), # env-only; never read from ini to keep secrets out of source control. - ("SIFT_GRPC_URI", _INI_GRPC_URI), - ("SIFT_REST_URI", _INI_REST_URI), + ("SIFT_GRPC_URI", _GRPC_URI), + ("SIFT_REST_URI", _REST_URI), ) def _resolve_credential( - pytestconfig: pytest.Config | None, env_name: str, ini_name: str | None + pytestconfig: pytest.Config | None, env_name: str, opt: _Option | None ) -> str | None: """Resolve a Sift credential: env var first, then ini key (if registered), else None.""" env_value = os.getenv(env_name) if env_value: return env_value - if ini_name is None or pytestconfig is None: + if opt is None or pytestconfig is None: return None - ini_value = pytestconfig.getini(ini_name) + ini_value = pytestconfig.getini(opt.ini_name) return ini_value if isinstance(ini_value, str) and ini_value else None @@ -213,7 +240,7 @@ def sift_client(pytestconfig: pytest.Config) -> SiftClient: in their ``conftest.py``; pytest fixture resolution prefers the local definition. """ - resolved = {env: _resolve_credential(pytestconfig, env, ini) for env, ini in _CREDENTIAL_KEYS} + resolved = {env: _resolve_credential(pytestconfig, env, opt) for env, opt in _CREDENTIAL_KEYS} missing = [env for env, value in resolved.items() if not value] if missing: raise pytest.UsageError( From 1d1b414796e0f178fd4bed33d2f33c46c5ff8a3d Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Mon, 18 May 2026 15:22:21 -0700 Subject: [PATCH 11/14] clean up tests --- .../_tests/pytest_plugin/conftest.py | 8 ++++++++ .../_tests/pytest_plugin/test_configuration.py | 18 ++++++++++-------- .../_tests/pytest_plugin/test_credentials.py | 8 ++++---- python/lib/sift_client/pytest_plugin.py | 1 + 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/python/lib/sift_client/_tests/pytest_plugin/conftest.py b/python/lib/sift_client/_tests/pytest_plugin/conftest.py index 8ed6e9673..71f31c060 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/conftest.py +++ b/python/lib/sift_client/_tests/pytest_plugin/conftest.py @@ -8,6 +8,14 @@ - ``write_probe_conftest``: conftest that loads the plugin and runs a probe block inside ``pytest_configure``, useful for inspecting internal state without running tests against a real backend + +Every test in this suite invokes the inner session via +``pytester.runpytest_subprocess(...)`` rather than ``pytester.runpytest(...)``. +``runpytest`` runs the inner pytest in-process, which re-imports the Sift +plugin on each test; the plugin transitively imports numpy, whose C +extensions refuse to initialize twice in one process and raise +``cannot load module more than once per process``. Spawning a subprocess +gives each inner session a fresh interpreter and sidesteps that guard. """ from __future__ import annotations diff --git a/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py b/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py index b324a3de5..0ffefcedf 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py +++ b/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py @@ -36,7 +36,7 @@ def test_ini_log_file_none( """ ) pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co") + result = pytester.runpytest_subprocess("-s", "--co") result.stdout.fnmatch_lines(["RESOLVED: None"]) def test_ini_log_file_path( @@ -59,7 +59,7 @@ def test_ini_log_file_path( """ ) pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co") + result = pytester.runpytest_subprocess("-s", "--co") result.stdout.fnmatch_lines([f"RESOLVED: {log_path}"]) def test_ini_check_connection_true( @@ -80,7 +80,7 @@ def test_ini_check_connection_true( """ ) pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co") + result = pytester.runpytest_subprocess("-s", "--co") result.stdout.fnmatch_lines(["CHECK: True"]) def test_ini_git_metadata_false( @@ -100,7 +100,7 @@ def test_ini_git_metadata_false( """ ) pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co") + result = pytester.runpytest_subprocess("-s", "--co") result.stdout.fnmatch_lines(["INI_GIT: False"]) def test_cli_overrides_ini( @@ -124,7 +124,9 @@ def test_cli_overrides_ini( """ ) pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co", f"--sift-test-results-log-file={cli_path}") + result = pytester.runpytest_subprocess( + "-s", "--co", f"--sift-test-results-log-file={cli_path}" + ) result.stdout.fnmatch_lines([f"RESOLVED: {cli_path}"]) def test_cli_check_connection_flag( @@ -140,7 +142,7 @@ def test_cli_check_connection_flag( """, ) pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co", "--sift-test-results-check-connection") + result = pytester.runpytest_subprocess("-s", "--co", "--sift-test-results-check-connection") result.stdout.fnmatch_lines(["CHECK: True"]) def test_cli_no_git_metadata_flag( @@ -160,7 +162,7 @@ def test_cli_no_git_metadata_flag( """, ) pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co", "--no-sift-test-results-git-metadata") + result = pytester.runpytest_subprocess("-s", "--co", "--no-sift-test-results-git-metadata") result.stdout.fnmatch_lines(["CLI_GIT: False"]) def test_defaults_when_neither_set( @@ -180,7 +182,7 @@ def test_defaults_when_neither_set( """, ) pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest("-s", "--co") + result = pytester.runpytest_subprocess("-s", "--co") result.stdout.fnmatch_lines( [ "RESOLVED: True", diff --git a/python/lib/sift_client/_tests/pytest_plugin/test_credentials.py b/python/lib/sift_client/_tests/pytest_plugin/test_credentials.py index e84f8f15e..9ee628e69 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/test_credentials.py +++ b/python/lib/sift_client/_tests/pytest_plugin/test_credentials.py @@ -43,7 +43,7 @@ def test_credentials_loaded(sift_client): assert "ini-grpc:1234" in cfg.uri """ ) - result = pytester.runpytest() + result = pytester.runpytest_subprocess() result.assert_outcomes(passed=1) def test_env_var_overrides_ini_uri( @@ -72,7 +72,7 @@ def test_env_wins(sift_client): assert "env-grpc:9999" in sift_client.grpc_client._config.uri """ ) - result = pytester.runpytest() + result = pytester.runpytest_subprocess() result.assert_outcomes(passed=1) def test_api_key_ignored_from_ini( @@ -94,7 +94,7 @@ def test_api_key_ignored_from_ini( """ ) pytester.makepyfile("def test_should_not_run(): pass") - result = pytester.runpytest() + result = pytester.runpytest_subprocess() assert result.ret != 0 combined = "\n".join(result.outlines + result.errlines) assert "SIFT_API_KEY" in combined, combined @@ -110,7 +110,7 @@ def test_missing_credentials_named_in_error( monkeypatch.delenv(name, raising=False) write_plugin_conftest() pytester.makepyfile("def test_should_not_run(): pass") - result = pytester.runpytest() + result = pytester.runpytest_subprocess() assert result.ret != 0 combined = "\n".join(result.outlines + result.errlines) for name in ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI"): diff --git a/python/lib/sift_client/pytest_plugin.py b/python/lib/sift_client/pytest_plugin.py index 48175f7f9..db12ce2ee 100644 --- a/python/lib/sift_client/pytest_plugin.py +++ b/python/lib/sift_client/pytest_plugin.py @@ -17,6 +17,7 @@ REPORT_CONTEXT: ReportContext | None = None + @dataclass(frozen=True) class _Option: """A single Sift plugin setting, registered as a CLI flag and/or an ini key. From f6a4d6baee0f841d26e1fe93f7316ed284636097 Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Mon, 18 May 2026 15:56:07 -0700 Subject: [PATCH 12/14] fix ci --- python/lib/sift_client/_tests/pytest_plugin/conftest.py | 2 -- python/pyproject.toml | 4 ++++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/python/lib/sift_client/_tests/pytest_plugin/conftest.py b/python/lib/sift_client/_tests/pytest_plugin/conftest.py index 71f31c060..1fbd61e46 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/conftest.py +++ b/python/lib/sift_client/_tests/pytest_plugin/conftest.py @@ -25,8 +25,6 @@ import pytest -pytest_plugins = ["pytester"] - @pytest.fixture def write_plugin_conftest(pytester: pytest.Pytester) -> Callable[[], None]: diff --git a/python/pyproject.toml b/python/pyproject.toml index 4e0e7165e..dc7169ed3 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -388,6 +388,10 @@ select = [ env_files = [ ".env" ] +# `pytester` is registered globally because pytest 8+ disallows `pytest_plugins` +# in non-top-level conftests. Only the plugin test suite uses it; activating it +# globally is harmless since the fixture is opt-in. +addopts = "-p pytester" testpaths = [ "lib/sift_py", "lib/sift_client/_tests", From 91695422ef42944437bb53682830c192099c4f1c Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Mon, 18 May 2026 19:33:08 -0700 Subject: [PATCH 13/14] fix handling of log file=false and add autouse gate --- .../pytest_plugin/test_configuration.py | 206 +++++++++++++++++- .../lib/sift_client/_tests/util/conftest.py | 29 +++ python/lib/sift_client/pytest_plugin.py | 126 +++++++++-- python/pyproject.toml | 5 + 4 files changed, 350 insertions(+), 16 deletions(-) diff --git a/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py b/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py index 0ffefcedf..9b9be2d63 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py +++ b/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py @@ -1,12 +1,14 @@ """Tests for the plugin's CLI/ini configuration surface. -Covers flag parsing, ini-key resolution, CLI-over-ini precedence, and the -defaults that apply when nothing is set. Credentials are tested in +Covers flag parsing, ini-key resolution, CLI-over-ini precedence, the +defaults that apply when nothing is set, and the marker-based gate that +governs the autouse fixtures. Credentials are tested in ``test_credentials.py``. """ from __future__ import annotations +import textwrap from typing import TYPE_CHECKING, Callable if TYPE_CHECKING: @@ -39,6 +41,29 @@ def test_ini_log_file_none( result = pytester.runpytest_subprocess("-s", "--co") result.stdout.fnmatch_lines(["RESOLVED: None"]) + def test_python_false_disables_log_file( + self, + pytester: pytest.Pytester, + write_probe_conftest: Callable[[str], None], + ) -> None: + """`config.option.sift_test_results_log_file = False` disables logging. + + Conftests use this pattern (see lib/sift_client/_tests/util/conftest.py) + to opt their subtree out of log-file mode. Regression test for the + resolver case where Python `False` was previously confused with `None` + and silently kept the temp-file default. + """ + write_probe_conftest( + """ + config.option.sift_test_results_log_file = False + from sift_client.pytest_plugin import _resolve_log_file + print("RESOLVED:", _resolve_log_file(config)) + """, + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest_subprocess("-s", "--co") + result.stdout.fnmatch_lines(["RESOLVED: None"]) + def test_ini_log_file_path( self, pytester: pytest.Pytester, @@ -190,3 +215,180 @@ def test_defaults_when_neither_set( "INI_GIT: True", ] ) + + +# A session-scoped `report_context` stub for the autouse-gate tests. Overrides +# the plugin's real `report_context` so the inner pytest sessions don't try to +# talk to a Sift backend; the gate tests only need to observe whether `step` +# resolves to a real value or to None. +_GATE_INNER_CONFTEST = textwrap.dedent( + """ + from unittest.mock import MagicMock + + import pytest + + pytest_plugins = ["sift_client.pytest_plugin"] + + + @pytest.fixture(scope="session") + def report_context(): + yield MagicMock() + """ +) + + +class TestAutouseGate: + """`sift_include` / `sift_exclude` markers and the `sift_test_results_autouse` ini gate.""" + + def test_default_ini_true_activates(self, pytester: pytest.Pytester) -> None: + """Plugin default (ini absent) keeps the autouse fixtures active.""" + pytester.makeconftest(_GATE_INNER_CONFTEST) + pytester.makepyfile( + """ + def test_inner(step): + assert step is not None + """ + ) + result = pytester.runpytest_subprocess() + result.assert_outcomes(passed=1) + + def test_default_ini_false_skips(self, pytester: pytest.Pytester) -> None: + """`sift_test_results_autouse = false` makes the autouse fixtures no-op by default.""" + pytester.makeconftest(_GATE_INNER_CONFTEST) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_test_results_autouse = false + """ + ) + pytester.makepyfile( + """ + def test_inner(step): + assert step is None + """ + ) + result = pytester.runpytest_subprocess() + result.assert_outcomes(passed=1) + + def test_sift_include_marker_forces_on(self, pytester: pytest.Pytester) -> None: + """`@pytest.mark.sift_include` overrides ini-false to enable the gate.""" + pytester.makeconftest(_GATE_INNER_CONFTEST) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_test_results_autouse = false + """ + ) + pytester.makepyfile( + """ + import pytest + + @pytest.mark.sift_include + def test_inner(step): + assert step is not None + """ + ) + result = pytester.runpytest_subprocess() + result.assert_outcomes(passed=1) + + def test_sift_exclude_marker_forces_off(self, pytester: pytest.Pytester) -> None: + """`@pytest.mark.sift_exclude` overrides ini-true to disable the gate.""" + pytester.makeconftest(_GATE_INNER_CONFTEST) + pytester.makepyfile( + """ + import pytest + + @pytest.mark.sift_exclude + def test_inner(step): + assert step is None + """ + ) + result = pytester.runpytest_subprocess() + result.assert_outcomes(passed=1) + + def test_exclude_beats_include(self, pytester: pytest.Pytester) -> None: + """When both markers are present, `sift_exclude` wins (safer default).""" + pytester.makeconftest(_GATE_INNER_CONFTEST) + pytester.makepyfile( + """ + import pytest + + @pytest.mark.sift_include + @pytest.mark.sift_exclude + def test_inner(step): + assert step is None + """ + ) + result = pytester.runpytest_subprocess() + result.assert_outcomes(passed=1) + + def test_module_pytestmark_inherits(self, pytester: pytest.Pytester) -> None: + """Module-level `pytestmark = pytest.mark.sift_include` covers every test in the module.""" + pytester.makeconftest(_GATE_INNER_CONFTEST) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_test_results_autouse = false + """ + ) + pytester.makepyfile( + """ + import pytest + + pytestmark = pytest.mark.sift_include + + def test_inner_a(step): + assert step is not None + + def test_inner_b(step): + assert step is not None + """ + ) + result = pytester.runpytest_subprocess() + result.assert_outcomes(passed=2) + + def test_bulk_apply_via_conftest_hook(self, pytester: pytest.Pytester) -> None: + """A subtree opts in via `pytest_collection_modifyitems`; siblings stay off. + + Regression test for this repo's wiring pattern: the project default is + autouse-off, the integration subtree's conftest bulk-applies + `sift_include`, and sibling subtrees remain disabled. Verifies the + per-directory mechanism works in a single pytest invocation. + """ + pytester.makeconftest(_GATE_INNER_CONFTEST) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_test_results_autouse = false + """ + ) + included = pytester.mkdir("included_subtree") + (included / "conftest.py").write_text( + textwrap.dedent( + """ + from pathlib import Path + + import pytest + + _HERE = Path(__file__).parent + + + def pytest_collection_modifyitems(config, items): + for item in items: + try: + item.path.relative_to(_HERE) + except ValueError: + continue + item.add_marker(pytest.mark.sift_include) + """ + ) + ) + (included / "test_included.py").write_text( + "def test_included(step):\n assert step is not None\n" + ) + untouched = pytester.mkdir("untouched_subtree") + (untouched / "test_untouched.py").write_text( + "def test_untouched(step):\n assert step is None\n" + ) + result = pytester.runpytest_subprocess() + result.assert_outcomes(passed=2) diff --git a/python/lib/sift_client/_tests/util/conftest.py b/python/lib/sift_client/_tests/util/conftest.py index 086d2f690..2f371e69e 100644 --- a/python/lib/sift_client/_tests/util/conftest.py +++ b/python/lib/sift_client/_tests/util/conftest.py @@ -1,6 +1,35 @@ +from pathlib import Path + import pytest +_HERE = Path(__file__).parent + def pytest_configure(config: pytest.Config) -> None: """Configure the pytest configuration to disable the Sift test results log file.""" config.option.sift_test_results_log_file = False + + +def pytest_collection_modifyitems(config: pytest.Config, items: "list[pytest.Item]") -> None: + """Bulk-apply ``@pytest.mark.sift_include`` to integration tests under util/. + + The project-wide default in ``pyproject.toml`` is ``sift_test_results_autouse + = false`` so unit tests pay nothing for the globally-loaded Sift plugin. + Integration tests in this subtree still need the autouse fixtures, so this + hook flips the gate back on for any test already marked + ``@pytest.mark.integration``. Unit tests in the same directory (e.g. + ``test_cel_utils.py``) are left alone. + + ``pytest_collection_modifyitems`` receives all items in the session (pytest + does not auto-scope it to the conftest's directory), so we filter by path + explicitly. ``Path.relative_to`` is the 3.8-compatible form of the path + containment check (``Path.is_relative_to`` arrived in 3.9). + """ + for item in items: + try: + item.path.relative_to(_HERE) + except ValueError: + continue + if item.get_closest_marker("integration") is None: + continue + item.add_marker(pytest.mark.sift_include) diff --git a/python/lib/sift_client/pytest_plugin.py b/python/lib/sift_client/pytest_plugin.py index db12ce2ee..f2699a954 100644 --- a/python/lib/sift_client/pytest_plugin.py +++ b/python/lib/sift_client/pytest_plugin.py @@ -88,12 +88,24 @@ class _Option: "this ini value.", ) +_AUTOUSE = _Option( + ini_name="sift_test_results_autouse", + ini_help="Default for the Sift autouse fixtures (report_context, step, " + "module_substep). When true (default), tests are included unless marked " + "with @pytest.mark.sift_exclude. When false, tests are skipped unless " + "marked with @pytest.mark.sift_include. Bulk-apply markers in a " + "directory's conftest via `pytest_collection_modifyitems`.", + ini_type="bool", + ini_default=True, +) + _OPTIONS: tuple[_Option, ...] = ( _LOG_FILE, _GIT_METADATA, _CHECK_CONNECTION, _GRPC_URI, _REST_URI, + _AUTOUSE, ) @@ -122,6 +134,49 @@ def pytest_addoption(parser: pytest.Parser) -> None: parser.addini(opt.ini_name, **ini_kwargs) +def pytest_configure(config: pytest.Config) -> None: + """Register the Sift gate markers so they show up in `pytest --markers`.""" + config.addinivalue_line( + "markers", + "sift_include: force the Sift autouse fixtures to activate for this test " + "regardless of the `sift_test_results_autouse` ini default.", + ) + config.addinivalue_line( + "markers", + "sift_exclude: force the Sift autouse fixtures to skip this test " + "regardless of the `sift_test_results_autouse` ini default.", + ) + + +def _sift_enabled_for(node: pytest.Item | pytest.Collector, default: bool) -> bool: + """Resolve the Sift gate for a node: sift_exclude > sift_include > default. + + `get_closest_marker` walks the node hierarchy upward, so markers applied + at any level (function, class, module, package, session) are honored. + """ + if node.get_closest_marker("sift_exclude"): + return False + if node.get_closest_marker("sift_include"): + return True + return default + + +def _module_has_included_tests(request: pytest.FixtureRequest, default: bool) -> bool: + """True when at least one test in `request`'s module is gated on. + + Used by the module-scoped `module_substep` fixture to decide whether to + activate without triggering `report_context` creation for modules where + every test is excluded. + """ + module_path = request.path + for item in request.session.items: + if item.path != module_path: + continue + if _sift_enabled_for(item, default): + return True + return False + + def _option_or_ini(pytestconfig: pytest.Config | None, opt: _Option) -> Any: """Resolve a Sift plugin setting from CLI > ini > None. @@ -141,10 +196,23 @@ def _option_or_ini(pytestconfig: pytest.Config | None, opt: _Option) -> Any: def _resolve_log_file(pytestconfig: pytest.Config | None) -> str | Path | bool | None: - """Determine log_file value from CLI flag or ini key.""" + """Determine log_file value from CLI flag or ini key. + + Three signal types arrive here: + + * ``None`` — unset; nothing was passed on the CLI and the ini key is + absent. Treat as the default "use a temp file." + * Python ``False`` — an explicit disable, typically set in a conftest via + ``config.option.sift_test_results_log_file = False``. Return ``None`` so + the rest of the pipeline knows to skip logging entirely. + * A string (from CLI or ini) — interpret ``"true"`` / ``"1"`` as the temp + file default, ``"false"`` / ``"none"`` as disable, anything else as a + file path. + """ raw = _option_or_ini(pytestconfig, _LOG_FILE) + if raw is False: + return None if not raw: - # None, empty string from ini, or False — treat as "use temp file default". return True lower = str(raw).lower() if lower in ("true", "1"): @@ -264,17 +332,23 @@ def sift_client(pytestconfig: pytest.Config) -> SiftClient: ) -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture(scope="session") def report_context( sift_client: SiftClient, request: pytest.FixtureRequest, pytestconfig: pytest.Config ) -> Generator[ReportContext | None, None, None]: - """Create a report context for the session. + """Lazy session-scoped Sift ReportContext. + + The fixture is no longer autouse; it's instantiated on the first call to + ``request.getfixturevalue("report_context")``, which today happens inside + the gated ``step`` and ``module_substep`` fixtures. If every test in the + session is excluded via the marker gate, this fixture is never resolved + and no ReportContext (and no teardown subprocess) is created. The log file destination is controlled by ``--sift-test-results-log-file``. Defaults to a temp file when not set. - When ``--sift-test-results-check-connection`` is passed, this fixture will no-op - (yield None) if the Sift client has no connection to the server. That mode + When ``--sift-test-results-check-connection`` is passed, this fixture will + yield ``None`` if the Sift client has no connection to the server. That mode requires a ``client_has_connection`` fixture to be available in the session. """ if _check_connection_enabled(pytestconfig) and not _has_sift_connection(request): @@ -302,26 +376,50 @@ def _step_impl( @pytest.fixture(autouse=True) def step( - report_context: ReportContext | None, request: pytest.FixtureRequest, + pytestconfig: pytest.Config, ) -> Generator[NewStep | None, None, None]: - """Create an outer step for the function. No-ops when ``report_context`` is None.""" - if report_context is None: + """Create an outer step for the function when the Sift gate is on. + + Resolves the gate via `_sift_enabled_for(request.node, ini_default)`: + `sift_exclude` marker forces off, `sift_include` forces on, otherwise the + `sift_test_results_autouse` ini default applies. When on, requests the + session `report_context` lazily — the first gated test in the session + triggers its creation, subsequent gated tests reuse it. + """ + default = bool(_option_or_ini(pytestconfig, _AUTOUSE)) + if not _sift_enabled_for(request.node, default): + yield None + return + rc = request.getfixturevalue("report_context") + if rc is None: yield None return - yield from _step_impl(report_context, request) + yield from _step_impl(rc, request) @pytest.fixture(scope="module", autouse=True) def module_substep( - report_context: ReportContext | None, request: pytest.FixtureRequest, + pytestconfig: pytest.Config, ) -> Generator[NewStep | None, None, None]: - """Create a step per module. No-ops when ``report_context`` is None.""" - if report_context is None: + """Create a per-module step when at least one test in the module is gated on. + + Inspects the module's collected items rather than gating on a single marker, + so a module with mixed inclusion/exclusion still produces the module-level + step (individual `step` fixtures then decide per-test). When every test in + the module is excluded, the substep is skipped without requesting + `report_context`. + """ + default = bool(_option_or_ini(pytestconfig, _AUTOUSE)) + if not _module_has_included_tests(request, default): + yield None + return + rc = request.getfixturevalue("report_context") + if rc is None: yield None return - yield from _step_impl(report_context, request) + yield from _step_impl(rc, request) @pytest.fixture(scope="session") diff --git a/python/pyproject.toml b/python/pyproject.toml index dc7169ed3..c54189c17 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -392,6 +392,11 @@ env_files = [ # in non-top-level conftests. Only the plugin test suite uses it; activating it # globally is harmless since the fixture is opt-in. addopts = "-p pytester" +# The Sift plugin is loaded for the whole project via `python/conftest.py`. +# The autouse gate defaults to off here so unit tests don't use it. The +# integration subtree (lib/sift_client/_tests/util/) opts back in via +# `pytest.mark.sift_include` applied in its conftest. +sift_test_results_autouse = false testpaths = [ "lib/sift_py", "lib/sift_client/_tests", From a3a6491eb3450025ed1b3fa31412227deebf1781 Mon Sep 17 00:00:00 2001 From: Alex Luck Date: Mon, 18 May 2026 20:13:53 -0700 Subject: [PATCH 14/14] update docs --- python/docs/examples/pytest_plugin.md | 45 +++++++++++++ .../sift_client/util/test_results/__init__.py | 66 ++++++++++++++----- 2 files changed, 95 insertions(+), 16 deletions(-) diff --git a/python/docs/examples/pytest_plugin.md b/python/docs/examples/pytest_plugin.md index a3bbc7573..3557dd9c7 100644 --- a/python/docs/examples/pytest_plugin.md +++ b/python/docs/examples/pytest_plugin.md @@ -115,6 +115,7 @@ CLI flags, when passed, override the ini values. | `sift_test_results_log_file` | string (`true` / `false` / `none` / path) | `--sift-test-results-log-file=` | | `sift_test_results_git_metadata` | bool (default `true`) | `--no-sift-test-results-git-metadata` (sets to `false`) | | `sift_test_results_check_connection` | bool (default `false`) | `--sift-test-results-check-connection` | +| `sift_test_results_autouse` | bool (default `true`) | _(no CLI flag; controls the marker gate below)_ | The default `sift_client` fixture reads its two URIs from environment first and falls back to ini keys when the env vars are unset. `SIFT_API_KEY` is @@ -198,6 +199,50 @@ metadata), call `report_context.report.update({...})` from any test or fixture. See [Linking a Run](#linking-a-run-to-the-report) for the same pattern applied to `run_id`. +## Controlling which tests produce reports + +By default every test in the session produces a Sift step. Two markers +and one ini key let you narrow that to a specific set of tests, which is +useful when a repo holds tests that you don't want included in the Sift test report. + +| Setting | Effect | +|---------------------------------------------------------|----------------------------------------------------------------------------------------------| +| `sift_test_results_autouse = false` in `pyproject.toml` | Flip the project-wide default off. Tests no longer produce steps unless explicitly opted in. | +| `@pytest.mark.sift_include` on a test, class, or module | Force reporting on for that scope, regardless of the project default. | +| `@pytest.mark.sift_exclude` on a test, class, or module | Force reporting off for that scope, regardless of the project default. | + +Closest marker determines setting. `sift_exclude` beats `sift_include` when both apply. +`pytestmark` at the class or module level inherits to every test in scope. + +### Bulk-applying a marker to a directory + +To opt an entire directory in (or out) without editing each file, hook +`pytest_collection_modifyitems` in the directory's `conftest.py`: + +```python title="tests/example/conftest.py" +from pathlib import Path + +import pytest + +_HERE = Path(__file__).parent + + +def pytest_collection_modifyitems(config, items): + for item in items: + try: + item.path.relative_to(_HERE) + except ValueError: + continue + item.add_marker(pytest.mark.sift_include) +``` + +This applies `sift_include` to every test collected under `tests/example/`. +Combine with `sift_test_results_autouse = false` in `pyproject.toml` for +opting in to specific directories. + +`pytest_collection_modifyitems` receives every item in the session, not just +this directory's, so the `relative_to` filter is what scopes the marker. + ## Basic usage With the conftest in place, the simplest test needs nothing extra. The `step` diff --git a/python/lib/sift_client/util/test_results/__init__.py b/python/lib/sift_client/util/test_results/__init__.py index a80396ded..ea213056e 100644 --- a/python/lib/sift_client/util/test_results/__init__.py +++ b/python/lib/sift_client/util/test_results/__init__.py @@ -59,24 +59,55 @@ def main(self): pytest_plugins = ["sift_client.pytest_plugin"] ``` -The plugin ships an autouse session-scoped `report_context` fixture (one -`TestReport` per session), an autouse function-scoped `step` fixture, and an -optional `module_substep` fixture. It also registers a default `sift_client` -fixture that reads `SIFT_API_KEY`, `SIFT_GRPC_URI`, and `SIFT_REST_URI` from -the environment. Override it by defining your own `sift_client` fixture in -your conftest. +By default, every test in the session produces a Sift report: one +`TestReport` per session, one step per test function (`step`), and one +parent step per test file (`module_substep`). The plugin also registers a +default `sift_client` fixture that reads `SIFT_API_KEY`, `SIFT_GRPC_URI`, +and `SIFT_REST_URI` from the environment. Override it by defining your own +`sift_client` fixture in your conftest. + +Note: FedRAMP users: results are buffered to a temp file and uploaded by a +subprocess at session end (no API calls during the run). Disable the buffer +entirely with `--sift-test-results-log-file=false` for inline uploads. + +### Controlling which tests produce reports + +The autouse fixtures fire for every test by default. To narrow that: + +- Set `sift_test_results_autouse = false` in `pyproject.toml` to flip the + project default off, then opt tests back in below. +- `@pytest.mark.sift_include` forces reporting on for a test, class, or + module. `@pytest.mark.sift_exclude` forces it off. Closest marker wins. + `sift_exclude` beats `sift_include` when both apply. +- `pytestmark` at the class or module level inherits to every test in scope. +- For a whole directory, apply the marker in bulk from that directory's + `conftest.py`: -Note: FedRAMP users: `report_context` will log test results to a temp file to -avoid API calls during test execution. If this is a shared environment, you -can disable logging by passing `--sift-test-results-log-file=false`. +```python +# tests/integration/conftest.py +from pathlib import Path + +import pytest + +_HERE = Path(__file__).parent + + +def pytest_collection_modifyitems(config, items): + for item in items: + try: + item.path.relative_to(_HERE) + except ValueError: + continue + item.add_marker(pytest.mark.sift_include) +``` #### Configuration CLI options registered by the plugin: - `--sift-test-results-log-file`: Path to write the JSONL log file. `true` - (default) auto-creates a temp file; `false`/`none` disables logging; a path - writes to that location. + (default) auto-creates a temp file. `false` or `none` disables logging. + Any other value is treated as a file path. - `--no-sift-test-results-git-metadata`: Exclude git metadata (repo, branch, commit) from the test report. Included by default. - `--sift-test-results-check-connection`: Make `report_context`, `step`, and @@ -85,14 +116,17 @@ def main(self): Each option has a matching ini key for per-project configuration under ``[tool.pytest.ini_options]`` in ``pyproject.toml`` (or ``[pytest]`` in -``pytest.ini``). CLI flags override ini values. The default ``sift_client`` -fixture also reads ``sift_grpc_uri`` and ``sift_rest_uri`` as fallbacks when -the corresponding env vars are unset (env vars win when both are set). -``SIFT_API_KEY`` is env-only — load it from a ``.env`` file via the -``pytest-dotenv`` plugin or inject it via your CI secret manager. +``pytest.ini``). CLI flags override ini values. The +``sift_test_results_autouse`` ini key (bool, default ``true``) sets the +project-wide default for the gate described above. The default +``sift_client`` fixture reads ``sift_grpc_uri`` and ``sift_rest_uri`` as +fallbacks when the corresponding env vars are unset (env vars win when +both are set). ``SIFT_API_KEY`` is env-only. Load it from a ``.env`` file +via the ``pytest-dotenv`` plugin or inject it via your CI secret manager. ```toml [tool.pytest.ini_options] +sift_test_results_autouse = false sift_test_results_log_file = "false" sift_test_results_check_connection = true sift_test_results_git_metadata = false