From f9acf464e9a5d60dacbe79a4bcda2944f6670bb6 Mon Sep 17 00:00:00 2001 From: nesquena-hermes <[email protected]> Date: Sat, 30 May 2026 21:09:25 +0000 Subject: [PATCH] ci: run test suite in 3 parallel shards + make suite shard-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the test-sharding half of #3197 (Docker-cache half shipped v0.51.177). Adds pytest-shard 3-way split to tests.yml (3 shards x 3 Python = 9 jobs, fail-fast: false). pytest-shard is 0-indexed so the matrix uses [0,1,2] — the original #3197 used [1,2,3] which would have crashed the out-of-range job and silently skipped shard 0's tests. Made the suite shard-safe by fixing 4 cross-test state-pollution bugs that passed sequentially but failed when sharded: - test_onboarding_mvp: reset onboarding_completed flag (settings.json) in the autouse fixture; the config-cleanup only cleared config.yaml/.env. - test_issue693_system_health_panel: invalidate the process-wide password-hash cache before/after so a prior test's "no password" cache doesn't defeat the auth-gate assertion. - test_auth_session_persistence: assert against auth._SESSIONS_FILE (where auth actually writes) instead of a local _TEST_STATE path that only matched under a lucky import order. - test_profile_env_isolation (root cause of the worst leak): stop deleting + re-importing api.profiles under a temp HERMES_BASE_HOME — that swapped the module object and poisoned the cached _DEFAULT_HERMES_HOME for every later test (broke test_title_aux_routing's load_config). Now points the cached path via monkeypatch.setattr (auto-restored, no module swap). - conftest: autouse fixture restores HERMES_HOME/HERMES_BASE_HOME after each test as defense-in-depth against future switch_profile leaks. Verified: all 3 shards green (6912 passed, 0 failed); full sequential run still green (6957 passed, 0 failed). Slowest shard ~70s vs ~180s sequential. --- .github/workflows/tests.yml | 22 ++++++++-- CHANGELOG.md | 6 +++ tests/conftest.py | 49 ++++++++++++++++++++++ tests/test_auth_session_persistence.py | 6 ++- tests/test_issue693_system_health_panel.py | 16 ++++++- tests/test_onboarding_mvp.py | 7 ++++ tests/test_profile_env_isolation.py | 20 +++++---- 7 files changed, 112 insertions(+), 14 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2248ffa3b..d0de40035 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -10,8 +10,19 @@ jobs: test: runs-on: ubuntu-latest strategy: + # Don't cancel the other shards/versions when one fails — we want the full + # failure picture across the matrix in a single run. + fail-fast: false matrix: python-version: ['3.11', '3.12', '3.13'] + # Split the suite across 3 parallel shards per Python version. pytest-shard + # partitions tests deterministically by test-id hash; the suite was made + # shard-safe (no cross-test state leakage) so every shard passes + # independently. See docs/agent-memory note on test-suite shard-safety. + # NOTE: pytest-shard is 0-indexed — shard ids must be 0..num_shards-1. + # Using 1-based ids would crash the out-of-range job AND silently skip + # shard 0's tests. + shard: [0, 1, 2] steps: - uses: actions/checkout@v4 @@ -20,11 +31,16 @@ jobs: uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} + cache: 'pip' + cache-dependency-path: | + **/setup.cfg + **/requirements*.txt + **/pyproject.toml - name: Install dependencies run: | python -m pip install --upgrade pip - pip install "pyyaml>=6.0" pytest pytest-timeout pytest-asyncio + pip install "pyyaml>=6.0" pytest pytest-timeout pytest-asyncio pytest-shard # Install the `mcp` package so tests/test_mcp_server.py runs in CI. # The package is an optional runtime dep of mcp_server.py — users # who run the MCP integration install it themselves; CI installs @@ -33,5 +49,5 @@ jobs: # importorskip and the matrix stays green. pip install mcp || echo "mcp install failed — test_mcp_server.py will importorskip" - - name: Run tests - run: pytest tests/ -v --timeout=60 + - name: Run tests (shard ${{ matrix.shard }} of 3) + run: pytest tests/ -v --timeout=60 --shard-id=${{ matrix.shard }} --num-shards=3 diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c8a77f8f..b0a60bb12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ ## [Unreleased] +## [v0.51.178] — 2026-05-30 — Release EX (stage-batch60 — parallel sharded CI test runs) + +### Changed + +- CI: the test suite now runs in 3 parallel shards per Python version (9 jobs total) via `pytest-shard`, cutting wall-clock test time roughly in half (slowest shard ~70s vs ~180s sequential). To make sharding safe, several tests that asserted a pristine default while a sibling test mutated shared process/server state were fixed to establish their own preconditions: onboarding-completed flag reset (`test_onboarding_mvp`), password-hash cache invalidation (`test_issue693_system_health_panel`), authoritative sessions-file path (`test_auth_session_persistence`), and — the root cause of the worst leak — `test_profile_env_isolation` no longer deletes + re-imports `api.profiles` (which poisoned the module's cached base-home global for every later test); it now points the cached path via `monkeypatch.setattr`. A conftest fixture also restores `HERMES_HOME`/`HERMES_BASE_HOME` after each test as defense-in-depth. Completes the test-sharding half of #3197 (the Docker-cache half shipped in v0.51.177). + ## [v0.51.177] — 2026-05-30 — Release EW (stage-batch59 — Docker smoke-test layer caching) ### Changed diff --git a/tests/conftest.py b/tests/conftest.py index 57ca4ba9b..43c91a618 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,6 +18,7 @@ import os import pathlib import shutil import subprocess +import sys import time import urllib.request import urllib.error @@ -83,6 +84,54 @@ def _isolate_hermes_config_path(): yield os.environ['HERMES_CONFIG_PATH'] = isolated_config_path + +@pytest.fixture(autouse=True) +def _restore_profile_home_globals(): + """Restore HERMES_HOME / HERMES_BASE_HOME after every test. + + Several tests call ``api.profiles.switch_profile()`` (or set HERMES_HOME + directly) which mutates ``os.environ['HERMES_HOME']`` IN PLACE — not via + monkeypatch — so the change is not auto-reverted at test teardown. In the + normal sequential run the next test usually re-establishes its own profile so + the leak is masked, but under pytest-shard (or pytest-randomly) the leaked + HERMES_HOME points at a deleted tmpdir and breaks any later test whose + config/profile resolution reads it (e.g. test_title_aux_routing's + background-worker profile routing, which then falls back to DEFAULT_CONFIG + where ``model`` is an empty string). Snapshotting at the conftest level fixes + the whole class at once, regardless of which test does the leaking. + """ + saved_home = os.environ.get('HERMES_HOME') + saved_base = os.environ.get('HERMES_BASE_HOME') + # Re-derive the cached base-home global BEFORE the test runs too: a prior + # test's teardown ordering (monkeypatch restoring sys.modules['api.profiles'] + # after this fixture's teardown) can leave the live module's + # _DEFAULT_HERMES_HOME stale. Fixing it at setup time guarantees each test + # starts from a base root that matches the current (restored) env. + _rederive_default_hermes_home() + yield + for key, val in (('HERMES_HOME', saved_home), ('HERMES_BASE_HOME', saved_base)): + if val is None: + os.environ.pop(key, None) + else: + os.environ[key] = val + _rederive_default_hermes_home() + + +def _rederive_default_hermes_home(): + """Recompute api.profiles._DEFAULT_HERMES_HOME from the current env. + + api.profiles caches the base home at import time. A test that re-imports + api.profiles under a temporary HERMES_BASE_HOME (e.g. test_profile_env_isolation) + corrupts that global to a now-deleted tmpdir, making get_hermes_home_for_profile + resolve later tests' profiles under the dead path. Re-deriving keeps it honest. + """ + prof_mod = sys.modules.get('api.profiles') + if prof_mod is not None and hasattr(prof_mod, '_resolve_base_hermes_home'): + try: + prof_mod._DEFAULT_HERMES_HOME = prof_mod._resolve_base_hermes_home() + except Exception: + pass + # ── Server script: always relative to repo root ─────────────────────────── SERVER_SCRIPT = REPO_ROOT / 'server.py' if not SERVER_SCRIPT.exists(): diff --git a/tests/test_auth_session_persistence.py b/tests/test_auth_session_persistence.py index 84b796f57..a54404661 100644 --- a/tests/test_auth_session_persistence.py +++ b/tests/test_auth_session_persistence.py @@ -81,7 +81,11 @@ class TestSessionPersistence(unittest.TestCase): def test_sessions_file_permissions(self) -> None: """Sessions file must be owner-read-only (0600).""" auth.create_session() - sessions_file = _TEST_STATE / '.sessions.json' + # Check the path auth actually writes to (auth._SESSIONS_FILE is computed + # from api.config.STATE_DIR at import time). Asserting against a local + # _TEST_STATE assumption is fragile under sharded / reordered runs where + # this module may import before api.config.STATE_DIR resolves to _TEST_STATE. + sessions_file = auth._SESSIONS_FILE self.assertTrue(sessions_file.exists(), ".sessions.json was not created") mode = oct(sessions_file.stat().st_mode & 0o777) self.assertEqual(mode, oct(0o600), diff --git a/tests/test_issue693_system_health_panel.py b/tests/test_issue693_system_health_panel.py index 81b584382..511aaa147 100644 --- a/tests/test_issue693_system_health_panel.py +++ b/tests/test_issue693_system_health_panel.py @@ -108,11 +108,23 @@ def test_system_health_route_registered_and_auth_gated(monkeypatch): assert '"/api/system/health"' not in AUTH_PY, "system metrics must not be public" monkeypatch.setenv("HERMES_WEBUI_PASSWORD", "test-password") + from api import auth as _auth from api.auth import check_auth + # The password hash is cached process-wide (PBKDF2 is ~1s). A prior test may + # have populated the cache with "no password" (None), so the env var we just + # set would be ignored on the fast path. Invalidate before AND after so this + # test sees its own password and doesn't leak the test-password cache to the + # next test — required for order-independence under sharded/random runs. + _auth._invalidate_password_hash_cache() + handler = _FakeHandler() - assert check_auth(handler, SimpleNamespace(path="/api/system/health", query="")) is False - assert handler.status in (302, 401) + try: + assert check_auth(handler, SimpleNamespace(path="/api/system/health", query="")) is False + assert handler.status in (302, 401) + finally: + monkeypatch.delenv("HERMES_WEBUI_PASSWORD", raising=False) + _auth._invalidate_password_hash_cache() def test_system_health_route_returns_only_sanitized_payload(monkeypatch): diff --git a/tests/test_onboarding_mvp.py b/tests/test_onboarding_mvp.py index 82ac6075d..664fe2200 100644 --- a/tests/test_onboarding_mvp.py +++ b/tests/test_onboarding_mvp.py @@ -64,9 +64,16 @@ def clean_hermes_config_files(): hermes_home = _server_hermes_home() for rel in ("config.yaml", ".env"): (hermes_home / rel).unlink(missing_ok=True) + # onboarding_completed lives in settings.json (not config.yaml/.env), so the + # unlinks above don't reset it. A prior test that completes onboarding would + # otherwise leak the flag and make tests asserting the pristine "incomplete" + # default fail under sharded/reordered runs. Reset it via the settings API + # (the source of truth the server reads) before AND after each test. + post("/api/settings", {"onboarding_completed": False}) yield for rel in ("config.yaml", ".env"): (hermes_home / rel).unlink(missing_ok=True) + post("/api/settings", {"onboarding_completed": False}) diff --git a/tests/test_profile_env_isolation.py b/tests/test_profile_env_isolation.py index 3b2faae61..c73d41520 100644 --- a/tests/test_profile_env_isolation.py +++ b/tests/test_profile_env_isolation.py @@ -18,10 +18,12 @@ def test_profile_switch_clears_previous_profile_env_vars(monkeypatch, tmp_path): monkeypatch.delenv("OPENAI_API_KEY", raising=False) monkeypatch.delenv("CUSTOM_TOKEN", raising=False) - # Use monkeypatch so sys.modules is restored after the test, preventing - # api.profiles from being permanently removed and poisoning subsequent tests. - monkeypatch.delitem(sys.modules, "api.profiles", raising=False) - profiles = importlib.import_module("api.profiles") + # Point the module's cached base-home at our temp base via monkeypatch + # (auto-restored at teardown) instead of deleting + re-importing api.profiles. + # The delitem+import_module approach swapped the module object and poisoned + # dependent modules' cached references, breaking later tests under sharding. + import api.profiles as profiles + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) profiles.init_profile_state() profiles.switch_profile("p1") @@ -53,10 +55,12 @@ def test_profile_switch_replaces_overlapping_keys(monkeypatch, tmp_path): monkeypatch.delenv("ONLY_P1", raising=False) monkeypatch.delenv("ONLY_P2", raising=False) - # Use monkeypatch so sys.modules is restored after the test, preventing - # api.profiles from being permanently removed and poisoning subsequent tests. - monkeypatch.delitem(sys.modules, "api.profiles", raising=False) - profiles = importlib.import_module("api.profiles") + # Point the module's cached base-home at our temp base via monkeypatch + # (auto-restored at teardown) instead of deleting + re-importing api.profiles. + # The delitem+import_module approach swapped the module object and poisoned + # dependent modules' cached references, breaking later tests under sharding. + import api.profiles as profiles + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) profiles.init_profile_state() profiles.switch_profile("p1")