diff --git a/python/docs/examples/pytest_plugin.md b/python/docs/examples/pytest_plugin.md index 3557dd9c7..2ac298256 100644 --- a/python/docs/examples/pytest_plugin.md +++ b/python/docs/examples/pytest_plugin.md @@ -88,21 +88,22 @@ def sift_client() -> SiftClient: | `report_context` | fixture (autouse) | session | The `ReportContext` backing the run's `TestReport`. Use it to attach metadata or open ad-hoc steps. | | `step` | fixture (autouse) | function | A `NewStep` created for the current test function. Exposes `measure*`, `substep`, `report_outcome`, and `current_step`. | | `module_substep` | fixture (autouse) | module | One step per test file with each function nested as a substep. | -| `client_has_connection` | fixture | session | Calls `sift_client.ping.ping()`; consulted only when `--sift-test-results-check-connection` is set. | +| `client_has_connection` | fixture | session | Calls `sift_client.ping.ping()`; consulted by `report_context` at session start in online mode (the default). Override to skip the ping or use a different reachability signal. | ### CLI options | Flag | Default | Effect | |---|---|---| -| `--sift-test-results-log-file=` | temp file | Where the JSONL log of create/update calls goes. With a log file set, the plugin spawns an `import-test-result-log --incremental` worker that polls the file and replays entries against Sift while the run is in flight. Pass `false` to disable the file entirely; create/update calls then go straight to the API synchronously during tests. | -| `--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. | +| `--sift-offline` | off (online) | Skip the session-start ping and don't contact Sift. All create/update calls go to the JSONL log file for later replay via `import-test-result-log`. Missing `SIFT_*` env vars are tolerated; placeholders are filled. | +| `--sift-disabled` | off | Skip Sift entirely. Nothing contacts the API and no log file is written; `step.measure(...)` still evaluates bounds and returns a real pass/fail boolean. Also honored via `SIFT_DISABLED=1`. Supersedes every other flag (disabled wins over offline). | +| `--sift-log-file=` | temp file | Where the JSONL log of create/update calls goes. With a log file set, the plugin spawns an `import-test-result-log --incremental` worker that polls the file and replays entries against Sift while the run is in flight. Pass `false` to disable the file entirely; create/update calls then go straight to the API synchronously during tests. Incompatible with `--sift-offline` since offline mode needs the log file as its sole sink. | +| `--no-sift-git-metadata` | git metadata on | Skip capturing git repo/branch/commit on the report's metadata. | These can be passed permanently via `addopts`: ```ini title="pytest.ini" [pytest] -addopts = --sift-test-results-check-connection +addopts = --sift-offline ``` Or set the matching ini key directly (recommended for stable per-project @@ -112,10 +113,11 @@ 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` | -| `sift_test_results_autouse` | bool (default `true`) | _(no CLI flag; controls the marker gate below)_ | +| `sift_log_file` | string (`true` / `false` / `none` / path) | `--sift-log-file=` | +| `sift_git_metadata` | bool (default `true`) | `--no-sift-git-metadata` (sets to `false`) | +| `sift_offline` | bool (default `false`) | `--sift-offline` | +| `sift_disabled` | bool (default `false`) | `--sift-disabled` (also honors `SIFT_DISABLED` env var) | +| `sift_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 @@ -133,18 +135,16 @@ flags for credentials. ```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_offline = true +sift_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_offline = true +sift_git_metadata = false sift_grpc_uri = your-org.sift.example:443 sift_rest_uri = https://your-org.sift.example ``` @@ -171,7 +171,7 @@ into `os.environ` before tests run. glue is needed. !!! warning "FedRAMP / shared environments" - Pass `--sift-test-results-log-file=false` (or set the ini key to `"false"`) + Pass `--sift-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. @@ -184,7 +184,7 @@ Every report the plugin creates includes: - `system_operator`: `getpass.getuser()`. - `start_time` / `end_time`: set on session enter/exit. - `status`: starts at `IN_PROGRESS`, finalized to `PASSED` or `FAILED` on session exit (failure if any step failed or an exception escaped the session). -- `metadata.git_repo`, `metadata.git_branch`, `metadata.git_commit`: captured via `git remote get-url origin` / `git rev-parse --abbrev-ref HEAD` / `git describe --always --dirty --exclude '*'`. Suppressed by `--no-sift-test-results-git-metadata` or when not in a git repo. +- `metadata.git_repo`, `metadata.git_branch`, `metadata.git_commit`: captured via `git remote get-url origin` / `git rev-parse --abbrev-ref HEAD` / `git describe --always --dirty --exclude '*'`. Suppressed by `--no-sift-git-metadata` or when not in a git repo. Example invocations: @@ -207,7 +207,7 @@ useful when a repo holds tests that you don't want included in the Sift test rep | 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. | +| `sift_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. | @@ -237,7 +237,7 @@ def pytest_collection_modifyitems(config, items): ``` This applies `sift_include` to every test collected under `tests/example/`. -Combine with `sift_test_results_autouse = false` in `pyproject.toml` for +Combine with `sift_autouse = false` in `pyproject.toml` for opting in to specific directories. `pytest_collection_modifyitems` receives every item in the session, not just @@ -657,151 +657,129 @@ The `unit` argument is a free-form string label (e.g. `"V"`, `"C"`, `"psi"`). pytest # Pin the log file so you can replay it later if the import worker dies -pytest --sift-test-results-log-file=./sift-results.jsonl +pytest --sift-log-file=./sift-results.jsonl ``` -See [Running offline](#running-offline) for the same suite running with or -without a reachable Sift server. +See [Running modes](#running-modes) for the offline and disabled flags +that let the same suite run without (or without contacting) Sift. -## Running offline +## Running modes -The plugin supports two offline workflows, depending on whether you want a -Sift report at all when the test environment can't reach Sift. The first -turns the plugin into a no-op when the server is unreachable. The second -keeps the plugin running normally and writes every create/update to a local -JSONL file that you upload from a connected machine afterward. +The plugin runs in one of three modes, picked at invocation: -| Pattern | Flag | Runtime behavior | Follow-up | -|---|---|---|---| -| Skip when offline | `--sift-test-results-check-connection` | Fixtures yield `None`, no log file, no report. Pytest still reports pass/fail. | None. | -| Capture locally, upload later | `--sift-test-results-log-file=` | Plugin writes every create/update to the JSONL file. | `import-test-result-log ` from a connected machine. | +| Mode | Flag | Network | Log file | `step.measure(...)` | When to use | +|---|---|---|---|---|---| +| Online (default) | _(none)_ | yes (pings at session start, aborts if it fails) | optional write-through backup | real measurement against Sift | CI with Sift credentials, local dev hitting your tenant | +| Offline | `--sift-offline` | none | required (the sole sink) | real measurement queued to log | field tests, air-gapped labs, CI without network | +| Disabled | `--sift-disabled` | none | none | bounds eval; returns a real bool | local dev or CI that doesn't have (or want) Sift | -Pattern 1 suits laptop dev and CI without Sift secrets. Pattern 2 suits -field tests, vehicles on remote sites, and air-gapped labs. +Pass both flags? Disabled wins. It's the "skip Sift entirely" hammer and +supersedes everything else. -### Pattern 1: skip when offline +### Online mode (default) -`--sift-test-results-check-connection` makes the plugin ping Sift once at -session start through the `client_has_connection` fixture (which by default -calls `sift_client.ping.ping()`). On a failed ping, `report_context`, -`step`, and `module_substep` yield `None` for the rest of the session. -Pytest still runs the tests and still reports pass/fail. +`report_context` resolves `client_has_connection` at session start. The +default implementation calls `sift_client.ping.ping()`. A failed ping +aborts the whole session with `pytest.UsageError` and points at +`--sift-offline` and `--sift-disabled` as escape hatches. -```bash -pytest --sift-test-results-check-connection -``` +This is loud on purpose. A CI run that silently no-ops on a flaky network +won't get noticed until somebody goes looking for the report, which is +usually weeks later, which is usually too late. -```ini title="pytest.ini" -[pytest] -addopts = --sift-test-results-check-connection -``` +With the default `--sift-log-file` setting on, create/update calls are +written to a JSONL log file during the run and an +`import-test-result-log --incremental` worker replays them against Sift +in the background. If the worker crashes mid-session (connection failure, +API error) or is still draining its backlog at session end, the failure +is logged at session end with a `replay-test-result-log` command for +manual recovery — test outcomes are unaffected and the local log file is +preserved. Pass `--sift-log-file=false` to make every create/update +synchronous against the API instead. -#### Handling `None` in tests +#### Overriding the connection check -Calls on `step` raise `AttributeError` when it's `None`, so tests that take -`step` as a parameter need a guard. The cleanest fix is to shadow the -plugin's `step` fixture in your conftest and turn the `None` case into an -automatic skip. +Override `client_has_connection` when ping isn't the right signal, for +example a token cache that's only warm when authenticated: ```python title="conftest.py" -import pytest +from pathlib import Path -pytest_plugins = ["sift_client.pytest_plugin"] +import pytest -@pytest.fixture(autouse=True) -def step(step): - if step is None: - pytest.skip("Sift unavailable") - yield step +@pytest.fixture(scope="session") +def client_has_connection(sift_client) -> bool: + return Path("~/.sift-token-cache").expanduser().is_file() ``` -The `step` parameter on the override resolves to the plugin's fixture, not -to the override itself. `autouse=True` is required so the skip applies to -tests that don't request `step` directly. The same shadowing trick works -for `module_substep` and `report_context`. +The override is ignored under `--sift-offline` and `--sift-disabled`. -For one-off tests that don't share a conftest, an inline guard works just -as well: +### Offline mode (`--sift-offline`) -```python -def test_battery_voltage(step): - if step is None: - pytest.skip("Sift unavailable") - step.measure(name="battery_voltage", value=4.97, bounds={"min": 4.8, "max": 5.2}) -``` +Same fixtures, same `step.measure(...)` semantics as online. The +difference is where the writes go: every create/update lands in a JSONL +log file instead of hitting the Sift API. The session-start ping is +skipped, missing `SIFT_*` env vars are tolerated (placeholders are +filled), and the replay worker (`import-test-result-log --incremental`) +does not get spawned at session end. -If you'd rather have tests pass through silently than skip them, wrap the -calls in a helper that no-ops on `None`: - -```python -def safe_measure(step, **kwargs): - if step is None: - return True - return step.measure(**kwargs) +```bash +pytest --sift-offline --sift-log-file=./run.jsonl ``` -#### Overriding the connection check +Once you have connectivity, replay it: -The default `client_has_connection` fixture calls `sift_client.ping.ping()`. -Override it in your conftest if pinging is the wrong signal for your -environment, for example a token cache that's only warm when authenticated: +```bash +import-test-result-log ./run.jsonl +``` -```python title="conftest.py" -from pathlib import Path +That replay creates the report, steps, and measurements against Sift. +See [Replaying a saved log file](#replaying-a-saved-log-file) for cleanup +and the incremental flag. -import pytest +`--sift-log-file=none` is rejected when offline is set. The +log file is the only sink in offline mode, so without it the results are +gone. +!!! warning "Pin the log path" + Without `--sift-log-file=`, offline mode writes to + a `tempfile.NamedTemporaryFile` and only surfaces the path via a + `logger.info` line. Pin a known path when you intend to replay later. -@pytest.fixture(scope="session") -def client_has_connection(sift_client) -> bool: - return Path("~/.sift-token-cache").expanduser().is_file() -``` +### Disabled mode (`--sift-disabled`) -The plugin only consults this fixture when `--sift-test-results-check-connection` -is set, so an unused override has no effect on a normal run. +The plugin stays loaded with the same fixtures and markers as the other +modes. Nothing contacts Sift, no log file is written, and no `SIFT_*` +env vars are required. `step.measure(...)`, `step.measure_avg(...)`, +`step.measure_all(...)`, `step.substep(...)`, and +`report_context.report.update({...})` all behave normally — bounds +evaluate and you get a real pass/fail boolean back. -### Pattern 2: capture locally, upload later +Entities returned in disabled mode report `is_simulated == True` (on +`TestReport`, `TestStep`, `TestMeasurement`, and `ReportContext`) so +consumers and tests can branch on provenance. Offline-mode entities +also report `is_simulated == True`. -This pattern keeps the plugin running normally even when Sift is -unreachable. The plugin writes to the log file, the worker dies on connect, -and the file is left on disk for you to upload later. Pin the log file path -so you can find it afterward, and don't pass -`--sift-test-results-check-connection`, which would suppress the logging -this pattern relies on. +How to turn it on, in the order most projects pick: ```bash -pytest --sift-test-results-log-file=./run.jsonl -``` - -What happens during the run: +# In an .envrc, devcontainer, or CI job config +export SIFT_DISABLED=1 -- Every report, step, and measurement create/update is written to - `run.jsonl`. The plugin doesn't contact the Sift API for any of these - calls; they return simulated responses keyed by UUIDs that the replay - later maps to real IDs. -- The `import-test-result-log --incremental` worker subprocess starts and - exits early when it can't reach Sift. The session does not fail when the - worker exits before the run ends. -- Tests run against a real `step` fixture, so `step.measure(...)`, - substeps, parametrize, fixtures, and `module_substep` behave exactly as - they do online. No conftest changes are needed. +# Per-invocation kill-switch +pytest --sift-disabled -Once you have connectivity, replay the file: - -```bash -import-test-result-log ./run.jsonl +# Per-project default (uncommon; online is usually the right default) +# pyproject.toml: +# [tool.pytest.ini_options] +# sift_disabled = true ``` -The replay creates the report, steps, and measurements against Sift in one -batch. See [Replaying a saved log file](#replaying-a-saved-log-file) for -details on cleanup and the incremental flag. - -!!! warning "Pin the log path for Pattern 2" - Without `--sift-test-results-log-file=`, the plugin writes to a - `tempfile.NamedTemporaryFile` and only surfaces the path via a - `logger.info` line. Always pin a known path when you intend to replay - the file later. +Good fit for local dev without Sift credentials. Also for library +consumers who don't have a Sift tenant. Also useful in CI for runs that +shouldn't add noise to the report stream, like a PR job re-running the +same suite five times in a row. ## Replaying a saved log file diff --git a/python/lib/sift_client/_internal/low_level_wrappers/test_results.py b/python/lib/sift_client/_internal/low_level_wrappers/test_results.py index d15f86c48..ff0c2b515 100644 --- a/python/lib/sift_client/_internal/low_level_wrappers/test_results.py +++ b/python/lib/sift_client/_internal/low_level_wrappers/test_results.py @@ -3,7 +3,7 @@ import logging import uuid from pathlib import Path -from typing import TYPE_CHECKING, Any, cast +from typing import TYPE_CHECKING, Any, TypeVar, cast from google.protobuf import json_format from sift.test_reports.v1.test_reports_pb2 import ( @@ -68,6 +68,9 @@ logger = logging.getLogger(__name__) +_EntityT = TypeVar("_EntityT", TestReport, TestStep, TestMeasurement) + + class TestResultsLowLevelClient(LowLevelClientBase, WithGrpcClient): """Low-level client for the TestResultsAPI. @@ -82,6 +85,16 @@ def __init__(self, grpc_client: GrpcClient): """ super().__init__(grpc_client) + @staticmethod + def _mark_simulated(instance: _EntityT) -> _EntityT: + """Stamp an entity as having been produced by the simulate path. + + Mirrors the ``__dict__`` write used by ``BaseType._apply_client_to_instance`` + to bypass pydantic's frozen-model guard. + """ + instance.__dict__["_simulated"] = True + return instance + @staticmethod def simulate_create_test_report_response( request: CreateTestReportRequest, @@ -387,7 +400,7 @@ async def create_test_report( request, response_id=simulated_proto.test_report_id, ) - return TestReport._from_proto(simulated_proto) + return self._mark_simulated(TestReport._from_proto(simulated_proto)) response = await self._grpc_client.get_stub(TestReportServiceStub).CreateTestReport(request) grpc_test_report = cast("CreateTestReportResponse", response).test_report @@ -505,7 +518,9 @@ async def update_test_report( if log_file is not None or simulate: if log_file is not None: log_request_to_file(log_file, "UpdateTestReport", request) - return self.simulate_update_test_report_response(request, existing=existing) + return self._mark_simulated( + self.simulate_update_test_report_response(request, existing=existing) + ) response = await self._grpc_client.get_stub(TestReportServiceStub).UpdateTestReport(request) grpc_test_report = cast("UpdateTestReportResponse", response).test_report @@ -560,7 +575,7 @@ async def create_test_step( request, response_id=simulated_proto.test_step_id, ) - return TestStep._from_proto(simulated_proto) + return self._mark_simulated(TestStep._from_proto(simulated_proto)) response = await self._grpc_client.get_stub(TestReportServiceStub).CreateTestStep(request) grpc_test_step = cast("CreateTestStepResponse", response).test_step @@ -661,7 +676,9 @@ async def update_test_step( if log_file is not None or simulate: if log_file is not None: log_request_to_file(log_file, "UpdateTestStep", request) - return self.simulate_update_test_step_response(request, existing=existing) + return self._mark_simulated( + self.simulate_update_test_step_response(request, existing=existing) + ) response = await self._grpc_client.get_stub(TestReportServiceStub).UpdateTestStep(request) grpc_test_step = cast("UpdateTestStepResponse", response).test_step @@ -716,7 +733,7 @@ async def create_test_measurement( request, response_id=simulated_proto.measurement_id, ) - return TestMeasurement._from_proto(simulated_proto) + return self._mark_simulated(TestMeasurement._from_proto(simulated_proto)) response = await self._grpc_client.get_stub(TestReportServiceStub).CreateTestMeasurement( request @@ -861,7 +878,9 @@ async def update_test_measurement( if log_file is not None or simulate: if log_file is not None: log_request_to_file(log_file, "UpdateTestMeasurement", request) - return self.simulate_update_test_measurement_response(request, existing=existing) + return self._mark_simulated( + self.simulate_update_test_measurement_response(request, existing=existing) + ) response = await self._grpc_client.get_stub(TestReportServiceStub).UpdateTestMeasurement( request diff --git a/python/lib/sift_client/_tests/conftest.py b/python/lib/sift_client/_tests/conftest.py index 79b079d39..0b939ae39 100644 --- a/python/lib/sift_client/_tests/conftest.py +++ b/python/lib/sift_client/_tests/conftest.py @@ -79,5 +79,13 @@ def ci_pytest_tag(sift_client): 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 + """Pick a Sift plugin mode based on whether integration tests are running. + + Integration runs (``-m integration``) stay online with the default + log-file pipeline enabled so CI exercises the JSONL write + import + worker replay path that production users hit. Every other run defaults + to ``--sift-disabled`` so unit tests don't need credentials. + """ + is_integration_run = "integration" in (config.option.markexpr or "") + if not is_integration_run: + config.option.sift_disabled = True diff --git a/python/lib/sift_client/_tests/pytest_plugin/conftest.py b/python/lib/sift_client/_tests/pytest_plugin/conftest.py index 1fbd61e46..783a12bf4 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/conftest.py +++ b/python/lib/sift_client/_tests/pytest_plugin/conftest.py @@ -25,6 +25,15 @@ import pytest +_SIFT_ENV_VARS = ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI", "SIFT_DISABLED") + + +@pytest.fixture +def clear_sift_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Unset all ``SIFT_*`` environment variables for the duration of the test.""" + for name in _SIFT_ENV_VARS: + monkeypatch.delenv(name, raising=False) + @pytest.fixture def write_plugin_conftest(pytester: pytest.Pytester) -> Callable[[], None]: 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 9b9be2d63..4efb9f554 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py +++ b/python/lib/sift_client/_tests/pytest_plugin/test_configuration.py @@ -34,7 +34,7 @@ def test_ini_log_file_none( pytester.makepyprojecttoml( """ [tool.pytest.ini_options] - sift_test_results_log_file = "none" + sift_log_file = "none" """ ) pytester.makepyfile("def test_noop(): pass") @@ -46,7 +46,7 @@ def test_python_false_disables_log_file( pytester: pytest.Pytester, write_probe_conftest: Callable[[str], None], ) -> None: - """`config.option.sift_test_results_log_file = False` disables logging. + """`config.option.sift_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 @@ -55,7 +55,7 @@ def test_python_false_disables_log_file( """ write_probe_conftest( """ - config.option.sift_test_results_log_file = False + config.option.sift_log_file = False from sift_client.pytest_plugin import _resolve_log_file print("RESOLVED:", _resolve_log_file(config)) """, @@ -80,33 +80,54 @@ def test_ini_log_file_path( pytester.makepyprojecttoml( f""" [tool.pytest.ini_options] - sift_test_results_log_file = "{log_path}" + sift_log_file = "{log_path}" """ ) pytester.makepyfile("def test_noop(): pass") result = pytester.runpytest_subprocess("-s", "--co") result.stdout.fnmatch_lines([f"RESOLVED: {log_path}"]) - def test_ini_check_connection_true( + def test_ini_offline_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)) + from sift_client.pytest_plugin import _is_offline + print("OFFLINE:", _is_offline(config)) """, ) pytester.makepyprojecttoml( """ [tool.pytest.ini_options] - sift_test_results_check_connection = true + sift_offline = true """ ) pytester.makepyfile("def test_noop(): pass") result = pytester.runpytest_subprocess("-s", "--co") - result.stdout.fnmatch_lines(["CHECK: True"]) + result.stdout.fnmatch_lines(["OFFLINE: True"]) + + def test_ini_disabled_true( + self, + pytester: pytest.Pytester, + write_probe_conftest: Callable[[str], None], + ) -> None: + write_probe_conftest( + """ + from sift_client.pytest_plugin import _is_disabled + print("DISABLED:", _is_disabled(config)) + """, + ) + pytester.makepyprojecttoml( + """ + [tool.pytest.ini_options] + sift_disabled = true + """ + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest_subprocess("-s", "--co") + result.stdout.fnmatch_lines(["DISABLED: True"]) def test_ini_git_metadata_false( self, @@ -115,13 +136,13 @@ def test_ini_git_metadata_false( ) -> None: write_probe_conftest( """ - print("INI_GIT:", config.getini("sift_test_results_git_metadata")) + print("INI_GIT:", config.getini("sift_git_metadata")) """, ) pytester.makepyprojecttoml( """ [tool.pytest.ini_options] - sift_test_results_git_metadata = false + sift_git_metadata = false """ ) pytester.makepyfile("def test_noop(): pass") @@ -145,37 +166,51 @@ def test_cli_overrides_ini( pytester.makepyprojecttoml( """ [tool.pytest.ini_options] - sift_test_results_log_file = "none" + sift_log_file = "none" """ ) pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest_subprocess( - "-s", "--co", f"--sift-test-results-log-file={cli_path}" - ) + result = pytester.runpytest_subprocess("-s", "--co", f"--sift-log-file={cli_path}") result.stdout.fnmatch_lines([f"RESOLVED: {cli_path}"]) - def test_cli_check_connection_flag( + def test_cli_offline_flag( + self, + pytester: pytest.Pytester, + write_probe_conftest: Callable[[str], None], + ) -> None: + """The ``--sift-offline`` CLI flag flips the resolver to True.""" + write_probe_conftest( + """ + from sift_client.pytest_plugin import _is_offline + print("OFFLINE:", _is_offline(config)) + """, + ) + pytester.makepyfile("def test_noop(): pass") + result = pytester.runpytest_subprocess("-s", "--co", "--sift-offline") + result.stdout.fnmatch_lines(["OFFLINE: True"]) + + def test_cli_disabled_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.""" + """The ``--sift-disabled`` 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)) + from sift_client.pytest_plugin import _is_disabled + print("DISABLED:", _is_disabled(config)) """, ) pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest_subprocess("-s", "--co", "--sift-test-results-check-connection") - result.stdout.fnmatch_lines(["CHECK: True"]) + result = pytester.runpytest_subprocess("-s", "--co", "--sift-disabled") + result.stdout.fnmatch_lines(["DISABLED: 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. + """The ``--no-sift-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 @@ -183,11 +218,11 @@ def test_cli_no_git_metadata_flag( """ write_probe_conftest( """ - print("CLI_GIT:", config.getoption("sift_test_results_git_metadata")) + print("CLI_GIT:", config.getoption("sift_git_metadata")) """, ) pytester.makepyfile("def test_noop(): pass") - result = pytester.runpytest_subprocess("-s", "--co", "--no-sift-test-results-git-metadata") + result = pytester.runpytest_subprocess("-s", "--co", "--no-sift-git-metadata") result.stdout.fnmatch_lines(["CLI_GIT: False"]) def test_defaults_when_neither_set( @@ -198,12 +233,14 @@ def test_defaults_when_neither_set( write_probe_conftest( """ from sift_client.pytest_plugin import ( - _check_connection_enabled, + _is_disabled, + _is_offline, _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")) + print("OFFLINE:", _is_offline(config)) + print("DISABLED:", _is_disabled(config)) + print("INI_GIT:", config.getini("sift_git_metadata")) """, ) pytester.makepyfile("def test_noop(): pass") @@ -211,7 +248,8 @@ def test_defaults_when_neither_set( result.stdout.fnmatch_lines( [ "RESOLVED: True", - "CHECK: False", + "OFFLINE: False", + "DISABLED: False", "INI_GIT: True", ] ) @@ -238,7 +276,7 @@ def report_context(): class TestAutouseGate: - """`sift_include` / `sift_exclude` markers and the `sift_test_results_autouse` ini gate.""" + """`sift_include` / `sift_exclude` markers and the `sift_autouse` ini gate.""" def test_default_ini_true_activates(self, pytester: pytest.Pytester) -> None: """Plugin default (ini absent) keeps the autouse fixtures active.""" @@ -253,12 +291,12 @@ def test_inner(step): 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.""" + """`sift_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 + sift_autouse = false """ ) pytester.makepyfile( @@ -276,7 +314,7 @@ def test_sift_include_marker_forces_on(self, pytester: pytest.Pytester) -> None: pytester.makepyprojecttoml( """ [tool.pytest.ini_options] - sift_test_results_autouse = false + sift_autouse = false """ ) pytester.makepyfile( @@ -328,7 +366,7 @@ def test_module_pytestmark_inherits(self, pytester: pytest.Pytester) -> None: pytester.makepyprojecttoml( """ [tool.pytest.ini_options] - sift_test_results_autouse = false + sift_autouse = false """ ) pytester.makepyfile( @@ -359,7 +397,7 @@ def test_bulk_apply_via_conftest_hook(self, pytester: pytest.Pytester) -> None: pytester.makepyprojecttoml( """ [tool.pytest.ini_options] - sift_test_results_autouse = false + sift_autouse = false """ ) included = pytester.mkdir("included_subtree") 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 9ee628e69..3f6d22a6e 100644 --- a/python/lib/sift_client/_tests/pytest_plugin/test_credentials.py +++ b/python/lib/sift_client/_tests/pytest_plugin/test_credentials.py @@ -31,8 +31,8 @@ def test_uris_from_ini( [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" + sift_offline = true + """ ) pytester.makepyfile( @@ -62,8 +62,8 @@ def test_env_var_overrides_ini_uri( [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" + sift_offline = true + """ ) pytester.makepyfile( diff --git a/python/lib/sift_client/_tests/pytest_plugin/test_disabled.py b/python/lib/sift_client/_tests/pytest_plugin/test_disabled.py new file mode 100644 index 000000000..cba4bc1ee --- /dev/null +++ b/python/lib/sift_client/_tests/pytest_plugin/test_disabled.py @@ -0,0 +1,183 @@ +"""Tests for ``--sift-disabled`` mode. + +Disabled mode skips Sift entirely. Autouse fixtures yield stub objects so +test code that calls ``step.measure(...)`` keeps working without any Sift +configuration; ``measure*`` evaluates bounds locally and returns the real +pass/fail boolean. Nothing reaches Sift and no log file is written. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Callable + +if TYPE_CHECKING: + from pathlib import Path + + import pytest + + +class TestDisabledMode: + def test_in_bounds_passes_out_of_bounds_fails( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + """Stub measure* evaluates bounds locally; pass/fail matches the real plugin.""" + write_plugin_conftest() + pytester.makepyfile( + """ + def test_passes_in_bounds(step): + assert step.measure(name="v", value=5.0, bounds={"min": 4.8, "max": 5.2}) + + def test_fails_out_of_bounds(step): + assert step.measure(name="v", value=99.0, bounds={"max": 5.2}) is False + + def test_substep_and_report_outcome(step): + with step.substep(name="inner") as inner: + assert inner.report_outcome(name="ok", result=True) is True + + def test_string_bounds(step): + assert step.measure(name="fw", value="1.0", bounds="1.0") is True + assert step.measure(name="fw", value="1.0", bounds="2.0") is False + + def test_measure_avg(step): + assert step.measure_avg( + name="bus", values=[4.97, 5.01, 5.03], bounds={"min": 4.9, "max": 5.1} + ) is True + + def test_measure_all_outlier(step): + assert step.measure_all( + name="p", values=[10.1, 10.2, 99.9], bounds={"max": 11.0} + ) is False + """ + ) + result = pytester.runpytest_subprocess("--sift-disabled") + result.assert_outcomes(passed=6) + + def test_disabled_does_not_require_credentials( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + """Disabled mode never reads SIFT_* env vars; runs cleanly without them.""" + write_plugin_conftest() + pytester.makepyfile("def test_runs(step): step.measure(name='v', value=1.0)") + result = pytester.runpytest_subprocess("--sift-disabled") + result.assert_outcomes(passed=1) + + def test_disabled_via_env_var( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """``SIFT_DISABLED=1`` triggers disabled mode without the CLI flag.""" + write_plugin_conftest() + pytester.makepyfile("def test_runs(step): step.measure(name='v', value=1.0)") + monkeypatch.setenv("SIFT_DISABLED", "1") + result = pytester.runpytest_subprocess() + result.assert_outcomes(passed=1) + + def test_disabled_supersedes_offline( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + """``--sift-disabled`` wins when combined with ``--sift-offline``. + + Disabled is the "skip Sift entirely" hammer; passing it alongside + offline shouldn't error. The session runs without credentials, without + a log file, and without the offline-mode replay machinery. + """ + write_plugin_conftest() + pytester.makepyfile( + """ + def test_runs(step): + assert step.measure(name="v", value=5.0, bounds={"max": 10.0}) is True + """ + ) + result = pytester.runpytest_subprocess("--sift-disabled", "--sift-offline") + result.assert_outcomes(passed=1) + + def test_disabled_yields_stub_fixtures( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + """`report_context` / `step` / `module_substep` are real instances backed by a simulate client.""" + write_plugin_conftest() + pytester.makepyfile( + """ + from sift_client.util.test_results import ReportContext + from sift_client.util.test_results.context_manager import NewStep + + def test_types(step, report_context, module_substep): + assert isinstance(report_context, ReportContext) + assert report_context.is_simulated is True + assert report_context.report.is_simulated is True + assert step.current_step.is_simulated is True + assert isinstance(step, NewStep) + assert isinstance(module_substep, NewStep) + """ + ) + result = pytester.runpytest_subprocess("--sift-disabled") + result.assert_outcomes(passed=1) + + def test_disabled_writes_no_log_file_even_when_path_pinned( + self, + pytester: pytest.Pytester, + tmp_path: Path, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + """Disabled mode skips the log-file pipeline even when a path is pinned.""" + log_path = tmp_path / "should-not-exist.jsonl" + write_plugin_conftest() + pytester.makepyfile("def test_runs(step): step.measure(name='v', value=1.0)") + result = pytester.runpytest_subprocess("--sift-disabled", f"--sift-log-file={log_path}") + result.assert_outcomes(passed=1) + assert not log_path.exists(), f"log file unexpectedly created at {log_path}" + + def test_disabled_skips_client_has_connection_and_sift_client( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + ) -> None: + """Disabled mode never resolves ``client_has_connection`` or ``sift_client``. + + The plugin's ``report_context`` short-circuits to the stub before + consulting either fixture. Overrides that raise on resolution stay + un-triggered, so the inner test passes cleanly. + """ + pytester.makeconftest( + """ + import pytest + + pytest_plugins = ["sift_client.pytest_plugin"] + + + @pytest.fixture(scope="session") + def sift_client(): + raise AssertionError("sift_client should not resolve in disabled mode") + + + @pytest.fixture(scope="session") + def client_has_connection(): + raise AssertionError( + "client_has_connection should not resolve in disabled mode" + ) + """ + ) + pytester.makepyfile( + """ + def test_runs(step): + assert step.measure(name="v", value=5.0, bounds={"max": 10.0}) is True + """ + ) + result = pytester.runpytest_subprocess("--sift-disabled") + result.assert_outcomes(passed=1) diff --git a/python/lib/sift_client/_tests/pytest_plugin/test_offline.py b/python/lib/sift_client/_tests/pytest_plugin/test_offline.py new file mode 100644 index 000000000..f0470bad3 --- /dev/null +++ b/python/lib/sift_client/_tests/pytest_plugin/test_offline.py @@ -0,0 +1,135 @@ +"""Tests for ``--sift-offline`` mode. + +Offline mode routes every create/update through the JSONL log file without +contacting Sift. The session-start ping is skipped, the import worker is not +spawned, and missing ``SIFT_*`` env vars are tolerated (placeholders are +filled). Offline + ``--sift-log-file=none`` is rejected as a +usage error since the log file is the sole sink in this mode. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Callable + +if TYPE_CHECKING: + from pathlib import Path + + import pytest + + +class TestOfflineMode: + def test_offline_runs_without_network( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + """Offline mode constructs the client locally and never pings.""" + write_plugin_conftest() + pytester.makepyfile( + """ + def test_in_bounds(step): + assert step.measure(name="v", value=5.0, bounds={"min": 4.8, "max": 5.2}) + + def test_out_of_bounds(step): + assert step.measure(name="v", value=10.0, bounds={"max": 5.2}) is False + """ + ) + result = pytester.runpytest_subprocess("--sift-offline") + result.assert_outcomes(passed=2) + + def test_log_file_none_incompatible_with_offline( + self, + pytester: pytest.Pytester, + write_plugin_conftest: Callable[[], None], + ) -> None: + """``--sift-log-file=none`` + ``--sift-offline`` is a usage error.""" + write_plugin_conftest() + pytester.makepyfile("def test_should_not_run(): pass") + result = pytester.runpytest_subprocess("--sift-offline", "--sift-log-file=none") + assert result.ret != 0 + combined = "\n".join(result.outlines + result.errlines) + assert "incompatible with --sift-offline" in combined, combined + + def test_offline_yields_real_fixtures( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + """Offline mode runs a real ReportContext; entities still report `is_simulated=True` because the log-file path synthesizes responses prior to replay.""" + write_plugin_conftest() + pytester.makepyfile( + """ + from sift_client.util.test_results import ReportContext + from sift_client.util.test_results.context_manager import NewStep + + def test_types(step, report_context): + assert isinstance(report_context, ReportContext) + assert isinstance(step, NewStep) + assert report_context.client._simulate is False + # log-file mode synthesizes responses, so entities are flagged simulated. + assert report_context.is_simulated is True + assert step.current_step.is_simulated is True + """ + ) + result = pytester.runpytest_subprocess("--sift-offline") + result.assert_outcomes(passed=1) + + def test_offline_writes_jsonl_to_pinned_log_file( + self, + pytester: pytest.Pytester, + tmp_path: Path, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + """Offline mode populates the pinned JSONL file with create/update entries.""" + log_path = tmp_path / "run.jsonl" + write_plugin_conftest() + pytester.makepyfile( + """ + def test_one(step): + assert step.measure( + name="v", value=5.0, bounds={"min": 4.8, "max": 5.2} + ) is True + """ + ) + result = pytester.runpytest_subprocess("--sift-offline", f"--sift-log-file={log_path}") + result.assert_outcomes(passed=1) + assert log_path.exists(), f"offline mode did not create {log_path}" + content = log_path.read_text() + assert content.strip(), "log file is empty" + # Each non-empty line is ``[Operation:uuid] {json}``. A successful + # session produces at least the report create + step create lines. + lines = [line for line in content.splitlines() if line.strip()] + assert any(line.startswith("[CreateTestReport:") for line in lines), content + assert any(line.startswith("[CreateTestStep:") for line in lines), content + + def test_offline_skips_client_has_connection( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + ) -> None: + """Offline mode never resolves ``client_has_connection``. + + Override the fixture to raise on resolution. If the override is + invoked, the session aborts. If it isn't, the inner test passes + cleanly, which confirms the offline path skipped the ping check. + """ + pytester.makeconftest( + """ + import pytest + + pytest_plugins = ["sift_client.pytest_plugin"] + + + @pytest.fixture(scope="session") + def client_has_connection(): + raise AssertionError( + "client_has_connection should not resolve in offline mode" + ) + """ + ) + pytester.makepyfile("def test_runs(step): pass") + result = pytester.runpytest_subprocess("--sift-offline") + result.assert_outcomes(passed=1) diff --git a/python/lib/sift_client/_tests/pytest_plugin/test_online.py b/python/lib/sift_client/_tests/pytest_plugin/test_online.py new file mode 100644 index 000000000..876fffb0e --- /dev/null +++ b/python/lib/sift_client/_tests/pytest_plugin/test_online.py @@ -0,0 +1,133 @@ +"""Tests for online mode (the default). + +Online mode requires connectivity to Sift. The plugin pings via +``client_has_connection`` at session start and aborts with +``pytest.UsageError`` on failure. Missing ``SIFT_API_KEY`` / +``SIFT_GRPC_URI`` / ``SIFT_REST_URI`` env vars are reported as a usage error +so the failure is actionable. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Callable + +if TYPE_CHECKING: + from pathlib import Path + + import pytest + + +class TestOnlineMode: + def test_ping_failure_aborts( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + ) -> None: + """Online mode with an unreachable ping aborts the session via UsageError.""" + pytester.makeconftest( + """ + import pytest + from unittest.mock import MagicMock + + pytest_plugins = ["sift_client.pytest_plugin"] + + + @pytest.fixture(scope="session") + def sift_client(): + client = MagicMock() + client.ping.ping.side_effect = ConnectionError("unreachable") + return client + """ + ) + pytester.makepyfile( + """ + import pytest + + @pytest.mark.sift_include + def test_should_not_run(): + assert True + """ + ) + result = pytester.runpytest_subprocess() + assert result.ret != 0 + combined = "\n".join(result.outlines + result.errlines) + assert "Sift ping failed" in combined, combined + + def test_missing_env_vars_named_in_error( + self, + pytester: pytest.Pytester, + clear_sift_env: None, + write_plugin_conftest: Callable[[], None], + ) -> None: + """The default ``sift_client`` fixture names missing env vars in its error.""" + write_plugin_conftest() + pytester.makepyfile( + """ + import pytest + + @pytest.mark.sift_include + def test_should_not_run(): + pass + """ + ) + result = pytester.runpytest_subprocess() + assert result.ret != 0 + combined = "\n".join(result.outlines + result.errlines) + for var in ("SIFT_API_KEY", "SIFT_GRPC_URI", "SIFT_REST_URI"): + assert var in combined, combined + + def test_online_resolves_client_has_connection_once( + self, + pytester: pytest.Pytester, + tmp_path: Path, + clear_sift_env: None, + ) -> None: + """Online mode resolves ``client_has_connection`` exactly once at session start. + + Overrides the fixture to bump a counter persisted to a file the outer + test reads after the inner session finishes. Outcomes aren't asserted + because the real ``ReportContext`` constructed against a ``MagicMock`` + client crashes downstream when Pydantic sees mock IDs; what we're + verifying is the ping path itself, which runs before construction. + """ + counter_file = tmp_path / "ping_calls.txt" + pytester.makeconftest( + f""" + from pathlib import Path + from unittest.mock import MagicMock + + import pytest + + pytest_plugins = ["sift_client.pytest_plugin"] + + _COUNTER = Path({str(counter_file)!r}) + + + @pytest.fixture(scope="session") + def sift_client(): + return MagicMock() + + + @pytest.fixture(scope="session") + def client_has_connection(): + prior = int(_COUNTER.read_text()) if _COUNTER.exists() else 0 + _COUNTER.write_text(str(prior + 1)) + return True + """ + ) + pytester.makepyfile( + """ + import pytest + + @pytest.mark.sift_include + def test_a(): pass + + @pytest.mark.sift_include + def test_b(): pass + """ + ) + pytester.runpytest_subprocess() + assert counter_file.exists(), "client_has_connection was not resolved" + assert counter_file.read_text() == "1", ( + f"expected session-scoped fixture to resolve once, got {counter_file.read_text()}" + ) diff --git a/python/lib/sift_client/_tests/util/conftest.py b/python/lib/sift_client/_tests/util/conftest.py index 2f371e69e..9e255da8a 100644 --- a/python/lib/sift_client/_tests/util/conftest.py +++ b/python/lib/sift_client/_tests/util/conftest.py @@ -7,13 +7,13 @@ 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 + config.option.sift_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 + The project-wide default in ``pyproject.toml`` is ``sift_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 diff --git a/python/lib/sift_client/_tests/util/test_report_context.py b/python/lib/sift_client/_tests/util/test_report_context.py new file mode 100644 index 000000000..f12247c7a --- /dev/null +++ b/python/lib/sift_client/_tests/util/test_report_context.py @@ -0,0 +1,95 @@ +"""Tier 1 tests for `ReportContext.__exit__`'s replay-worker handling. + +Each test substitutes the `import-test-result-log` argv with a tiny Python +`-c` invocation that produces a controlled end-state (clean exit / hang / +non-zero exit), then enters and exits a `ReportContext` against a +simulate-mode `SiftClient`. This validates that real subprocess outcomes +route to the right branch of `__exit__` without depending on the real +replay binary or a Sift backend. +""" + +from __future__ import annotations + +import logging +import sys +from typing import TYPE_CHECKING + +from sift_client import SiftClient, SiftConnectionConfig +from sift_client.util.test_results import ReportContext + +if TYPE_CHECKING: + import pytest + + +def _make_simulate_client() -> SiftClient: + """Build a SiftClient flagged for in-process simulation. + + Constructor URLs are placeholders; nothing dials them because every + test-results write short-circuits through the simulate path. + """ + client = SiftClient( + connection_config=SiftConnectionConfig( + api_key="test", + grpc_url="test.invalid:0", + rest_url="http://test.invalid", + ) + ) + client._simulate = True + return client + + +def _make_context(command: list[str]) -> ReportContext: + """Build a ReportContext whose replay subprocess is the provided command. + + `log_file=True` triggers the temp-file path so `_open_import_proc` fires + on `__enter__`. The substitute argv is swapped in via the public-ish + `_build_replay_command` hook so the production Popen kwargs stay + exercised. + """ + rc = ReportContext(_make_simulate_client(), name="test", log_file=True) + rc._build_replay_command = lambda: command # type: ignore[method-assign] + return rc + + +def test_worker_clean_exit_is_silent(caplog: pytest.LogCaptureFixture) -> None: + """Worker exits with code 0 → __exit__ is silent (case 1).""" + rc = _make_context([sys.executable, "-c", "pass"]) + with caplog.at_level(logging.ERROR): + with rc: + pass + assert "Import process" not in caplog.text + assert "replay-test-result-log" not in caplog.text + assert rc._import_proc is not None + assert rc._import_proc.returncode == 0 + + +def test_worker_timeout_kills_and_logs(caplog: pytest.LogCaptureFixture) -> None: + """Worker still running at session end → kill + log, no raise (case 2).""" + rc = _make_context([sys.executable, "-c", "import time; time.sleep(30)"]) + with caplog.at_level(logging.ERROR): + with rc: + pass + assert rc._import_proc is not None + # `kill()` + `wait()` were called; process is dead. + assert rc._import_proc.poll() is not None + assert "did not exit in 1s" in caplog.text + assert "replay-test-result-log" in caplog.text + + +def test_worker_nonzero_exit_logs_stderr_no_raise(caplog: pytest.LogCaptureFixture) -> None: + """Worker exits non-zero with stderr → log stderr + replay hint, no raise (case 3).""" + rc = _make_context( + [ + sys.executable, + "-c", + "import sys; sys.stderr.write('rpc deadline exceeded'); sys.exit(2)", + ] + ) + with caplog.at_level(logging.ERROR): + with rc: + pass + assert rc._import_proc is not None + assert rc._import_proc.returncode == 2 + assert "exited with code 2" in caplog.text + assert "rpc deadline exceeded" in caplog.text + assert "replay-test-result-log" in caplog.text diff --git a/python/lib/sift_client/client.py b/python/lib/sift_client/client.py index 95fd25b71..ff574adba 100644 --- a/python/lib/sift_client/client.py +++ b/python/lib/sift_client/client.py @@ -152,6 +152,11 @@ def __init__( WithGrpcClient.__init__(self, grpc_client=grpc_client) WithRestClient.__init__(self, rest_client=rest_client) + # When set, test-results writes return synthesized responses without + # contacting Sift. Read by `TestResultsAPIAsync._simulate`. Used by the + # pytest plugin's ``--sift-disabled`` mode. + self._simulate: bool = False + self.ping = PingAPI(self) self.assets = AssetsAPI(self) self.calculated_channels = CalculatedChannelsAPI(self) diff --git a/python/lib/sift_client/pytest_plugin.py b/python/lib/sift_client/pytest_plugin.py index f2699a954..494ded3b6 100644 --- a/python/lib/sift_client/pytest_plugin.py +++ b/python/lib/sift_client/pytest_plugin.py @@ -15,7 +15,7 @@ if TYPE_CHECKING: from sift_client.util.test_results.context_manager import NewStep -REPORT_CONTEXT: ReportContext | None = None +REPORT_CONTEXT: Any = None @dataclass(frozen=True) @@ -37,39 +37,53 @@ class _Option: _LOG_FILE = _Option( - cli_flag="--sift-test-results-log-file", - ini_name="sift_test_results_log_file", + cli_flag="--sift-log-file", + ini_name="sift_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').", + ini_help="Default value for --sift-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", + cli_flag="--no-sift-git-metadata", + ini_name="sift_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.", + "Defaults to true. The --no-sift-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", +_OFFLINE = _Option( + cli_flag="--sift-offline", + ini_name="sift_offline", 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.", + cli_help="Run without contacting Sift. All create/update calls are written " + "to a JSONL log file for later replay via `import-test-result-log`. " + "No session-start ping is attempted.", + ini_help="When true, run in offline mode (same effect as --sift-offline). Defaults to false.", + ini_type="bool", + ini_default=False, +) + +_DISABLED = _Option( + cli_flag="--sift-disabled", + ini_name="sift_disabled", + action="store_true", + cli_help="Disable Sift integration entirely. Nothing contacts the API " + "and no log file is written. `step.measure(...)` still returns real " + "pass/fail booleans. Returned entities expose `is_simulated == True`. " + "Also honored via the `SIFT_DISABLED` env var. Supersedes every other " + "flag.", + ini_help="When true, run in disabled mode (same effect as --sift-disabled). " + "Also honored via the SIFT_DISABLED env var. Supersedes every other " + "setting. Defaults to false.", ini_type="bool", ini_default=False, ) @@ -89,7 +103,7 @@ class _Option: ) _AUTOUSE = _Option( - ini_name="sift_test_results_autouse", + ini_name="sift_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 " @@ -102,7 +116,8 @@ class _Option: _OPTIONS: tuple[_Option, ...] = ( _LOG_FILE, _GIT_METADATA, - _CHECK_CONNECTION, + _OFFLINE, + _DISABLED, _GRPC_URI, _REST_URI, _AUTOUSE, @@ -139,15 +154,25 @@ def pytest_configure(config: pytest.Config) -> None: 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.", + "regardless of the `sift_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.", + "regardless of the `sift_autouse` ini default.", ) +def _is_offline(pytestconfig: pytest.Config | None) -> bool: + return bool(_option_or_ini(pytestconfig, _OFFLINE)) + + +def _is_disabled(pytestconfig: pytest.Config | None) -> bool: + if bool(_option_or_ini(pytestconfig, _DISABLED)): + return True + return os.getenv("SIFT_DISABLED", "").lower() in ("1", "true", "yes") + + def _sift_enabled_for(node: pytest.Item | pytest.Collector, default: bool) -> bool: """Resolve the Sift gate for a node: sift_exclude > sift_include > default. @@ -203,13 +228,23 @@ def _resolve_log_file(pytestconfig: pytest.Config | None) -> str | Path | bool | * ``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 + ``config.option.sift_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. + + Rejects ``--sift-log-file=none`` combined with ``--sift-offline`` since + offline mode needs the log file as its sole sink. """ raw = _option_or_ini(pytestconfig, _LOG_FILE) + disabled = raw is False or (isinstance(raw, str) and raw.lower() in ("false", "none")) + if disabled and _is_offline(pytestconfig): + raise pytest.UsageError( + "--sift-log-file=none is incompatible with --sift-offline; offline " + "mode requires a log file. Pin one with --sift-log-file=, or " + "drop --sift-log-file=none to use a temp file." + ) if raw is False: return None if not raw: @@ -239,7 +274,7 @@ def _report_context_impl( sift_client: SiftClient, request: pytest.FixtureRequest, pytestconfig: pytest.Config | None = None, -) -> Generator[ReportContext | None, None, None]: +) -> Generator[ReportContext, None, None]: args = request.config.invocation_params.args test_path = Path(args[0]) if args else None if test_path is not None and test_path.exists(): @@ -248,7 +283,13 @@ def _report_context_impl( else: base_name = "pytest " + " ".join(args) if args else "pytest" test_case = base_name - log_file = _resolve_log_file(pytestconfig) + # Mode → ReportContext flags: + # online (default): log_file=, replay_log_file=True + # --sift-offline: log_file=, replay_log_file=False + # --sift-disabled: log_file=False, replay_log_file=False + disabled = sift_client._simulate + offline = False if disabled else _is_offline(pytestconfig) + log_file: str | Path | bool | None = False if disabled else _resolve_log_file(pytestconfig) git_metadata = _option_or_ini(pytestconfig, _GIT_METADATA) include_git_metadata = True if git_metadata is None else bool(git_metadata) with ReportContext( @@ -257,28 +298,46 @@ def _report_context_impl( test_case=str(test_case), log_file=log_file, include_git_metadata=include_git_metadata, + replay_log_file=not (disabled or offline), ) as context: global REPORT_CONTEXT REPORT_CONTEXT = context yield context -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, _CHECK_CONNECTION)) - - -def _has_sift_connection(request: pytest.FixtureRequest) -> bool: - """Resolve the `client_has_connection` fixture lazily; only called when the check is enabled.""" - return bool(request.getfixturevalue("client_has_connection")) - - _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", _GRPC_URI), ("SIFT_REST_URI", _REST_URI), ) +# Placeholder credentials used in --sift-offline mode when env/ini values +# are missing. Offline mode never makes network calls, so the values are +# only syntactically required by SiftConnectionConfig. +_OFFLINE_DEFAULTS = { + "SIFT_API_KEY": "offline", + "SIFT_GRPC_URI": "offline.invalid:0", + "SIFT_REST_URI": "http://offline.invalid", +} + + +def _build_disabled_client() -> SiftClient: + """Construct a SiftClient for ``--sift-disabled`` mode. + + Tagged with ``_simulate=True`` so test-results writes short-circuit through + the existing low-level simulate path without contacting Sift. The URLs are + syntactically valid but unreachable; nothing dials them. + """ + client = SiftClient( + connection_config=SiftConnectionConfig( + api_key="disabled", + grpc_url="disabled.invalid:0", + rest_url="http://disabled.invalid", + ) + ) + client._simulate = True + return client + def _resolve_credential( pytestconfig: pytest.Config | None, env_name: str, opt: _Option | None @@ -308,10 +367,19 @@ def sift_client(pytestconfig: pytest.Config) -> SiftClient: etc.) can override this fixture by defining their own ``sift_client`` in their ``conftest.py``; pytest fixture resolution prefers the local definition. + + In ``--sift-offline`` mode the missing-credential check is relaxed: + real env vars and ini values still win when set (so the client is + constructible against a real backend even though no calls are made), but + anything still missing is filled with a placeholder. In ``--sift-disabled`` + mode the credential resolution is skipped entirely and placeholders are + always used. """ + if _is_disabled(pytestconfig): + return _build_disabled_client() 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: + if missing and not _is_offline(pytestconfig): raise pytest.UsageError( "Sift credentials missing: " + ", ".join(missing) @@ -319,8 +387,11 @@ def sift_client(pytestconfig: pytest.Config) -> SiftClient: "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." + "conftest.py, or pass --sift-offline / --sift-disabled to run " + "without contacting Sift." ) + for env in missing: + resolved[env] = _OFFLINE_DEFAULTS[env] # `or ""` is unreachable in practice since the `missing` check above guarantees # non-None values return SiftClient( @@ -334,32 +405,61 @@ def sift_client(pytestconfig: pytest.Config) -> SiftClient: @pytest.fixture(scope="session") def report_context( - sift_client: SiftClient, request: pytest.FixtureRequest, pytestconfig: pytest.Config -) -> Generator[ReportContext | None, None, None]: + request: pytest.FixtureRequest, pytestconfig: pytest.Config +) -> Generator[ReportContext, None, None]: """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 - 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. + 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 (or teardown subprocess) is created. + + What gets yielded depends on the mode: + + * ``--sift-disabled``: a real ``ReportContext`` against a placeholder + ``SiftClient`` with ``_simulate=True``. Every test-results write + returns a synthesized response without contacting Sift; no log file + is written; the replay subprocess never spawns. Test code that calls + ``step.measure(...)`` keeps working because bounds are evaluated as + usual and routed through the simulate path. + * ``--sift-offline``: a real ReportContext, but the session-start ping + is skipped, all create/update calls go to the JSONL log file, and + the import-test-result-log replay subprocess is not spawned at + session end. + * default (online): verify connectivity via ``client_has_connection`` + before constructing the context. A failed ping aborts the session + with ``pytest.UsageError`` and points at ``--sift-offline`` and + ``--sift-disabled`` as escape hatches. + + The log-file destination is controlled by + ``--sift-log-file``; defaults to a temp file when unset. """ - if _check_connection_enabled(pytestconfig) and not _has_sift_connection(request): - yield None + if _is_disabled(pytestconfig): + yield from _report_context_impl( + _build_disabled_client(), request, pytestconfig=pytestconfig + ) return + sift_client = request.getfixturevalue("sift_client") + if not _is_offline(pytestconfig): + try: + request.getfixturevalue("client_has_connection") + except pytest.UsageError: + raise + except Exception as exc: + grpc_config = getattr(getattr(sift_client, "grpc_client", None), "_config", None) + grpc_url = getattr(grpc_config, "uri", "") + raise pytest.UsageError( + f"Sift ping failed against {grpc_url}: {exc}. " + "Pass --sift-offline to run without contacting Sift, or " + "--sift-disabled to skip Sift entirely." + ) from exc yield from _report_context_impl(sift_client, request, pytestconfig=pytestconfig) def _step_impl( report_context: ReportContext, request: pytest.FixtureRequest -) -> Generator[NewStep | None, None, None]: +) -> Generator[NewStep, None, None]: name = str(request.node.name) existing_docstring = request.node.obj.__doc__ or None with report_context.new_step( @@ -383,18 +483,18 @@ def step( 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 + `sift_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. + triggers its creation, subsequent gated tests reuse it. In + ``--sift-disabled`` mode the report context is backed by a + ``SiftClient(_simulate=True)`` placeholder, so every write returns a + synthesized response without contacting Sift. """ 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(rc, request) @@ -416,21 +516,22 @@ def module_substep( yield None return rc = request.getfixturevalue("report_context") - if rc is None: - yield None - return yield from _step_impl(rc, request) @pytest.fixture(scope="session") -def client_has_connection(sift_client): - """Check if the SiftClient has a connection to the Sift server. - - 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. +def client_has_connection(pytestconfig: pytest.Config, request: pytest.FixtureRequest) -> bool: + """Verify the ``SiftClient`` can reach Sift via ``/ping``. + + Consulted at session start by ``report_context`` in online mode. A failed + ping raises through ``report_context`` and aborts the session with + ``pytest.UsageError``. Override this fixture in your conftest to use a + different reachability signal (e.g. a cached auth token) for environments + where pinging is the wrong check. Returns ``False`` in ``--sift-disabled`` + mode without constructing a client. """ - try: - sift_client.ping.ping() - return True - except Exception: + if _is_disabled(pytestconfig): return False + sift_client = request.getfixturevalue("sift_client") + sift_client.ping.ping() + return True diff --git a/python/lib/sift_client/resources/test_results.py b/python/lib/sift_client/resources/test_results.py index 22e984b5e..9e88b6081 100644 --- a/python/lib/sift_client/resources/test_results.py +++ b/python/lib/sift_client/resources/test_results.py @@ -96,6 +96,7 @@ async def create( created_report = await self._low_level_client.create_test_report( test_report=test_report, log_file=log_file, + simulate=self.client._simulate, ) return self._finalize(created_report, log_file) @@ -271,7 +272,7 @@ async def update( update.resource_id = test_report_id existing = test_report if isinstance(test_report, TestReport) else None updated_test_report = await self._low_level_client.update_test_report( - update, log_file=log_file, existing=existing + update, log_file=log_file, existing=existing, simulate=self.client._simulate ) return self._finalize(updated_test_report, log_file) @@ -319,7 +320,7 @@ async def create_step( if isinstance(test_step, dict): test_step = TestStepCreate.model_validate(test_step) test_step_result = await self._low_level_client.create_test_step( - test_step, log_file=log_file + test_step, log_file=log_file, simulate=self.client._simulate ) return self._finalize(test_step_result, log_file) @@ -450,7 +451,7 @@ async def update_step( update.resource_id = test_step_id existing = test_step if isinstance(test_step, TestStep) else None updated_test_step = await self._low_level_client.update_test_step( - update, log_file=log_file, existing=existing + update, log_file=log_file, existing=existing, simulate=self.client._simulate ) return self._finalize(updated_test_step, log_file) @@ -484,10 +485,10 @@ async def create_measurement( if isinstance(test_measurement, dict): test_measurement = TestMeasurementCreate.model_validate(test_measurement) test_measurement_result = await self._low_level_client.create_test_measurement( - test_measurement, log_file=log_file + test_measurement, log_file=log_file, simulate=self.client._simulate ) measurement = self._finalize(test_measurement_result, log_file) - if update_step and log_file is None: + if update_step and log_file is None and not self.client._simulate: step = await self.get_step(test_step=test_measurement_result.test_step_id) if step.status == TestStatus.PASSED and not measurement.passed: await self.update_step(test_step=step, update={"status": TestStatus.FAILED}) @@ -508,7 +509,7 @@ async def create_measurements( A tuple of (measurements_created_count, measurement_ids). """ return await self._low_level_client.create_test_measurements( - test_measurements, log_file=log_file + test_measurements, log_file=log_file, simulate=self.client._simulate ) async def list_measurements( @@ -621,10 +622,16 @@ async def update_measurement( update.resource_id = test_measurement.id_ updated_test_measurement = await self._low_level_client.update_test_measurement( - update, log_file=log_file, existing=test_measurement + update, log_file=log_file, existing=test_measurement, simulate=self.client._simulate ) updated_test_measurement = self._finalize(updated_test_measurement, log_file) - if update_step and log_file is None and update.passed is not None and not update.passed: + if ( + update_step + and log_file is None + and not self.client._simulate + and update.passed is not None + and not update.passed + ): step = await self.get_step(test_step=updated_test_measurement.test_step_id) if step.status == TestStatus.PASSED: await self.update_step(test_step=step, update={"status": TestStatus.FAILED}) diff --git a/python/lib/sift_client/sift_types/_mixins/simulated.py b/python/lib/sift_client/sift_types/_mixins/simulated.py new file mode 100644 index 000000000..bdc2c572a --- /dev/null +++ b/python/lib/sift_client/sift_types/_mixins/simulated.py @@ -0,0 +1,32 @@ +"""Mixin that exposes ``is_simulated`` on test-results entity types.""" + +from __future__ import annotations + + +class SimulatedMixin: + """Mixin for sift_types whose response can be produced by the simulate path. + + The low-level wrapper stamps ``_simulated=True`` on entities it returns from + a simulated branch (see ``TestResultsLowLevelClient._mark_simulated``). This + mixin exposes that flag as a read-only ``is_simulated`` property so + consumers and tests can detect when an instance was synthesized rather than + round-tripped through Sift. + + Inheriting classes are expected to declare a private field + ``_simulated: bool = False`` so pydantic tracks the default correctly. + """ + + _simulated: bool + + @property + def is_simulated(self) -> bool: + """True when this instance was returned from the simulate path. + + Set by the low-level wrapper when the call short-circuited to a + synthesized response (either ``SiftClient._simulate`` mode or per-call + ``log_file`` / ``simulate=True``). False for entities returned from a + normal online call or constructed manually outside the SDK. Offline + mode also reports True since responses are synthesized prior to + replay. + """ + return self._simulated diff --git a/python/lib/sift_client/sift_types/test_report.py b/python/lib/sift_client/sift_types/test_report.py index ecc24f52f..c4abfc548 100644 --- a/python/lib/sift_client/sift_types/test_report.py +++ b/python/lib/sift_client/sift_types/test_report.py @@ -36,6 +36,7 @@ ModelUpdate, ) from sift_client.sift_types._mixins.file_attachments import FileAttachmentsMixin +from sift_client.sift_types._mixins.simulated import SimulatedMixin from sift_client.sift_types.channel import Channel from sift_client.util.metadata import metadata_dict_to_proto, metadata_proto_to_dict @@ -153,7 +154,7 @@ def to_proto(self) -> TestStepProto: return proto -class TestStep(BaseType[TestStepProto, "TestStep"], FileAttachmentsMixin): +class TestStep(BaseType[TestStepProto, "TestStep"], FileAttachmentsMixin, SimulatedMixin): """TestStep model representing a step in a test.""" test_report_id: str @@ -169,6 +170,8 @@ class TestStep(BaseType[TestStepProto, "TestStep"], FileAttachmentsMixin): metadata: dict[str, str | float | bool] | None = None # Set by the resource layer when this instance was produced from a logging-mode call _log_file: str | Path | None = None + # Set by the low-level wrapper when this instance came from the simulate path + _simulated: bool = False @classmethod def _from_proto(cls, proto: TestStepProto, sift_client: SiftClient | None = None) -> TestStep: @@ -383,7 +386,7 @@ def to_proto(self) -> TestMeasurementProto: return proto -class TestMeasurement(BaseType[TestMeasurementProto, "TestMeasurement"]): +class TestMeasurement(BaseType[TestMeasurementProto, "TestMeasurement"], SimulatedMixin): """TestMeasurement model representing a measurement in a test.""" measurement_type: TestMeasurementType @@ -404,6 +407,8 @@ class TestMeasurement(BaseType[TestMeasurementProto, "TestMeasurement"]): # Set by the resource layer when this instance was produced from a logging-mode call _log_file: str | Path | None = None + # Set by the low-level wrapper when this instance came from the simulate path + _simulated: bool = False @classmethod def _from_proto( @@ -599,7 +604,7 @@ def _to_proto(self) -> ErrorInfoProto: ) -class TestReport(BaseType[TestReportProto, "TestReport"], FileAttachmentsMixin): +class TestReport(BaseType[TestReportProto, "TestReport"], FileAttachmentsMixin, SimulatedMixin): """TestReport model representing a test report.""" status: TestStatus @@ -617,6 +622,8 @@ class TestReport(BaseType[TestReportProto, "TestReport"], FileAttachmentsMixin): is_archived: bool # Set by the resource layer when this instance was produced from a logging-mode call _log_file: str | Path | None = None + # Set by the low-level wrapper when this instance came from the simulate path + _simulated: bool = False @classmethod def _from_proto( diff --git a/python/lib/sift_client/util/test_results/__init__.py b/python/lib/sift_client/util/test_results/__init__.py index ea213056e..ddce0326c 100644 --- a/python/lib/sift_client/util/test_results/__init__.py +++ b/python/lib/sift_client/util/test_results/__init__.py @@ -68,13 +68,13 @@ def main(self): 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. +entirely with `--sift-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 +- Set `sift_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. @@ -105,19 +105,24 @@ def pytest_collection_modifyitems(config, items): CLI options registered by the plugin: -- `--sift-test-results-log-file`: Path to write the JSONL log file. `true` +- `--sift-offline`: Run without contacting Sift. All create/update calls are + written to the JSONL log file for later replay via `import-test-result-log`. + No session-start ping is attempted. +- `--sift-disabled`: Skip Sift entirely. Nothing contacts the API and no + log file is written. `step.measure(...)` still evaluates bounds and + returns a real pass/fail boolean. Returned entities expose + ``is_simulated == True``. Also honored via the `SIFT_DISABLED` env + var. Supersedes every other flag. +- `--sift-log-file`: Path to write the JSONL log file. `true` (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, +- `--no-sift-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). 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 -``sift_test_results_autouse`` ini key (bool, default ``true``) sets the +``sift_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 @@ -126,10 +131,9 @@ def pytest_collection_modifyitems(config, items): ```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 +sift_autouse = false +sift_offline = true +sift_git_metadata = false sift_grpc_uri = "your-org.sift.example:443" sift_rest_uri = "https://your-org.sift.example" ``` diff --git a/python/lib/sift_client/util/test_results/bounds.py b/python/lib/sift_client/util/test_results/bounds.py index ef5c67ce5..b734cc126 100644 --- a/python/lib/sift_client/util/test_results/bounds.py +++ b/python/lib/sift_client/util/test_results/bounds.py @@ -1,5 +1,10 @@ from __future__ import annotations +from typing import TYPE_CHECKING + +import numpy as np +import pandas as pd + from sift_client.sift_types.test_report import ( NumericBounds, TestMeasurement, @@ -8,6 +13,55 @@ TestMeasurementUpdate, ) +if TYPE_CHECKING: + from numpy.typing import NDArray + + +def to_numpy_array( + values: list[float | int] | NDArray[np.float64] | pd.Series, +) -> NDArray[np.float64]: + """Normalize a list / ndarray / pandas Series into a numpy array. + + Shared by ``measure_avg`` and ``measure_all`` on ``NewStep`` so the + accepted input types stay in sync across measurement variants. + """ + if isinstance(values, list): + return np.array(values) + if isinstance(values, np.ndarray): + return values + if isinstance(values, pd.Series): + return values.to_numpy() + raise ValueError(f"Invalid value type: {type(values)}") + + +def out_of_bounds_mask( + arr: NDArray[np.float64], + bounds: dict[str, float] | NumericBounds, +) -> NDArray[np.bool_]: + """Return a boolean mask selecting elements of ``arr`` that violate ``bounds``. + + Raises ``ValueError`` when ``bounds`` has neither ``min`` nor ``max`` set. + """ + if isinstance(bounds, dict): + bounds = NumericBounds(min=bounds.get("min"), max=bounds.get("max")) + mask: NDArray[np.bool_] | None = None + if bounds.min is not None: + mask = arr < bounds.min + if bounds.max is not None: + above = arr > bounds.max + mask = mask | above if mask is not None else above + if mask is None: + raise ValueError("No bounds provided") + return mask + + +def all_within_bounds( + arr: NDArray[np.float64], + bounds: dict[str, float] | NumericBounds, +) -> bool: + """Return True when every element of ``arr`` is within ``bounds``.""" + return bool(arr[out_of_bounds_mask(arr, bounds)].size == 0) + def assign_value_to_measurement( measurement: TestMeasurement | TestMeasurementCreate | TestMeasurementUpdate, @@ -32,6 +86,38 @@ def assign_value_to_measurement( raise ValueError(f"Invalid value type: {type(value)}") +def value_passes_bounds( + value: float | str | bool, + bounds: dict[str, float] | NumericBounds | str | bool | None, +) -> bool: + """Evaluate a value against bounds without recording a measurement.""" + if bounds is None: + return True + if isinstance(bounds, dict): + bounds = NumericBounds(min=bounds.get("min"), max=bounds.get("max")) + if isinstance(bounds, bool): + if isinstance(value, str): + return str(value).lower() == str(bounds).lower() + return bool(value) == bounds + if isinstance(bounds, str): + if not (isinstance(value, str) or isinstance(value, bool)): + raise ValueError("Value must be a string if bounds provided is a string") + if isinstance(value, bool): + return str(value).lower() == str(bounds).lower() + return value == bounds + # NumericBounds + try: + if bounds.min is not None and bounds.min > value: # type: ignore[operator] + return False + if bounds.max is not None and bounds.max < value: # type: ignore[operator] + return False + except TypeError: + raise TypeError( + f"Value must be a float or int to evaluate numeric bounds but gave {type(value)}" + ) from None + return True + + def evaluate_measurement_bounds( measurement: TestMeasurement | TestMeasurementCreate | TestMeasurementUpdate, value: float | str | bool, @@ -53,31 +139,10 @@ def evaluate_measurement_bounds( if isinstance(bounds, dict): bounds = NumericBounds(min=bounds.get("min"), max=bounds.get("max")) - if isinstance(bounds, bool): - if isinstance(value, str): - measurement.passed = str(value).lower() == str(bounds).lower() - else: - measurement.passed = bool(value) == bounds - return bool(measurement.passed) - elif isinstance(bounds, str): - if not (isinstance(value, str) or isinstance(value, bool)): - raise ValueError("Value must be a string if bounds provided is a string") + if isinstance(bounds, str) and not isinstance(bounds, bool): measurement.string_expected_value = bounds - if isinstance(value, bool): - measurement.passed = str(value).lower() == str(bounds).lower() - else: - measurement.passed = value == bounds elif isinstance(bounds, NumericBounds): measurement.numeric_bounds = bounds - measurement.passed = True - try: - if measurement.numeric_bounds.min is not None: - measurement.passed = measurement.passed and measurement.numeric_bounds.min <= value # type: ignore - if measurement.numeric_bounds.max is not None: - measurement.passed = measurement.passed and measurement.numeric_bounds.max >= value # type: ignore - except TypeError: - raise TypeError( - f"Value must be a float or int to evaluate numeric bounds but gave {type(value)}" - ) from None + measurement.passed = value_passes_bounds(value, bounds) return bool(measurement.passed) diff --git a/python/lib/sift_client/util/test_results/context_manager.py b/python/lib/sift_client/util/test_results/context_manager.py index 354f8564d..3d375814a 100644 --- a/python/lib/sift_client/util/test_results/context_manager.py +++ b/python/lib/sift_client/util/test_results/context_manager.py @@ -13,7 +13,6 @@ from typing import TYPE_CHECKING import numpy as np -import pandas as pd from sift_client.sift_types.test_report import ( ErrorInfo, @@ -28,9 +27,12 @@ ) from sift_client.util.test_results.bounds import ( evaluate_measurement_bounds, + out_of_bounds_mask, + to_numpy_array, ) if TYPE_CHECKING: + import pandas as pd from numpy.typing import NDArray from sift_client.client import SiftClient @@ -118,6 +120,7 @@ def __init__( test_case: str | None = None, log_file: str | Path | bool | None = None, include_git_metadata: bool = False, + replay_log_file: bool = True, ): """Initialize a new report context. @@ -128,10 +131,18 @@ def __init__( system_operator: The operator of the test system. Will default to the current user if not provided. test_case: The name of the test case. Will default to the basename of the file containing the test if not provided. log_file: If True, create a temp log file. If a path, use that path. - All create/update operations will be logged to this file. + If False/None, no log file is written and create/update calls + the API. include_git_metadata: If True, include git metadata in the report. + replay_log_file: When True (the default) and ``log_file`` is set, + spawn ``import-test-result-log --incremental`` to push log + entries to Sift in the background during the session. When + False, the log file is just a record and no worker is spawned. + Replay happens later via ``replay-test-result-log ``. + Has no effect when ``log_file`` is None. """ self.client = client + self.replay_log_file = replay_log_file self.step_is_open = False self.step_stack = [] self.step_number_at_depth = {} @@ -163,28 +174,41 @@ def __init__( ) self.report = client.test_results.create(create, log_file=self.log_file) + def _build_replay_command(self) -> list[str]: + """Build the argv for the import-test-result-log replay subprocess. + + Factored out for testability — tests substitute commands that exit + with controlled returncodes / stderr to exercise the ``__exit__`` + branches without depending on the real replay binary. + """ + return [ + "import-test-result-log", + "--incremental", + str(self.log_file), + "--grpc-url", + self.client.grpc_client._config.uri, + "--rest-url", + self.client.rest_client._config.base_url, + "--api-key", + self.client.grpc_client._config.api_key, + ] + def _open_import_proc(self): - """Open a subprocess to import the log file.""" + """Open a subprocess to import the log file. + + ``stderr`` is captured so a worker crash mid-session can surface its + error at session end via ``__exit__`` rather than failing silently. + """ with _quiet_fork_stderr(): self._import_proc = subprocess.Popen( - [ - "import-test-result-log", - "--incremental", - str(self.log_file), - "--grpc-url", - self.client.grpc_client._config.uri, - "--rest-url", - self.client.rest_client._config.base_url, - "--api-key", - self.client.grpc_client._config.api_key, - ], + self._build_replay_command(), stdin=subprocess.PIPE, stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, + stderr=subprocess.PIPE, ) def __enter__(self): - if self.log_file: + if self.log_file and self.replay_log_file: self._open_import_proc() return self @@ -199,17 +223,49 @@ def __exit__(self, exc_type, exc_value, traceback): self.report.update(update) if self._import_proc is not None: + # Three outcomes for the replay worker at session end. None of + # them fail the session — tests already ran and their outcome + # is independent of delivery. The local log file is the source + # of recovery for both failure modes via + # `replay-test-result-log `: + # 1. Exits cleanly (returncode 0). Silent. + # 2. Still running after the 1s grace window (TimeoutExpired). + # Healthy worker with a large backlog; kill and surface + # replay instructions. + # 3. Exited with non-zero. Connection failures and API call + # errors land here — the worker's replay loop has no retry, + # so the first failed RPC crashes the subprocess. Log the + # captured stderr at ERROR with replay instructions. try: - self._import_proc.communicate(timeout=1) + _, stderr_bytes = self._import_proc.communicate(timeout=1) except subprocess.TimeoutExpired: - logger.error("Import process did not exit in 10s, killing it") + logger.error("Import process did not exit in 1s, killing it") self._import_proc.kill() self._import_proc.wait() log_replay_instructions(self.log_file) - raise + return True # Ensures the session is marked as passed in pytest + if self._import_proc.returncode != 0: + stderr_text = ( + stderr_bytes.decode("utf-8", errors="replace").strip() if stderr_bytes else "" + ) + logger.error( + "Import process exited with code %d. stderr: %s", + self._import_proc.returncode, + stderr_text or "", + ) + log_replay_instructions(self.log_file) return True + @property + def is_simulated(self) -> bool: + """True when this context's report came from the simulate path. + + Delegates to ``self.report.is_simulated``; see ``TestReport.is_simulated`` + for the full semantics. + """ + return self.report.is_simulated + def new_step( self, name: str, @@ -505,15 +561,7 @@ def measure_avg( returns: The true if the average of the values is within the bounds, false otherwise. """ timestamp = timestamp if timestamp else datetime.now(timezone.utc) - np_array = None - if isinstance(values, list): - np_array = np.array(values) - elif isinstance(values, np.ndarray): - np_array = values - elif isinstance(values, pd.Series): - np_array = values.to_numpy() - else: - raise ValueError(f"Invalid value type: {type(values)}") + np_array = to_numpy_array(values) avg = float(np.mean(np_array)) result = self.measure( name=name, @@ -561,31 +609,8 @@ def measure_all( returns: The true if all values are within the bounds, false otherwise. """ timestamp = timestamp if timestamp else datetime.now(timezone.utc) - np_array = None - if isinstance(values, list): - np_array = np.array(values) - elif isinstance(values, np.ndarray): - np_array = values - elif isinstance(values, pd.Series): - np_array = values.to_numpy() - else: - raise ValueError(f"Invalid value type: {type(values)}") - - numeric_bounds = bounds - if isinstance(numeric_bounds, dict): - numeric_bounds = NumericBounds(min=bounds.get("min"), max=bounds.get("max")) # type: ignore - - # Construct a mask of the values that are outside the bounds. - mask = None - if numeric_bounds.min is not None: - mask = np_array < numeric_bounds.min - if numeric_bounds.max is not None: - val_above_max = np_array > numeric_bounds.max - mask = mask | val_above_max if mask is not None else val_above_max - if mask is None: - raise ValueError("No bounds provided") - - rows_outside_bounds = np_array[mask] + np_array = to_numpy_array(values) + rows_outside_bounds = np_array[out_of_bounds_mask(np_array, bounds)] for row in rows_outside_bounds: self.measure( name=name, diff --git a/python/pyproject.toml b/python/pyproject.toml index c0156f679..e1523bb43 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -333,6 +333,7 @@ ignore_errors = true [tool.setuptools.packages.find] where = ["lib"] +exclude = ["sift_client._tests", "sift_client._tests.*"] [tool.setuptools.package-data] sift_grafana = ["py.typed"] @@ -391,12 +392,18 @@ env_files = [ # `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" -# 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 +# The Sift pytest plugin is loaded so the project's own integration tests can +# use its fixtures. Unit-test runs are flipped to `--sift-disabled` mode by +# `lib/sift_client/_tests/conftest.py`. +# `--import-mode=importlib` loads test files by path with unique synthetic +# module names. The default `prepend` mode would try to import +# `lib/sift_client/_tests/conftest.py` as `sift_client._tests.conftest`, which +# fails because `_tests` is excluded from the wheel (see packages.find above). +addopts = "-p pytester -p sift_client.pytest_plugin --import-mode=importlib" +# The autouse gate defaults to off so unit tests don't use the Sift +# fixtures. The integration subtree (lib/sift_client/_tests/util/) opts +# back in via `pytest.mark.sift_include` applied in its conftest. +sift_autouse = false testpaths = [ "lib/sift_py", "lib/sift_client/_tests", diff --git a/python/scripts/dev b/python/scripts/dev index 5a1397803..81d5ec826 100755 --- a/python/scripts/dev +++ b/python/scripts/dev @@ -201,4 +201,5 @@ case "$1" in ;; esac -exit 0 +# Leave the script's exit code as the subcommand's. A trailing `exit 0` here +# silently masked ruff / mypy / pytest failures from the pre-push hook.