test: use subprocesses for each test file - #29016
Merged
Merged
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Replaces fragile manual
_reset_module_stateautouse fixtures + pytest-xdist with per-file process isolation — each test file runs in a freshly-spawnedpython -m pytest <file>subprocess viascripts/run_tests_parallel.py.Cross-file module-level state leakage (module-level dicts, ContextVars, caches) is impossible: every file gets a clean interpreter. Intra-file ordering is the test author's responsibility.
Why per-file, not per-test?
Per-test subprocess isolation is the obvious-sounding answer but doesn't survive the math:
Per-file is the actual isolation boundary that matters:
_reset_module_stateexisted to clean up — fixed for free by giving each file a fresh interpreterfoo.pymutates state test B infoo.pyreads, that's a real bug that bites anyone runningpytest tests/foo.pydirectlyWhy drop xdist?
xdist's persistent worker pool accumulates state across files within a worker, which is exactly the leakage class we want to prevent. xdist also brings
--dist=loadvsloadfilevsloadscopevsworksteal,--max-worker-restartsemantics, and an internal control plane — all of which we don't need when the unit of work is "run pytest on one file in a fresh process." AThreadPoolExecutorof N workers each callingsubprocess.run([python, -m, pytest, file])is ~60 lines, has cleaner semantics, and gives stronger guarantees.Performance
Local (16-core box, full suite ~1125 files / ~17k tests):
-n auto+ per-test spawn isolation pluginsubprocess.runwithos.cpu_count()workersCI (4-core ubuntu-latest): ~8 min, well within the 60-min job budget.
Type of Change
Surfaced and Fixed Bugs
Switching to per-file isolation exposed ~30 previously-hidden test failures across 14 files. Under xdist, these tests passed only because other files' import side effects populated shared module-level state. All are now fixed:
Tool registry not populated in isolation (9 files, ~16 tests)
These tests accessed the tool or web-search-provider registry before
discover_builtin_tools()had run. Under xdist, a co-worker's earlier import populated the registry as a side effect.tests/tools/test_video_generation_tool_surface_matrix.pydiscover_builtin_tools()call before registry accesstests/tools/test_web_providers_brave_free.pytests/tools/conftest.py) registering all 8 bundled web providers +is_safe_url/check_website_accessmockstests/tools/test_web_providers_ddgs.pytests/tools/test_web_providers_searxng.pytests/tools/test_web_providers.pytests/tools/test_website_policy.pytests/tools/test_web_tools_tavily.pytests/tools/test_discord_tool.pytests/tools/test_homeassistant_tool.pyinvalidate_check_fn_cache()before registry queriesStale check_fn / tool-defs cache (1 file, 2 tests)
test_kanban_guidance_not_in_normal_promptcachedcheck_fn(kanban_show) → False. Next test setHERMES_KANBAN_TASKbut never invalidated the cache.tests/tools/test_kanban_tools.pyinvalidate_check_fn_cache()+_clear_tool_defs_cache()in both kanban guidance testsModule-level state pollution (4 files, ~12 tests)
tests/agent/test_auxiliary_client.py_aux_unhealthy_until/_aux_unhealthy_logged_atdicts persisted across test boundariestests/agent/test_skill_commands.pypatch.dict(os.environ, {"HERMES_SESSION_PLATFORM": "telegram"})— ContextVar takes precedence over os.environset_session_vars(platform="telegram")+clear_session_vars()in finally blocktests/gateway/test_dm_topics.pysys.modules.setdefaultfor telegram mock shadowing attributes; cachedgateway.platforms.telegramimportsys.modules[name](not setdefault), registertelegram.constantsas separate module,del sys.modules["gateway.platforms.telegram"]tests/tools/test_terminal_tool_requirements.pyclass TestTerminalRequirements:(IndentationError) + stale tool-defs cache_clear_cachesfixtureTiming flakes (1 file, 1 test)
tests/plugins/test_achievements_plugin.pyevaluate_all()returns stale data (10 fake sessions too fast)scan_delay=2.0on_FakeSessionDBso background thread can't win the raceOther fixes
tests/tools/test_send_message_tool.pytelegrampackage not installedtests/tools/test_browser_supervisor.pyworker_id(no xdist)tests/gateway/conftest.pytests/tools/test_approval_plugin_hooks.pytests/tools/test_write_deny.pytests/hermes_cli/test_pty_bridge.pytests/plugins/web/test_web_search_provider_plugins.pyOther Changes
hermes_cli/main.py— py_compile race fix_validate_critical_files_syntaxused to write.pycinto the source tree's__pycache__/, which races with concurrent test workers. Now usestempfile.TemporaryDirectoryso the compiled output is isolated and discarded.hermes_cli/profiles.py— NixOS rmtree fixshutil.rmtree(profile_dir)fails on NixOS where profile dirs contain read-only copies from the immutable Nix store. Added a_make_writableonexc/onerror handler that adds+wonPermissionError(both the path itself and its parent). Compatible with Python 3.11 (onerror) and 3.12+ (onexc).CI — ripgrep via prebuilt tarball
Replaced
sudo apt-get install -y ripgrep(~4 min) with a pinned, sha256-verified prebuilt binary download (~5s). Eliminates theapt-get updatecold-start penalty..gitignore—.pytest-cache/Added
.pytest-cache/(emitted by the parallel runner's per-file subprocesses).Files Changed (summary)
Added:
scripts/run_tests_parallel.py(~650 lines) — per-file subprocess runner withThreadPoolExecutor, captured stdout, live progress, exit-code-5-as-pass handling, per-file test-level pass/fail countstests/test_run_tests_parallel.py(187 lines) — self-validating tests for the runnertests/tools/conftest.py(50 lines) — shared autouse fixture for web provider registry + SSRF mocksRemoved:
pytest-xdist==3.8.0from dev deps-n autofrompyproject.tomladdopts_reset_module_state+ related autouse fixtures fromtests/conftest.pyNet: +1695 / −583 across 35 files
How to Test