Python(feat): clean up the abort propagation and output location settings (#642)

Author: alexluck-sift

python/docs/guides/pytest_plugin/configuration.md

@@ -67,7 +67,7 @@ Prefer real environment variables (shell exports, CI secrets) for anything you
 can't keep in a local file.
 
 !!! warning "FedRAMP / shared environments"
-    Pass `--sift-log-file=false` (or set the ini key to `"false"`) to skip the
+    Pass `--no-sift-log-file` (or set `sift_log_file = false`) to skip the
     temp file + worker pipeline. Create/update calls then run inline against the
     API instead of being deferred through a subprocess.
 
@@ -146,8 +146,9 @@ suggestion, so typos like `SIFT_REPORT_SERIALNUM` surface immediately.
 
 | Setting | CLI flag | Ini (`[tool.pytest.ini_options]`) |
 |---|---|---|
-| Path to the JSONL log of create/update calls (path \| true \| false \| none). | `--sift-log-file` | `sift_log_file` |
-| DEBUG-level audit trace of plugin behavior (path \| true \| false). On by default to a temp file, with warnings echoed to stdout; set a path to pin the file, or false to disable. | `--sift-audit-log` | `sift_audit_log` |
+| Directory for this run's artifacts (JSONL log, audit trace). Each run gets its own random subfolder. Defaults to a temp directory. | `--sift-output-dir` | `sift_output_dir` |
+| Write the JSONL log of create/update calls. On by default; --no-sift-log-file disables it (incompatible with --sift-offline). | `--no-sift-log-file` | `sift_log_file` |
+| Write the DEBUG audit trace of plugin behavior, with warnings echoed to stdout. On by default; --no-sift-audit-log disables it. | `--no-sift-audit-log` | `sift_audit_log` |
 | Capture git repo/branch/commit on the report. | `--no-sift-git-metadata` | `sift_git_metadata` |
 | Skip the session-start ping; route create/update through the JSONL log. | `--sift-offline` | `sift_offline` |
 | Disable Sift entirely (no API calls, no log file). Supersedes --sift-offline. | `--sift-disabled` | `sift_disabled` |

python/docs/guides/pytest_plugin/index.md

@@ -66,6 +66,14 @@ A `TestReport` shows up in Sift once the session finishes.
     it does not short-circuit on the first failure and skip every measurement
     after it.
 
+!!! tip "Stopping the whole run early"
+    `pytest.fail()` fails a single test. To stop the session,
+    `pytest.exit("...")` ends it and rolls the report up as `FAILED`, while
+    `sift_client.pytest_plugin.abort("...")` ends it and rolls the report up as
+    `ABORTED`, for a system-level stop where the run was cut off rather than a
+    test failing (a real Ctrl-C does the same). See
+    [Pass/Fail Behavior](pass_fail_behavior.md#stopping-a-run-as-aborted).
+
 ## Sensible defaults
 
 With nothing but the `conftest.py` above, you get:

python/docs/guides/pytest_plugin/pass_fail_behavior.md

@@ -34,48 +34,72 @@ mapping to `FAILED`. A non-assertion exception gets its formatted traceback
 
 ## Hard exits
 
-Hard exits map to `ABORTED`. The step is resolved during fixture teardown, not
-at the instant of the exit:
-
-- When the exit produces a call-phase report (`sys.exit(1)`, `SystemExit`), the
-  plugin reads the status off that report.
-- When a `KeyboardInterrupt` aborts the session before any call-phase report
-  (Ctrl-C, or `raise KeyboardInterrupt` in the body), pytest still runs fixture
-  finalizers as it unwinds. The plugin sees setup completed with no call outcome
-  and resolves the cut-off step to `ABORTED` there.
-
-The status only reaches the report if those finalizers run. If the process is
-killed before they do (`SIGKILL`, the OOM killer, power loss), nothing is written
-and the step keeps the `IN_PROGRESS` it was created with. That is the only path
-that leaves a step `IN_PROGRESS` in a finalized report.
-
-| Scenario                                       | Trigger                            | Outcome                                          |
-| ---------------------------------------------- | ---------------------------------- | ------------------------------------------------ |
-| `SystemExit` from the test body                | `sys.exit(1)`                      | `ABORTED` (read from the call-phase report)      |
-| `KeyboardInterrupt` from the test body         | `raise KeyboardInterrupt`          | `ABORTED` (resolved during teardown)             |
-| Session-aborting `KeyboardInterrupt`           | Ctrl-C terminates pytest           | `ABORTED` (resolved during teardown)             |
-| Process killed before finalizers run           | `SIGKILL` / OOM / power loss       | `IN_PROGRESS` (nothing written after creation)   |
-
-### Abort propagation through nested substeps
-
-Every step that was open when the abort fired records
-`ABORTED`.
+A hard exit resolves the cut-off step to `ABORTED`, recorded while pytest runs
+fixture finalizers on the way out. What the containers (class, module, package)
+and the report resolve to depends on why the run stopped:
+
+- A **failure stop** rolls up `FAILED`. `pytest.exit()`, `sys.exit()` /
+  `SystemExit`, an `assert`, or an exception means the test ended the run on a
+  fault, so the exited step is `ABORTED` (or `FAILED`/`ERROR` for an
+  assert/exception) while its containers and the report read `FAILED`.
+- A **system stop** rolls up `ABORTED`. Ctrl-C / `KeyboardInterrupt`, or the
+  `abort()` helper below, means the run was cut off rather than a test failing,
+  so the exited step, its containers, and the report all read `ABORTED`.
+
+`SystemExit` is read from the call-phase report; the session-stopping exits abort
+before that report fires, so the step resolves during teardown instead. If the
+process dies before finalizers run (`SIGKILL`, OOM, power loss) nothing more is
+written and the step stays `IN_PROGRESS`, the only path that leaves a step
+`IN_PROGRESS` in a finished report.
+
+| Trigger                          | Exited step        | Containers + report |
+| -------------------------------- | ------------------ | ------------------- |
+| `assert` / exception             | `FAILED` / `ERROR` | `FAILED`            |
+| `sys.exit()` / `SystemExit`      | `ABORTED`          | `FAILED`            |
+| `pytest.exit("...")`             | `ABORTED`          | `FAILED`            |
+| `abort("...")` (Sift)            | `ABORTED`          | `ABORTED`           |
+| Ctrl-C / `KeyboardInterrupt`     | `ABORTED`          | `ABORTED`           |
+| process killed (`SIGKILL`/OOM)   | `IN_PROGRESS`      | `IN_PROGRESS`       |
+
+Within a stop, `ABORTED` is recorded on each step the exit unwinds through: the
+open substeps and the test step. A substep that closed before the exit keeps its
+own status.
 
 ```python title="test_abort.py"
 import sys
 
 
 def test_x(step):
     with step.substep(name="completed_sub"):
-        pass  # closes as PASSED before the abort
+        pass  # closed PASSED before the abort
     with step.substep(name="outer_sub") as outer_sub:
         with outer_sub.substep(name="inner_sub"):
-            sys.exit(1)  # ABORTED applied to inner_sub, outer_sub, and the test step
+            sys.exit(1)
+```
+
+`completed_sub` stays `PASSED`; `inner_sub`, `outer_sub`, and the test step are
+`ABORTED`. The enclosing module reads `FAILED`, since `sys.exit()` is a failure
+stop.
+
+### Stopping a run as aborted
+
+`sift_client.pytest_plugin.abort(reason)` stops the session and records the
+report and the open parent steps as `ABORTED` rather than `FAILED`. Use it for a
+system-level stop where the run was cut off rather than a test failing, such as
+the device under test losing power. A real Ctrl-C does the same automatically.
+
+```python
+from sift_client.pytest_plugin import abort
+
+
+def test_flash(step):
+    if not device_responding():
+        abort("device under test is not responding")
+    ...
 ```
 
-The Sift report shows `completed_sub` as `PASSED` and the three steps
-still open at the abort (`inner_sub`, `outer_sub`, and the test step
-itself) as `ABORTED`.
+For a stop that should read as a failure, use `pytest.exit()`; for a single
+failing test, use `pytest.fail()`.
 
 ## Skips
 
@@ -180,14 +204,20 @@ Every non-`PASSED`/`SKIPPED` step marks its parent as failed. What the
 parent records depends on whether its own scope had an abort and whether
 a child already failed:
 
-- A hard exit (`SystemExit` or an observed `KeyboardInterrupt`) in the
-  step's own scope records `ABORTED`. `ABORTED` propagates through every
-  step the abort passes through on its way up.
-- A child that already recorded a non-`PASSED`/`SKIPPED` outcome marks
-  the parent as `FAILED`. This holds whether or not an exception is still
-  propagating through the parent's scope: only the originating substep
-  records `ERROR`; ancestors inherit `FAILED`. The traceback stays on
-  the originating step's `error_info`.
+- A hard exit (`SystemExit` or an observed `KeyboardInterrupt`) records
+  `ABORTED` on the step in whose scope it fired, and `ABORTED` propagates
+  through every step the exception unwinds through on its way up: the
+  open substeps and the test step. Container parents (class, module,
+  package) are closed out-of-band rather than by the unwinding exception,
+  so they are not on that path. On a failure stop (`pytest.exit()`,
+  `sys.exit`) they inherit `FAILED` like any other non-pass child; on a
+  system stop (Ctrl-C / `KeyboardInterrupt`, or `abort()`) the run is flagged
+  aborted and they resolve `ABORTED` instead. See [Hard exits](#hard-exits).
+- A child that recorded a non-`PASSED`/`SKIPPED` outcome marks the parent
+  as `FAILED`. This holds whether or not an exception is still propagating
+  through the parent's scope: only the originating step records `ERROR` (or
+  `ABORTED`); ancestors that inherit the result take `FAILED`. The
+  traceback stays on the originating step's `error_info`.
 - A step records `ERROR` only when its own scope raised a non-Assertion
   exception AND no child has failed.
 

python/lib/sift_client/_internal/pytest_plugin/audit_log.py

@@ -2,9 +2,9 @@
 
 On by default: every session attaches two handlers to the ``sift_client`` root
 logger so plugin-behavior modules AND high-value SDK call sites land in one file
-(a temp file unless ``--sift-audit-log=<path>`` pins one), with warnings also
-echoed to stdout. Pass ``--sift-audit-log=false`` (or set ``sift_audit_log =
-"false"``) to turn it off. The replay subprocess gets its own sibling file via
+(in the run's ``--sift-output-dir``, or a temp dir), with warnings also echoed
+to stdout. Pass ``--no-sift-audit-log`` (or set ``sift_audit_log = false``) to
+turn it off. The replay subprocess gets its own sibling file via
 ``replay_audit_path``.
 
 Handlers are removed at session end (``pytest_unconfigure`` ->
@@ -110,41 +110,17 @@ def replay_audit_path(main_path: Path) -> Path:
     return main_path.with_suffix(".replay" + main_path.suffix)
 
 
-def audit_disabled(value: object) -> bool:
-    """Whether audit logging is explicitly turned off.
+def _make_session_dir(base: Path | None = None) -> Path:
+    """Create and return ``<base>/<random>/`` for this run's artifacts.
 
-    Default on: only ``False`` / ``"false"`` / ``"none"`` disables. Anything
-    else — unset, ``"true"``, or a path — leaves it enabled.
+    ``base`` is the ``--sift-output-dir`` directory when set, else
+    ``<tmpdir>/sift_test_results``. Each run gets its own random subfolder (from
+    ``tempfile.mkdtemp``) so repeated or concurrent runs never collide, and all
+    of a run's artifacts (JSONL log, tracking sidecar, audit log, replay audit
+    log) land inside it together.
     """
-    if value is False:
-        return True
-    return isinstance(value, str) and value.strip().lower() in ("false", "none")
-
-
-def explicit_audit_path(value: object) -> Path | None:
-    """The file path the user pinned, or ``None`` to use a temp default.
-
-    ``"true"`` / ``"1"`` / unset all mean "enabled, no specific path", so the
-    caller falls back to :func:`default_audit_path`.
-    """
-    if not isinstance(value, str):
-        return None
-    text = value.strip()
-    if text.lower() in ("", "true", "1", "false", "none"):
-        return None
-    return Path(text)
-
-
-def _make_session_dir() -> Path:
-    """Create and return ``<tmpdir>/sift_test_results/<random>/``.
-
-    All per-session temp artifacts (JSONL log, tracking sidecar, audit log,
-    replay audit log) land inside this directory so they're easy to locate and
-    clean up together. The random component comes from ``tempfile.mkdtemp`` —
-    the same OS-backed source used by ``NamedTemporaryFile``.
-    """
-    parent = Path(tempfile.gettempdir()) / "sift_test_results"
-    parent.mkdir(exist_ok=True)
+    parent = base if base is not None else Path(tempfile.gettempdir()) / "sift_test_results"
+    parent.mkdir(parents=True, exist_ok=True)
     return Path(tempfile.mkdtemp(dir=parent, prefix=""))
 
 
@@ -198,10 +174,9 @@ def configure_audit_logging(
     """
     from sift_client._internal.pytest_plugin.options import AUDIT_LOG_OPTION
 
-    raw = AUDIT_LOG_OPTION.resolve(config)
-    if audit_disabled(raw):
+    if not AUDIT_LOG_OPTION.resolve(config):
         return None
-    path = explicit_audit_path(raw) or default_audit_path(session_dir=session_dir)
+    path = default_audit_path(session_dir=session_dir)
     attach_file_handler(path)
     logger = logging.getLogger(ROOT_LOGGER)
     if not any(

python/lib/sift_client/_internal/pytest_plugin/options.py

@@ -217,22 +217,36 @@ def _walk_toml(data: dict[str, Any], path: tuple[str, ...]) -> Any:
 #     loads .env for local dev; CI sets the same names from its secret store).
 # ---------------------------------------------------------------------------
 
-# Pytest behavior. The CLI flag survives because the per-run override is real.
+# Pytest behavior. The CLI flags survive because the per-run override is real.
+OUTPUT_DIR_OPTION = Option(
+    name="output_dir",
+    category=CAT_BEHAVIOR,
+    help="Directory for this run's artifacts (JSONL log, audit trace). Each run gets "
+    "its own random subfolder. Defaults to a temp directory.",
+    cli="--sift-output-dir",
+    ini="sift_output_dir",
+)
 LOG_FILE_OPTION = Option(
     name="log_file",
     category=CAT_BEHAVIOR,
-    help="Path to the JSONL log of create/update calls (path | true | false | none).",
-    cli="--sift-log-file",
+    help="Write the JSONL log of create/update calls. On by default; "
+    "--no-sift-log-file disables it (incompatible with --sift-offline).",
+    cli="--no-sift-log-file",
+    cli_action="store_false",
     ini="sift_log_file",
+    ini_type="bool",
+    ini_default=True,
 )
 AUDIT_LOG_OPTION = Option(
     name="audit_log",
     category=CAT_BEHAVIOR,
-    help="DEBUG-level audit trace of plugin behavior (path | true | false). On by "
-    "default to a temp file, with warnings echoed to stdout; set a path to pin the "
-    "file, or false to disable.",
-    cli="--sift-audit-log",
+    help="Write the DEBUG audit trace of plugin behavior, with warnings echoed to "
+    "stdout. On by default; --no-sift-audit-log disables it.",
+    cli="--no-sift-audit-log",
+    cli_action="store_false",
     ini="sift_audit_log",
+    ini_type="bool",
+    ini_default=True,
 )
 GIT_METADATA_OPTION = Option(
     name="git_metadata",
@@ -406,6 +420,7 @@ def _walk_toml(data: dict[str, Any], path: tuple[str, ...]) -> Any:
 )
 
 PLUGIN_OPTIONS: tuple[Option, ...] = (
+    OUTPUT_DIR_OPTION,
     LOG_FILE_OPTION,
     AUDIT_LOG_OPTION,
     GIT_METADATA_OPTION,

python/lib/sift_client/_internal/pytest_plugin/report.py

@@ -55,7 +55,7 @@
 def resolve_real_report_id(context: Any) -> str | None:
     """Resolve the real server-side report id for the online footer link.
 
-    In synchronous online mode (``--sift-log-file=false``) the report is created
+    In synchronous online mode (``--no-sift-log-file``) the report is created
     directly against the API, so ``report.id_`` is already the real id. In the
     default incremental mode the report is created through the simulate path
     (a client-side UUID) and the background worker maps it to the real id on
@@ -377,41 +377,20 @@ def format_template(
         return fallback
 
 
-def resolve_log_file(pytestconfig: pytest.Config | None) -> str | Path | bool | None:
-    """Determine log_file value from CLI flag or ini key.
+def log_file_enabled(pytestconfig: pytest.Config | None) -> bool:
+    """Whether the JSONL log of create/update calls is written.
 
-    Three signal types arrive here:
-
-    * ``None``: unset; nothing was passed on the CLI and the ini key is
-      absent. Treat as the default "use a temp file."
-    * Python ``False``: an explicit disable, typically set in a conftest via
-      ``config.option.sift_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.
+    On by default; ``--no-sift-log-file`` disables it. Offline mode routes every
+    create/update call through the log as its only sink, so disabling the log
+    while offline is a usage error.
     """
-    raw = LOG_FILE_OPTION.resolve(pytestconfig)
-    disabled = raw is False or (isinstance(raw, str) and raw.lower() in ("false", "none"))
-    if disabled and is_offline(pytestconfig):
+    enabled = bool(LOG_FILE_OPTION.resolve(pytestconfig))
+    if not enabled 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=<path>, or "
-            "drop --sift-log-file=none to use a temp file."
+            "--no-sift-log-file is incompatible with --sift-offline; offline mode "
+            "requires the JSONL log as its only sink. Drop one of the two flags."
         )
-    if raw is False:
-        return None
-    if not raw:
-        return True
-    lower = str(raw).lower()
-    if lower in ("true", "1"):
-        return True
-    if lower in ("false", "none"):
-        return None
-    return Path(raw)
+    return enabled
 
 
 def report_context_impl(
@@ -458,15 +437,16 @@ def report_context_impl(
         "pytest_command": command,
     }
     # Mode → ReportContext flags:
-    #   online (default): log_file=<temp or user path>, replay_log_file=True
-    #   --sift-offline:   log_file=<temp or user path>, replay_log_file=False
-    #   --sift-disabled:  log_file=False,               replay_log_file=False
+    #   online (default): log_file=<path in session dir>, replay_log_file=True
+    #   --sift-offline:   log_file=<path in session dir>, replay_log_file=False
+    #   --sift-disabled / --no-sift-log-file: 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)
-    # When the log would use a default temp file and the plugin already created
-    # a session dir, pin the JSONL inside that dir so it lands alongside the
-    # audit log rather than having ReportContext mint a separate session dir.
+    log_file: str | Path | bool = False if disabled else log_file_enabled(pytestconfig)
+    # Place the JSONL inside the run's session dir so it lands alongside the
+    # audit log. pytest_configure created the dir whenever the log is enabled; if
+    # one isn't present (e.g. ReportContext used outside pytest), log_file stays
+    # True and ReportContext mints its own dir.
     if log_file is True and pytestconfig is not None:
         from sift_client.pytest_plugin import SIFT_SESSION_DIR_STASH_KEY