fix(ci): run each matrix leg on its own Python version - #374
runpod-Henrik wants to merge 4 commits into
Conversation
The quality-gates matrix declared 3.10/3.11/3.12/3.13 but every leg ran
Python 3.11. `make dev` runs a bare `uv sync`, and uv resolves its
interpreter from `.python-version` (pinned to 3.11) rather than from the
one actions/setup-python just installed. Confirmed across four CI runs on
three different branches, the oldest from 2026-08-10: every leg reports
`Using CPython 3.11.x` and builds `.venv/lib/python3.11`.
UV_PYTHON takes precedence over the pin file. Set at job level rather
than on the install step because `make ci-quality-github` runs
`uv run pytest`, which resolves the interpreter again.
uv.lock is re-locked in the same commit because it was stale in two ways
and 3.13 could not resolve without it:
* `requires-python = ">=3.10, <3.13"` while pyproject.toml says
`>=3.10,<3.14`, so the lock never covered 3.13 at all
* `tomlkit>=0.13.0` is declared in pyproject.toml but was missing from
the runpod-flash dependency and requires-dist lists
`uv lock --check` fails against the previous lock and passes against this
one, for each of 3.10/3.11/3.12/3.13. CI did not catch the drift because
pre-check uses `--frozen` (which does not verify freshness) and `make dev`
uses plain `uv sync`, which silently re-resolves on the runner.
Verified: the full suite on a real 3.13 interpreter gives 2680 passed,
with no 3.13-specific failures.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test_deploy_all_background was the source of a flaky failure that hit an
unrelated test, most often
FAILED tests/unit/test_regressions.py::TestREG008NonInteractiveEnvDelete
::test_undeploy_resource_force_remove_no_tty
_pickle.PicklingError: args[0] from __newobj__ args has the wrong class
deploy_all_background() starts a daemon thread and returns immediately,
and the test never joined it. The `patch.object(..., "get_or_deploy_resource")`
context was therefore lifted while the thread was still starting, so the
thread ran against the unpatched manager and registered the fixture's
MagicMock(spec=ServerlessResource) objects into the ResourceManager
singleton -- during whichever unrelated test happened to be running at
that moment.
ResourceManager._save_resources() cloudpickles its entire state on every
registration, and a MagicMock cannot be pickled: `obj.__class__` is the
spec'd class while `type(obj)` is MagicMock, which is exactly the
mismatch pickle.save_reduce rejects. The mock arrived as a dict *key*,
via _migrate_to_name_based_keys() calling `resource.get_resource_key()`
on it, so the offending key rendered as
`<MagicMock name='mock.get_resource_key()'>`.
Because it depended on thread scheduling and on xdist's dynamic worker
assignment, the victim, the worker and the matrix leg all varied per run:
the same failure appears on three unrelated branches, on legs 3.10, 3.12
and 3.13 and on workers gw0, gw2 and gw3. Forcing everything into one
process reproduced it every time; `-n 4` reproduced it in 0 of 6 runs.
threading.Thread is now stubbed, which keeps what this test actually
asserts -- the call is non-blocking and spawns a daemon thread -- and lets
nothing escape. The test previously asserted nothing at all; its own
comment said "not much we can test here without waiting for thread".
Verified: full suite single-process, 2680 passed, 0 PicklingError (the
same run reproduced the failure before this change). The two remaining
local failures are TestVersionFlag, which is colour-dependent and passes
both in CI and locally under NO_COLOR=1.
Two deeper faults are left alone here, as they are wider changes:
ResourceManager._resources / _resource_configs are class variables that
instance mutation writes through, and conftest's worker_flash_dir is
scope="session", so every test in a worker shares one resources.pkl.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Making the version matrix real (previous commit) exposed three failures on
Python 3.10 that had been masked for months. All three are test artifacts,
not product bugs, and all three reproduce on a real 3.10 interpreter
independently of this branch.
tests/unit/cli/commands/test_run_server_helpers.py
call_with_body -> _map_body_to_params calls inspect.signature(func), and
signature() of a Mock is version-dependent: on 3.10 it raises
`TypeError: 'Mock' object is not subscriptable` (Mock auto-creates a
__signature__ child which inspect then tries to use), while on 3.11+ it
reports (*args, **kwargs). Passing spec= does not help -- verified; the
failure is in signature() itself.
call_with_body catches Exception and converts it into a 500 JSONResponse,
so the visible failure was the uninformative
`assert <JSONResponse object> == {'ok': True}`, with the real TypeError
readable only inside the response body.
The mock is now wrapped in a real `async def (*args, **kwargs)` -- the
same signature 3.11+ inferred from the Mock, so both branches of
_map_body_to_params behave exactly as before -- and delegating to the mock
keeps `assert_called_once_with()`.
tests/unit/core/resources/test_resource_manager_extended.py
test_loads_legacy_dict_format cloudpickle.dump()s a MagicMock, which 3.10
rejects with "Could not pickle object as excessively deep recursion
required". Replaced with a module-level _LegacyResource: only .config_hash
is read (via _refresh_config_hashes), and the *absence* of
get_resource_key is what keeps the legacy key un-migrated, which is what
this test asserts. Module-level rather than defined in the test body,
since cloudpickle serialises a function-local class by value.
Verified on 3.10, 3.11 and 3.13: 40 passed in both affected files on each,
where 3.10 previously failed all three tests. ruff format and check clean.
Worth following up separately: the blanket `except Exception ->
JSONResponse(500)` in call_with_body turns genuine errors into silent 500s,
which is what made this take three steps to diagnose.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
What the real matrix found on 3.10, and the fixMaking the matrix real ( All three failures are pre-existing and 3.10-specific — nothing to do with the matrix change itself, which only stopped hiding them. All three reproduce on a real 3.10 interpreter independently of this branch. 1 & 2.
|
| Python | inspect.signature(AsyncMock()) |
|---|---|
| 3.10.19 | raises TypeError: 'Mock' object is not subscriptable |
| 3.13.11 | returns (*args, **kwargs) |
Mock auto-creates a __signature__ child attribute, which inspect then tries to use. Passing spec= does not help — verified; the failure is inside signature(), not in spec resolution.
This was double-masked. call_with_body catches Exception and converts it into a 500 JSONResponse, so the visible failure was the uninformative assert <JSONResponse object> == {'ok': True}. The real TypeError was readable only inside the response body:
returned: JSONResponse
body: {"error":"'Mock' object is not subscriptable"}
_map_body_to_params raised TypeError: 'Mock' object is not subscriptable # py3.10
_map_body_to_params -> {'args': {'key': 'value'}} # py3.13
Fix: wrap the mock in a real async def (*args, **kwargs). That is the same signature 3.11+ inferred from the Mock, so both branches of _map_body_to_params behave exactly as before — the non-empty body maps to the first parameter, the empty body spreads as kwargs — and delegating to the mock keeps assert_called_once_with().
3. test_resource_manager_extended.py — pickling a Mock
FAILED TestLoadResources::test_loads_legacy_dict_format
_pickle.PicklingError: Could not pickle object as excessively deep recursion required.
The test does cloudpickle.dump({"key1": MagicMock(config_hash="hash1")}). Whether cloudpickle can pickle a Mock is version-dependent; 3.10 rejects it.
Fix: a module-level _LegacyResource stand-in. Only .config_hash is read (via _refresh_config_hashes), and the absence of get_resource_key is what keeps the legacy key un-migrated — which is exactly what the test asserts. Module-level rather than defined in the test body, because cloudpickle serialises a function-local class by value.
Verification
CI on 0625cc9 — the first run in this repo's history where all four legs use four different interpreters:
| Leg | Result |
|---|---|
| Quality Gates (3.10) | pass |
| Quality Gates (3.11) | pass |
| Quality Gates (3.12) | pass |
| Quality Gates (3.13) | pass |
| Validation | pass |
Locally, before pushing, both affected files were run against three real interpreters:
| Python | Result |
|---|---|
| 3.10 | 40 passed (previously 3 failed) |
| 3.11 | 40 passed |
| 3.13 | 40 passed |
ruff format --check and ruff check clean.
Worth a separate look
The blanket except Exception: return JSONResponse(status_code=500, ...) in call_with_body turns genuine errors into silent 500s. It is what made a one-line TypeError take three steps to diagnose, and in production it would do the same to a real bug. Not changed here — it is behaviour, not a test fix.
…rix-thread-leak # Conflicts: # tests/unit/test_deployment.py
The
quality-gatesmatrix declares four Python versions but ran 3.11 on every leg. This makes the matrix real, and fixes the three tests that turned out to be broken on 3.10 once 3.10 actually ran.1. The Python version matrix was decorative
quality-gatesdeclares['3.10', '3.11', '3.12', '3.13'], but every leg ran Python 3.11.make devruns a bareuv sync, and uv resolves its interpreter from.python-version(pinned to3.11) rather than from the oneactions/setup-pythonjust installed.Confirmed across four CI runs on three unrelated branches, the oldest from 2026-08-10 — every leg reports
Using CPython 3.11.xand builds.venv/lib/python3.11:deanq/sls-python-parity-by-defaultfix/367-app-delete-removes-endpointfix/365-deploy-empty-resourcesHenrik/ci-coverage-artifactFix:
UV_PYTHON: ${{ matrix.python-version }}, which takes precedence over the pin file. Set at job level rather than on the install step, becausemake ci-quality-githubrunsuv run pytest, which resolves the interpreter again.uv.lockwas stale, and 3.13 could not resolve without itRe-locked in the same commit. On
mainit is wrong in two ways:requires-python = ">=3.10, <3.13"whilepyproject.tomlsays>=3.10,<3.14— the lock never covered 3.13 at alltomlkit>=0.13.0is declared inpyproject.toml, but the lock'srunpod-flashentry omits it from bothdependenciesandrequires-dist(the package block itself is present, pulled in by another dependency)CI never caught the drift:
pre-checkuses--frozen, which does not verify freshness, andmake devuses plainuv sync, which silently re-resolves on the runner.2. Three tests only ever passed because they only ever ran on 3.11
Turning the matrix on is what surfaced these. All three pass on 3.11+ and fail on 3.10, so they were green for as long as the matrix was decorative. Each failure is a version-dependent difference in how mocks behave, not a product bug.
test_run_server_helpers.py—test_allows_plain_dict_body,test_allows_empty_dict_bodycall_with_body→_map_body_to_paramscallsinspect.signature(func), andsignature()of aMockis version-dependent: on 3.10 it raisesTypeError: 'Mock' object is not subscriptable(Mock auto-creates a__signature__child, whichinspectthen tries to use), while on 3.11+ it reports(*args, **kwargs). Passingspec=does not help — the failure is insignature(), not in spec resolution.call_with_bodycatchesExceptionand converts it into a 500JSONResponse, so on 3.10 these failed asassert <JSONResponse object> == {'ok': True}, with the realTypeErrorvisible only inside the response body.Fix: wrap the mock in a real
async def (*args, **kwargs)— the same signature 3.11+ inferred from the Mock, so both branches of_map_body_to_paramsbehave exactly as before, and delegating to the mock keeps the call assertions.test_resource_manager_extended.py— the legacy-state-file testWhether cloudpickle can pickle a
Mockis version-dependent; on 3.10 this failed withCould not pickle object as excessively deep recursion required. Replaced theMagicMockwith a module-level_LegacyResourcestub. Only.config_hashis read here (via_refresh_config_hashes), and the absence ofget_resource_keyis what keeps the legacy key un-migrated — which is exactly what the test asserts. Module-level rather than defined in the test body, because cloudpickle serialises a function-local class by value, which is the thing being avoided.Dropped from this PR
The original second half of this PR fixed
test_deploy_all_backgroundleaking a daemon thread into unrelated tests (surfacing as_pickle.PicklingError: args[0] from __newobj__ args has the wrong classon whichever test happened to be running). It stubbedthreading.Thread.#371 has since landed on
mainwith a different fix for the same leak — it joins the real thread and additionally assertsmock_deploy.await_count == 3. Mergingmainconflicted on exactly that test, and this PR takesmain's version: it asserts strictly more, and a CI-plumbing PR should not revert a landed fix.tests/unit/test_deployment.pyis now byte-identical tomain.The commit that introduced the stub is still in this branch's history; its effect is reverted by the merge, so the net diff against
maincontains no change to that test.Verification
Run on the merge commit:
make test-coverage, parallel passmake test-coverage, serial pass (-m serial)ruff check+ruff format --check, 260 tracked filesuv lock --checkAnd on CI, the check that actually matters for this fix — each leg now reports a different interpreter, where previously all four reported 3.11.x (run 35759535135):
Using CPython 3.10.21Using CPython 3.11.16Using CPython 3.12.14Using CPython 3.13.15All four pass, which is also the first evidence that the suite is green on 3.10, 3.12 and 3.13 at all.
Not addressed here
Two deeper faults that made the leaked-thread bug possible are still present — #371 fixed the symptom in the test, not these:
ResourceManager._resources/_resource_configsare class variables, so instance mutation writes through to global state — while_load_resources()uses assignment, which shadows them with instance attributes. The manager silently switches between class-level and instance-level state depending on whether the state file existed.conftest.worker_flash_dirisscope="session", so every test in a worker shares oneresources.pkl, andreset_singletonsforces a reload from it on every test.🤖 Generated with Claude Code