From 1240344a2879f366d8db868fa36060780ccbe94e Mon Sep 17 00:00:00 2001 From: nilsmechtel Date: Sun, 27 Sep 2026 03:26:58 +0200 Subject: [PATCH 1/3] test: gate live-cluster tests behind --live and announce resolved credentials `tests/conftest.py` calls `load_dotenv()` unconditionally, so the same pytest command means different things in the main checkout (which has a .env carrying HYPHA_TOKEN and BIOIMAGE_IO_TOKEN) and in a git worktree (which has none). The documented --ignore list does not exclude the affected tests: all 25 of tests/apps/model-runner/ deploy to and call bioimage-io on hypha.aicell.io, and they are inside the standard scope. Live tests are now deselected unless --live is passed, and any resolved credential is announced with its server and workspace before the first test runs, so the exposure is visible even when the flag is absent. Co-Authored-By: Claude Opus 5 (1M context) --- pytest.ini | 1 + tests/README.md | 9 +++ tests/conftest.py | 109 ++++++++++++++++++++++++++++- tests/test_live_gate.py | 150 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 267 insertions(+), 2 deletions(-) create mode 100644 tests/test_live_gate.py diff --git a/pytest.ini b/pytest.ini index 66ed1a36..18aaa402 100644 --- a/pytest.ini +++ b/pytest.ini @@ -4,6 +4,7 @@ markers = integration: marks tests that use real Ray or other subsystems unit: marks tests that use mocked external dependencies requires_gpu: marks tests that require a GPU runtime deployment to be running + live: marks tests that act on a real Hypha cluster; deselected unless --live is passed testpaths = tests python_files = test_*.py python_classes = Test* diff --git a/tests/README.md b/tests/README.md index 90f8bdc1..7addfdb5 100644 --- a/tests/README.md +++ b/tests/README.md @@ -19,6 +19,15 @@ pip install -r requirements-test.txt ### 3. Environment Configuration The `.env` file in the project root contains required environment variables including `HYPHA_TOKEN`. This is automatically loaded by the test configuration. +Because that load is automatic, whether a credential resolves depends on which checkout you start pytest from — a git worktree has no `.env`, the main checkout does. Tests that act on a real Hypha cluster are therefore deselected unless you pass `--live`, and whenever a credential does resolve the run prints the server and workspace it could reach before the first test. Both are independent of the `--ignore` list; `tests/apps/model-runner/` is live even though it is not under `tests/end_to_end/`. + +```bash +pytest tests/ # offline tests only +pytest tests/ --live # also runs tests that deploy to and call the live cluster +``` + +A test counts as live if it requests `hypha_client`, `hypha_token` or `model_runner`, or is marked `@pytest.mark.live`. Anything new that reaches a cluster by some other route has to carry the marker. + ## Running Tests ### All Tests diff --git a/tests/conftest.py b/tests/conftest.py index d1e7b684..1d3dbb74 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,6 +8,8 @@ """ import asyncio +import base64 +import json import os import re import subprocess @@ -15,7 +17,7 @@ import tempfile from datetime import datetime from pathlib import Path -from typing import AsyncGenerator, Generator +from typing import AsyncGenerator, Dict, Generator, List, Optional, Tuple import pytest import pytest_asyncio @@ -28,6 +30,109 @@ # Load environment variables from .env file load_dotenv() +# Every Hypha connection in the suite is hard-coded to this deployment; no +# environment variable redirects it. +HYPHA_SERVER_URL = "https://hypha.aicell.io" + +# Requesting one of these fixtures means the test authenticates against the +# real deployment above. Anything else that reaches a cluster has to say so +# with @pytest.mark.live. +LIVE_FIXTURES = frozenset({"hypha_client", "hypha_token", "model_runner"}) + +# Credentials the suite picks up on its own, in the order a reader should +# worry about them. `load_dotenv()` above supplies these from a repo-root +# .env, so whether they resolve depends on which checkout pytest was run from. +LIVE_TOKEN_VARS = ("HYPHA_TOKEN", "BIOIMAGE_IO_TOKEN") + + +def pytest_addoption(parser: pytest.Parser) -> None: + parser.addoption( + "--live", + action="store_true", + default=False, + help=( + "Run tests that deploy to and call a real Hypha cluster. Without " + "this flag they are deselected even when a token is available." + ), + ) + + +def _token_workspace(token: str) -> str: + """Workspace a Hypha JWT is scoped to, or '' if it is not a readable JWT.""" + try: + segment = token.split(".")[1] + claims = json.loads( + base64.urlsafe_b64decode(segment + "=" * (-len(segment) % 4)) + ) + except Exception: + return "" + for part in str(claims.get("scope", "")).split(): + if part.startswith("wid:"): + return part[len("wid:") :] + return "" + + +def resolved_live_targets( + environ: Optional[Dict[str, str]] = None, +) -> List[Tuple[str, str]]: + """(variable, workspace) for every cluster credential visible to this run.""" + environ = os.environ if environ is None else environ + return [ + (var, _token_workspace(environ[var]) or "") + for var in LIVE_TOKEN_VARS + if environ.get(var) + ] + + +def live_exposure_banner( + targets: List[Tuple[str, str]], live_count: int, enabled: bool +) -> str: + """The text printed before the first test when a cluster credential resolves.""" + lines = [ + f"{live_count} collected test(s) can reach the live cluster at " + f"{HYPHA_SERVER_URL}, and a credential for it resolved:" + ] + lines += [f" {var} -> workspace {workspace}" for var, workspace in targets] + lines.append( + f"THEY WILL RUN: --live was given, so {live_count} test(s) will act on " + "that workspace." + if enabled + else f"Deselected {live_count} test(s); pass --live to run them." + ) + return "\n".join(lines) + + +def _is_live(item: pytest.Item) -> bool: + if any(item.iter_markers("live")): + return True + return bool(LIVE_FIXTURES.intersection(getattr(item, "fixturenames", ()))) + + +def pytest_collection_modifyitems( + config: pytest.Config, items: List[pytest.Item] +) -> None: + live, offline = [], [] + for item in items: + (live if _is_live(item) else offline).append(item) + + enabled = config.getoption("--live") + if live and not enabled: + items[:] = offline + config.hook.pytest_deselected(items=live) + + targets = resolved_live_targets() + if not targets: + return + + banner = live_exposure_banner(targets, len(live), enabled) + reporter = config.pluginmanager.get_plugin("terminalreporter") + if reporter is None: + print(banner) + return + reporter.write_sep("=", "live cluster credentials resolved", red=True) + reporter.write_line(banner) + reporter.write_sep("=", red=True) + @pytest.fixture( scope="session", @@ -192,7 +297,7 @@ def head_node_port(ray_address: str) -> int: @pytest.fixture(scope="session") def server_url() -> str: """Return Hypha server URL for test connections.""" - return "https://hypha.aicell.io" + return HYPHA_SERVER_URL @pytest.fixture(scope="session") diff --git a/tests/test_live_gate.py b/tests/test_live_gate.py new file mode 100644 index 00000000..410bfdc3 --- /dev/null +++ b/tests/test_live_gate.py @@ -0,0 +1,150 @@ +"""The gate that keeps a stray .env from pointing the suite at production. + +`tests/conftest.py` calls `load_dotenv()` unconditionally, so whether a Hypha +credential resolves depends on which checkout pytest was started from. These +tests pin the two consequences of that: live tests are deselected unless +``--live`` is passed, and a resolved credential is announced before the first +test runs. + +The sub-runs below use ``--collect-only``, so nothing here contacts a cluster. +""" + +import base64 +import functools +import json +import os +import subprocess +import sys +from pathlib import Path +from typing import Dict, List, Tuple + +from tests.conftest import ( + HYPHA_SERVER_URL, + _token_workspace, + live_exposure_banner, + resolved_live_targets, +) + +REPO_ROOT = Path(__file__).resolve().parent.parent + +# One live test in tests/apps/model-runner/, by node name. +A_LIVE_TEST = "test_search_models_returns_list" + + +def _fake_jwt(workspace: str) -> str: + claims = {"scope": f"ws:{workspace}#a wid:{workspace}", "sub": "auth0|test"} + payload = base64.urlsafe_b64encode(json.dumps(claims).encode()).decode() + return f"header.{payload.rstrip('=')}.signature" + + +def _collect(extra_args: List[str], env_overrides: Dict[str, str]) -> str: + return _collect_cached(tuple(extra_args), tuple(sorted(env_overrides.items()))) + + +@functools.lru_cache(maxsize=None) +def _collect_cached( + extra_args: Tuple[str, ...], overrides: Tuple[Tuple[str, str], ...] +) -> str: + """Collect tests/apps/model-runner in a sub-run and return its output. + + Both token variables are always set explicitly: python-dotenv does not + override a variable that is already in the environment, so passing an + empty string is what actually neutralises the repo-root .env. + """ + env = { + **os.environ, + "HYPHA_TOKEN": "", + "BIOIMAGE_IO_TOKEN": "", + **dict(overrides), + } + result = subprocess.run( + [ + sys.executable, + "-m", + "pytest", + "-o", + "addopts=", + "-p", + "no:cacheprovider", + "--collect-only", + "-q", + "tests/apps/model-runner", + *extra_args, + ], + cwd=REPO_ROOT, + env=env, + capture_output=True, + text=True, + ) + # 5 is EXIT_NOTESTSCOLLECTED, which is exactly what a fully deselected + # scope produces. + assert result.returncode in (0, 5), result.stdout + result.stderr + return result.stdout + + +def test_token_workspace_reads_the_wid_scope() -> None: + assert _token_workspace(_fake_jwt("bioimage-io")) == "bioimage-io" + + +def test_token_workspace_tolerates_a_non_jwt() -> None: + assert _token_workspace("not-a-jwt") == "" + + +def test_resolved_live_targets_is_empty_without_a_credential() -> None: + assert resolved_live_targets({"HYPHA_TOKEN": "", "BIOIMAGE_IO_TOKEN": ""}) == [] + + +def test_resolved_live_targets_names_every_credential_in_scope() -> None: + assert resolved_live_targets( + { + "HYPHA_TOKEN": _fake_jwt("ws-user-github|1"), + "BIOIMAGE_IO_TOKEN": _fake_jwt("bioimage-io"), + } + ) == [("HYPHA_TOKEN", "ws-user-github|1"), ("BIOIMAGE_IO_TOKEN", "bioimage-io")] + + +def test_banner_names_the_server_the_workspace_and_the_count() -> None: + banner = live_exposure_banner([("BIOIMAGE_IO_TOKEN", "bioimage-io")], 25, False) + assert HYPHA_SERVER_URL in banner + assert "bioimage-io" in banner + assert "25" in banner + + +def test_banner_distinguishes_deselected_from_about_to_run() -> None: + targets = [("BIOIMAGE_IO_TOKEN", "bioimage-io")] + assert "WILL RUN" in live_exposure_banner(targets, 25, True) + assert "WILL RUN" not in live_exposure_banner(targets, 25, False) + + +def test_every_resolved_credential_is_announced_before_the_first_test() -> None: + stdout = _collect( + [], + { + "HYPHA_TOKEN": _fake_jwt("ws-user-github|1"), + # The model-runner gate prefers this one, so a run can reach + # production even with HYPHA_TOKEN scoped somewhere harmless. + "BIOIMAGE_IO_TOKEN": _fake_jwt("bioimage-io"), + }, + ) + assert "live cluster credentials resolved" in stdout + assert "HYPHA_TOKEN -> workspace ws-user-github|1" in stdout + assert "BIOIMAGE_IO_TOKEN -> workspace bioimage-io" in stdout + assert HYPHA_SERVER_URL in stdout + assert "25 collected test(s) can reach" in stdout + + +def test_nothing_is_announced_when_no_credential_resolves() -> None: + stdout = _collect([], {}) + assert "live cluster credentials resolved" not in stdout + + +def test_live_tests_are_deselected_by_default() -> None: + stdout = _collect([], {}) + assert A_LIVE_TEST not in stdout + assert "25 deselected" in stdout + + +def test_live_tests_are_selected_with_the_flag() -> None: + stdout = _collect(["--live"], {}) + assert A_LIVE_TEST in stdout + assert "deselected" not in stdout From 88c78cb048b7ac6315801fe3e142440fd59b1ccc Mon Sep 17 00:00:00 2001 From: nilsmechtel Date: Sun, 27 Sep 2026 04:15:40 +0200 Subject: [PATCH 2/3] test: make the live-test announcement survive xdist, and gate two files it missed Follow-up to the --live gate. Six defects, each verified: The banner never fired on the default invocation. `addopts` carries `--numprocesses=1`, so collection runs inside an xdist worker whose terminalreporter output is discarded and whose deselection stats never reach the controller: measured 0 banner lines and no deselected count. Every sub-run in the new tests passed `-o addopts=`, which is exactly why none of them caught it. A worker's stderr *is* inherited by the controller, so the banner goes there instead, and two tests now run under pytest.ini as shipped. tests/apps/cellpose/test_ui_e2e.py drives a browser at the deployed app and injects HYPHA_TOKEN into localStorage, but uses only the `page` fixture, so nothing gated it. tests/test_artifact_version.py calls upload_app and deletes artifacts on bioimage-io/bioengine-worker; the fixture-name rule caught it only because it happens to name its own fixture `hypha_client`. Both now carry an explicit module marker. The live set in the standard scope is 29, not 25; tree-wide it is 80. A token whose payload decodes to a JSON scalar raised AttributeError out of a collection hook, which pytest turns into INTERNALERROR: the scope lookup sat outside the try guarding the decode. The count is now restated next to the wall clock at the end of the run, with or without --live. The flag only stops the accidental case; once someone has opted in deliberately, the count and the runtime in the log are the only things that make an after-the-fact audit possible. `pytest tests/end_to_end/ -v` in the maintainer skill and the command in the model-runner conftest docstring both became silent no-ops; both now carry --live. The README claimed a git worktree has no .env. find_dotenv() walks up, so a worktree under /.claude/worktrees/ reaches the repo root's and is not protected; one outside the repo tree is. Neither protects against a credential exported into the shell, which is the only clause that holds in every case, so that is the one the README leads with. Co-Authored-By: Claude Opus 5 (1M context) --- .claude/skills/bioengine-maintainer/SKILL.md | 9 +- tests/README.md | 18 ++- tests/apps/cellpose/test_ui_e2e.py | 5 + tests/apps/model-runner/conftest.py | 5 +- tests/conftest.py | 85 +++++++++-- tests/test_artifact_version.py | 8 +- tests/test_live_gate.py | 147 +++++++++++++++++-- 7 files changed, 242 insertions(+), 35 deletions(-) diff --git a/.claude/skills/bioengine-maintainer/SKILL.md b/.claude/skills/bioengine-maintainer/SKILL.md index 8c4a36a5..2b7b10da 100644 --- a/.claude/skills/bioengine-maintainer/SKILL.md +++ b/.claude/skills/bioengine-maintainer/SKILL.md @@ -118,9 +118,16 @@ Local artifact development: `export BIOENGINE_LOCAL_ARTIFACT_PATH=/path/to/bioen ### Run tests ```bash -pytest tests/end_to_end/ -v +pytest tests/ # offline tests only +pytest tests/end_to_end/ -v --live # deploys to and calls a real cluster ``` +Tests that act on a live cluster are deselected without `--live`, so the +end-to-end command above exits 5 ("no tests collected") if you omit it. +Whenever a credential resolves, the run prints the server, the workspace each +token is scoped to, and how many live-reaching tests are enabled — with or +without `--live`, so a finished run's log says what it could have touched. + Test organisation: - `tests/end_to_end/` — integration tests for the core worker - `tests/apps/` — per-app tests, one subdirectory per app (`tests/apps/cellpose/`, …) diff --git a/tests/README.md b/tests/README.md index 7addfdb5..f5fbc246 100644 --- a/tests/README.md +++ b/tests/README.md @@ -19,14 +19,26 @@ pip install -r requirements-test.txt ### 3. Environment Configuration The `.env` file in the project root contains required environment variables including `HYPHA_TOKEN`. This is automatically loaded by the test configuration. -Because that load is automatic, whether a credential resolves depends on which checkout you start pytest from — a git worktree has no `.env`, the main checkout does. Tests that act on a real Hypha cluster are therefore deselected unless you pass `--live`, and whenever a credential does resolve the run prints the server and workspace it could reach before the first test. Both are independent of the `--ignore` list; `tests/apps/model-runner/` is live even though it is not under `tests/end_to_end/`. +### 4. Tests that act on a live cluster + +Some tests deploy applications to, and call services on, the production workspaces at `https://hypha.aicell.io`. They are deselected unless you pass `--live`: ```bash pytest tests/ # offline tests only -pytest tests/ --live # also runs tests that deploy to and call the live cluster +pytest tests/ --live # also runs tests that act on the live cluster ``` -A test counts as live if it requests `hypha_client`, `hypha_token` or `model_runner`, or is marked `@pytest.mark.live`. Anything new that reaches a cluster by some other route has to carry the marker. +A test counts as live if it requests `hypha_client`, `hypha_token` or `model_runner`, or is marked `@pytest.mark.live`. Anything that reaches a cluster by some other route — a browser driven at the deployed app, a client built inline — has to carry the marker. Liveness has nothing to do with the `--ignore` lists in circulation: `tests/apps/model-runner/` and `tests/test_artifact_version.py` are live and neither lives under `tests/end_to_end/`. + +**Do not rely on your working directory to keep a credential away from the suite.** There are at least three ways one arrives, and only the last is under your control: + +- `load_dotenv()` above searches **upward** from `tests/`, not just the directory you started in. A worktree under `/.claude/worktrees/` has no `.env` of its own but still reaches the repo root's, so it is *not* protected. A worktree outside the repo tree (say under `/tmp`) does stop the walk. +- `tests/test_artifact_version.py` loads the repo-root `.env` by explicit path, wherever you run it from. +- `source .env`, `export HYPHA_TOKEN=…`, or a token already in your shell defeats all of the above regardless of location. + +Running inside the worker image with only the repo bind-mounted also stops the upward walk, because the search cannot escape the mount. + +Because none of that is reliable, the gate does not depend on it. Whenever a credential resolves, the run prints — before the first test, and again next to the wall clock at the end — which server it is, which workspace each token is scoped to, and how many live-reaching tests are enabled. That last number is printed **with or without `--live`**: the flag only prevents the accidental case, so once someone has opted in deliberately the count in the log is what makes an after-the-fact audit possible. Runtime is the corroborating signal; the same scope takes roughly 30s offline and several minutes against a cluster. ## Running Tests diff --git a/tests/apps/cellpose/test_ui_e2e.py b/tests/apps/cellpose/test_ui_e2e.py index ef1a576a..a37d0b48 100644 --- a/tests/apps/cellpose/test_ui_e2e.py +++ b/tests/apps/cellpose/test_ui_e2e.py @@ -6,6 +6,11 @@ import pytest from playwright.sync_api import Page, expect +# Every test here drives a browser at the deployed app and injects HYPHA_TOKEN +# into its localStorage. The `page` fixture is not in LIVE_FIXTURES, so the +# marker is what gates them. +pytestmark = pytest.mark.live + SERVER_URL = "https://hypha.aicell.io" APP_WORKSPACE = os.environ.get("HYPHA_TEST_WORKSPACE", "ri-scale") APP_URL = f"{SERVER_URL}/{APP_WORKSPACE}/view/cellpose-finetuning" diff --git a/tests/apps/model-runner/conftest.py b/tests/apps/model-runner/conftest.py index 10acde07..b2b39d25 100644 --- a/tests/apps/model-runner/conftest.py +++ b/tests/apps/model-runner/conftest.py @@ -3,7 +3,10 @@ Requires the model-runner app to be deployed to bioimage-io/bioengine-worker. Set BIOIMAGE_IO_TOKEN (or HYPHA_TOKEN) in the environment before running. - pytest tests/apps/model-runner/ -v -o "addopts=" +These call the live service, so they need --live; without it they are +deselected and the command exits 5 with no tests collected. + + pytest tests/apps/model-runner/ -v -o "addopts=" --live """ import io diff --git a/tests/conftest.py b/tests/conftest.py index 1d3dbb74..a97c6c8e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,6 +15,7 @@ import subprocess import sys import tempfile +import time from datetime import datetime from pathlib import Path from typing import AsyncGenerator, Dict, Generator, List, Optional, Tuple @@ -44,6 +45,8 @@ # .env, so whether they resolve depends on which checkout pytest was run from. LIVE_TOKEN_VARS = ("HYPHA_TOKEN", "BIOIMAGE_IO_TOKEN") +_START_TIME = pytest.StashKey[float]() + def pytest_addoption(parser: pytest.Parser) -> None: parser.addoption( @@ -64,9 +67,12 @@ def _token_workspace(token: str) -> str: claims = json.loads( base64.urlsafe_b64decode(segment + "=" * (-len(segment) % 4)) ) + scope = str(claims.get("scope", "")) except Exception: + # A token we cannot parse must not abort the session; the caller + # reports the credential as present with an unknown workspace. return "" - for part in str(claims.get("scope", "")).split(): + for part in scope.split(): if part.startswith("wid:"): return part[len("wid:") :] return "" @@ -87,10 +93,16 @@ def resolved_live_targets( def live_exposure_banner( targets: List[Tuple[str, str]], live_count: int, enabled: bool ) -> str: - """The text printed before the first test when a cluster credential resolves.""" + """The text printed before the first test when a cluster credential resolves. + + The count is stated whether or not --live was given: the flag only stops + the accidental case, and once someone has opted in deliberately the count + in the log is the only thing that makes an after-the-fact audit possible. + """ lines = [ + "=" * 22 + " live cluster credentials resolved " + "=" * 22, f"{live_count} collected test(s) can reach the live cluster at " - f"{HYPHA_SERVER_URL}, and a credential for it resolved:" + f"{HYPHA_SERVER_URL}, and a credential for it resolved:", ] lines += [f" {var} -> workspace {workspace}" for var, workspace in targets] lines.append( @@ -99,21 +111,51 @@ def live_exposure_banner( if enabled else f"Deselected {live_count} test(s); pass --live to run them." ) + lines.append("=" * 78) return "\n".join(lines) +def _emit(config: pytest.Config, text: str) -> None: + """Write where the controller will actually see it. + + Under pytest-xdist -- which `addopts` turns on with `--numprocesses=1` -- + collection runs inside a worker whose terminalreporter output is dropped + and whose deselection stats never reach the summary. The worker's stderr + is inherited by the controller, so that is the channel that survives. + """ + worker = getattr(config, "workerinput", None) + if worker is not None: + if worker.get("workerid", "gw0") == "gw0": + sys.stderr.write(text + "\n") + sys.stderr.flush() + return + + reporter = config.pluginmanager.get_plugin("terminalreporter") + if reporter is None: + print(text) + else: + reporter.write_line(text) + + def _is_live(item: pytest.Item) -> bool: if any(item.iter_markers("live")): return True return bool(LIVE_FIXTURES.intersection(getattr(item, "fixturenames", ()))) +@pytest.hookimpl(tryfirst=True) def pytest_collection_modifyitems( config: pytest.Config, items: List[pytest.Item] ) -> None: + # tryfirst so the markers below land before pytest's own -m filtering, + # which would otherwise see only the explicitly declared ones. live, offline = [], [] for item in items: - (live if _is_live(item) else offline).append(item) + if _is_live(item): + item.add_marker(pytest.mark.live) + live.append(item) + else: + offline.append(item) enabled = config.getoption("--live") if live and not enabled: @@ -121,17 +163,34 @@ def pytest_collection_modifyitems( config.hook.pytest_deselected(items=live) targets = resolved_live_targets() - if not targets: - return + if targets: + _emit(config, live_exposure_banner(targets, len(live), enabled)) - banner = live_exposure_banner(targets, len(live), enabled) - reporter = config.pluginmanager.get_plugin("terminalreporter") - if reporter is None: - print(banner) + +def pytest_configure(config: pytest.Config) -> None: + config.stash[_START_TIME] = time.time() + + +def pytest_terminal_summary( + terminalreporter, exitstatus: int, config: pytest.Config +) -> None: + """Restate the exposure next to the wall clock. + + Runtime is the cheapest signal that live tests ran -- the same command + takes ~30s offline and several minutes against a cluster -- so an audit + needs the credential and the duration in one place. + """ + targets = resolved_live_targets() + if not targets: return - reporter.write_sep("=", "live cluster credentials resolved", red=True) - reporter.write_line(banner) - reporter.write_sep("=", red=True) + elapsed = time.time() - config.stash.get(_START_TIME, time.time()) + workspaces = ", ".join(workspace for _, workspace in targets) + terminalreporter.write_line( + f"live cluster exposure: credentials for {workspaces} on " + f"{HYPHA_SERVER_URL} were in scope for this {elapsed:.1f}s run " + f"({'--live GIVEN' if config.getoption('--live') else '--live not given'}).", + red=True, + ) @pytest.fixture( diff --git a/tests/test_artifact_version.py b/tests/test_artifact_version.py index 8cc8cc63..b68975e4 100644 --- a/tests/test_artifact_version.py +++ b/tests/test_artifact_version.py @@ -10,10 +10,14 @@ 3. Re-saving the same version → rejected (would overwrite a published release). 4. Saving an older version → rejected. +These act on ``bioimage-io/bioengine-worker`` in production: they call +``upload_app`` and delete the artifacts again. They are the most destructive +tests in the suite, hence the module-wide ``live`` marker. + Run with: conda activate bioengine source .env - pytest tests/test_artifact_version.py -v + pytest tests/test_artifact_version.py -v --live """ import os @@ -26,6 +30,8 @@ load_dotenv(Path(__file__).parent.parent / ".env") +pytestmark = pytest.mark.live + # ── helpers ──────────────────────────────────────────────────────────────────── diff --git a/tests/test_live_gate.py b/tests/test_live_gate.py index 410bfdc3..9d32c29b 100644 --- a/tests/test_live_gate.py +++ b/tests/test_live_gate.py @@ -1,16 +1,20 @@ """The gate that keeps a stray .env from pointing the suite at production. -`tests/conftest.py` calls `load_dotenv()` unconditionally, so whether a Hypha -credential resolves depends on which checkout pytest was started from. These -tests pin the two consequences of that: live tests are deselected unless -``--live`` is passed, and a resolved credential is announced before the first -test runs. - -The sub-runs below use ``--collect-only``, so nothing here contacts a cluster. +`tests/conftest.py` calls `load_dotenv()` unconditionally and `find_dotenv()` +walks *up*, so a credential can arrive without anyone asking for it. These +tests pin the three consequences: live tests are deselected unless ``--live`` +is passed, every resolved credential is announced before the first test with +the count it enables, and that announcement survives the xdist worker that +``pytest.ini``'s ``addopts`` puts collection inside. + +Sub-runs either deselect everything or use ``--collect-only``, and the tokens +are synthetic, so nothing here contacts a cluster. """ +import ast import base64 import functools +import importlib.util import json import os import subprocess @@ -18,6 +22,8 @@ from pathlib import Path from typing import Dict, List, Tuple +import pytest + from tests.conftest import ( HYPHA_SERVER_URL, _token_workspace, @@ -30,6 +36,14 @@ # One live test in tests/apps/model-runner/, by node name. A_LIVE_TEST = "test_search_models_returns_list" +# pytest.ini's addopts carries --numprocesses=1, so a sub-run that honours it +# needs xdist. It is in requirements-test.txt; ad-hoc runners sometimes +# install a narrower set. +needs_shipped_addopts = pytest.mark.skipif( + importlib.util.find_spec("xdist") is None, + reason="pytest-xdist absent, so pytest.ini's addopts cannot be honoured", +) + def _fake_jwt(workspace: str) -> str: claims = {"scope": f"ws:{workspace}#a wid:{workspace}", "sub": "auth0|test"} @@ -38,18 +52,36 @@ def _fake_jwt(workspace: str) -> str: def _collect(extra_args: List[str], env_overrides: Dict[str, str]) -> str: - return _collect_cached(tuple(extra_args), tuple(sorted(env_overrides.items()))) + """A sub-run with addopts neutralised -- the fast path, for selection logic.""" + return _run_cached( + ("-o", "addopts=", "--collect-only", "-q", *extra_args), + tuple(sorted(env_overrides.items())), + ) + + +def _run_with_default_addopts( + extra_args: List[str], env_overrides: Dict[str, str] +) -> str: + """A sub-run under pytest.ini as shipped. + + `addopts` carries `--numprocesses=1`, so collection happens inside an + xdist worker whose terminalreporter output is discarded. Passing + `-o addopts=` -- as every other sub-run here does -- hides that entirely, + which is why this variant exists. + """ + return _run_cached(tuple(extra_args), tuple(sorted(env_overrides.items()))) @functools.lru_cache(maxsize=None) -def _collect_cached( +def _run_cached( extra_args: Tuple[str, ...], overrides: Tuple[Tuple[str, str], ...] ) -> str: - """Collect tests/apps/model-runner in a sub-run and return its output. + """Run pytest over tests/apps/model-runner and return its combined output. Both token variables are always set explicitly: python-dotenv does not override a variable that is already in the environment, so passing an - empty string is what actually neutralises the repo-root .env. + empty string is what actually neutralises the repo-root .env -- which + `load_dotenv()` finds by walking up, even from a worktree. """ env = { **os.environ, @@ -62,12 +94,8 @@ def _collect_cached( sys.executable, "-m", "pytest", - "-o", - "addopts=", "-p", "no:cacheprovider", - "--collect-only", - "-q", "tests/apps/model-runner", *extra_args, ], @@ -79,7 +107,7 @@ def _collect_cached( # 5 is EXIT_NOTESTSCOLLECTED, which is exactly what a fully deselected # scope produces. assert result.returncode in (0, 5), result.stdout + result.stderr - return result.stdout + return result.stdout + result.stderr def test_token_workspace_reads_the_wid_scope() -> None: @@ -148,3 +176,90 @@ def test_live_tests_are_selected_with_the_flag() -> None: stdout = _collect(["--live"], {}) assert A_LIVE_TEST in stdout assert "deselected" not in stdout + + +@needs_shipped_addopts +def test_the_banner_survives_the_default_addopts() -> None: + """pytest.ini runs collection inside an xdist worker; the banner must + still reach the controller, count and all.""" + output = _run_with_default_addopts([], {"HYPHA_TOKEN": _fake_jwt("bioimage-io")}) + assert "live cluster credentials resolved" in output + assert "HYPHA_TOKEN -> workspace bioimage-io" in output + assert "25 collected test(s) can reach" in output + + +@needs_shipped_addopts +def test_the_wall_clock_summary_states_the_exposure_even_with_the_flag() -> None: + """--live stops nothing being reported: the count and the runtime are the + only audit trail left once someone has opted in on purpose.""" + output = _run_with_default_addopts( + ["--live", "--collect-only"], {"HYPHA_TOKEN": _fake_jwt("bioimage-io")} + ) + assert "25 collected test(s) can reach" in output + assert "live cluster exposure:" in output + assert "--live GIVEN" in output + + +def test_the_artifact_version_tests_are_gated() -> None: + """Four tests that call upload_app and delete artifacts on + bioimage-io/bioengine-worker. Their fixture asserts on the token rather + than skipping, so without the gate they error rather than skip.""" + result = subprocess.run( + [ + sys.executable, + "-m", + "pytest", + "-o", + "addopts=", + "-p", + "no:cacheprovider", + "--collect-only", + "-q", + "tests/test_artifact_version.py", + ], + cwd=REPO_ROOT, + env={**os.environ, "HYPHA_TOKEN": "", "BIOIMAGE_IO_TOKEN": ""}, + capture_output=True, + text=True, + ) + assert "4 deselected" in result.stdout, result.stdout + + +def _module_pytestmarks(relpath: str) -> List[str]: + tree = ast.parse((REPO_ROOT / relpath).read_text()) + return [ + ast.unparse(node.value) + for node in tree.body + if isinstance(node, ast.Assign) + and any(isinstance(t, ast.Name) and t.id == "pytestmark" for t in node.targets) + ] + + +@pytest.mark.parametrize( + "relpath", + [ + # Drives a browser at the deployed app and injects HYPHA_TOKEN into + # its localStorage. Uses only the `page` fixture, so the marker is + # the only thing that gates it. Checked on the source rather than by + # collection because playwright is not installed here -- which is + # also why it collects as an import error, and why nobody noticed. + "tests/apps/cellpose/test_ui_e2e.py", + # Calls upload_app and deletes artifacts on bioimage-io/bioengine-worker. + # The fixture-name rule catches these too, but only because the file + # happens to call its own fixture `hypha_client`; rename it and the + # gate silently stops applying. The marker is what makes it deliberate, + # so assert on the marker and not just on the deselection below. + "tests/test_artifact_version.py", + ], +) +def test_the_modules_a_marker_alone_can_gate_carry_one(relpath: str) -> None: + assert _module_pytestmarks(relpath) == ["pytest.mark.live"] + + +def test_a_malformed_token_does_not_abort_the_session() -> None: + """A payload decoding to a JSON scalar used to raise AttributeError out of + a collection hook, which pytest turns into INTERNALERROR.""" + scalar_payload = base64.urlsafe_b64encode(b"123").decode().rstrip("=") + output = _collect([], {"HYPHA_TOKEN": f"header.{scalar_payload}.sig"}) + assert "INTERNALERROR" not in output + assert "HYPHA_TOKEN -> workspace " in output From ca2114e2f118eb58f2b109bcb110037fcf14a61f Mon Sep 17 00:00:00 2001 From: nilsmechtel Date: Sun, 27 Sep 2026 04:46:22 +0200 Subject: [PATCH 3/3] test: pin that -m live resolves the same set --live gates Attaching the marker in pytest_collection_modifyitems is what makes `-m live` agree with the gate: most live tests are detected from their fixture closure, so without it they are gated but unnamed and `-m live --live` returns only the modules that declare the marker themselves -- 4 of 29 on a mixed scope. That line had no test; deleting it left the whole gate suite green. The assertion compares the count `-m live --live` selects against the count the baseline deselects, rather than a literal, so it stays correct as live tests are added. Verified it kills both the missing attachment and a trylast reordering. Dropping tryfirst alone survives, because a conftest hookimpl already runs before pytest's mark plugin under pluggy's reverse-registration order -- so the comment now calls it a guarantee against a future earlier-registered plugin rather than the fix, which is what the mutation actually shows. Also pins that -m live without the flag selects nothing, so naming the marker cannot become a way past the gate. Co-Authored-By: Claude Opus 5 (1M context) --- tests/conftest.py | 9 ++++-- tests/test_live_gate.py | 67 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index a97c6c8e..adb9c98a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -147,8 +147,13 @@ def _is_live(item: pytest.Item) -> bool: def pytest_collection_modifyitems( config: pytest.Config, items: List[pytest.Item] ) -> None: - # tryfirst so the markers below land before pytest's own -m filtering, - # which would otherwise see only the explicitly declared ones. + # Attaching the marker is what makes `-m live` agree with `--live`: most + # live tests are detected from their fixture closure, so without this they + # are gated but unnamed and `-m live` returns a wrong subset. It has to + # happen before pytest's own mark filtering -- which a conftest hookimpl + # already does under pluggy's reverse-registration order, so tryfirst is a + # guarantee against a plugin registering an earlier modifyitems, not a + # load-bearing fix. Switching it to trylast does break the invariant. live, offline = [], [] for item in items: if _is_live(item): diff --git a/tests/test_live_gate.py b/tests/test_live_gate.py index 9d32c29b..dd48da73 100644 --- a/tests/test_live_gate.py +++ b/tests/test_live_gate.py @@ -17,6 +17,7 @@ import importlib.util import json import os +import re import subprocess import sys from pathlib import Path @@ -33,9 +34,16 @@ REPO_ROOT = Path(__file__).resolve().parent.parent +MODEL_RUNNER = "tests/apps/model-runner" + # One live test in tests/apps/model-runner/, by node name. A_LIVE_TEST = "test_search_models_returns_list" +# A scope holding all three kinds at once: live by fixture closure, live by +# module marker, and offline. The marker test below needs the mix, because +# the failure it guards is a marker set that covers only the second kind. +MIXED_SCOPE = (MODEL_RUNNER, "tests/test_artifact_version.py", "tests/_app") + # pytest.ini's addopts carries --numprocesses=1, so a sub-run that honours it # needs xdist. It is in requirements-test.txt; ad-hoc runners sometimes # install a narrower set. @@ -51,11 +59,16 @@ def _fake_jwt(workspace: str) -> str: return f"header.{payload.rstrip('=')}.signature" -def _collect(extra_args: List[str], env_overrides: Dict[str, str]) -> str: +def _collect( + extra_args: List[str], + env_overrides: Dict[str, str], + scope: Tuple[str, ...] = (MODEL_RUNNER,), +) -> str: """A sub-run with addopts neutralised -- the fast path, for selection logic.""" return _run_cached( ("-o", "addopts=", "--collect-only", "-q", *extra_args), tuple(sorted(env_overrides.items())), + scope, ) @@ -69,14 +82,18 @@ def _run_with_default_addopts( `-o addopts=` -- as every other sub-run here does -- hides that entirely, which is why this variant exists. """ - return _run_cached(tuple(extra_args), tuple(sorted(env_overrides.items()))) + return _run_cached( + tuple(extra_args), tuple(sorted(env_overrides.items())), (MODEL_RUNNER,) + ) @functools.lru_cache(maxsize=None) def _run_cached( - extra_args: Tuple[str, ...], overrides: Tuple[Tuple[str, str], ...] + extra_args: Tuple[str, ...], + overrides: Tuple[Tuple[str, str], ...], + scope: Tuple[str, ...], ) -> str: - """Run pytest over tests/apps/model-runner and return its combined output. + """Run pytest over `scope` and return its combined output. Both token variables are always set explicitly: python-dotenv does not override a variable that is already in the environment, so passing an @@ -96,7 +113,7 @@ def _run_cached( "pytest", "-p", "no:cacheprovider", - "tests/apps/model-runner", + *scope, *extra_args, ], cwd=REPO_ROOT, @@ -200,6 +217,46 @@ def test_the_wall_clock_summary_states_the_exposure_even_with_the_flag() -> None assert "--live GIVEN" in output +def _selected(output: str) -> int: + """Tests pytest reports as collected, from a `-q --collect-only` run.""" + if re.search(r"^no tests collected", output, re.M): + return 0 + match = re.search(r"^(\d+)(?:/\d+)? tests? collected", output, re.M) + assert match, output + return int(match.group(1)) + + +def _deselected(output: str) -> int: + match = re.search(r"\((\d+) deselected\)", output) + return int(match.group(1)) if match else 0 + + +def test_m_live_selects_exactly_what_the_gate_deselects() -> None: + """The `live` marker has to describe every test the gate acts on. + + Most are detected from their fixture closure rather than from a marker + anyone wrote, so `-m live` only agrees with the gate if the hook attaches + the marker to them. Without that attachment `-m live --live` silently + returns just the modules that declare the marker themselves -- a wrong + subset that looks like a complete answer, which is worse than the option + not working at all. + + Comparing the two counts rather than asserting a literal keeps this test + correct as live tests are added, and it is exactly the invariant that + breaks: registering a `live` marker in pytest.ini while `-m live` resolves + a different set than `--live` gates is the trap. + """ + gated = _deselected(_collect([], {}, MIXED_SCOPE)) + assert gated > 0, "scope must contain live tests for this to mean anything" + assert _selected(_collect(["-m", "live", "--live"], {}, MIXED_SCOPE)) == gated + + +def test_m_live_without_the_flag_selects_nothing() -> None: + """Deselection wins over selection: naming the marker must not be a way + in.""" + assert _selected(_collect(["-m", "live"], {}, MIXED_SCOPE)) == 0 + + def test_the_artifact_version_tests_are_gated() -> None: """Four tests that call upload_app and delete artifacts on bioimage-io/bioengine-worker. Their fixture asserts on the token rather