From bb3b41c5c1dd73a4e105fefc3bdeff99d58c178d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dean=20Qui=C3=B1anola?= Date: Fri, 7 Aug 2026 14:28:34 -0700 Subject: [PATCH 1/2] fix(deployment): return background thread so callers can join it deploy_all_background spawned a daemon thread and returned nothing, so callers had no way to wait for it. test_deploy_all_background patched ResourceManager.get_or_deploy_resource, started the thread, and returned immediately -- the patch unwound while the worker was still running, so the thread went on to hit the real deploy path with AsyncMock resources and cached them via _add_resource. ResourceManager._resources is a class variable, and the late write lands in whichever dict is current when it happens, i.e. one belonging to a later test. The autouse reset_singletons fixture cannot prevent this -- the write occurs after the reset. Any test that subsequently triggers _save_resources() dies in cloudpickle: _pickle.PicklingError: args[0] from __newobj__ args has the wrong class Which test pays for it depends on thread scheduling and on how xdist distributes work, which is why this surfaced as an intermittent failure in test_regressions.py::TestREG008 on a single Python version. Return the thread so callers can join it, and join it in the test inside the patch context. - deploy_all_background now returns Optional[threading.Thread] - test_deploy_all_background joins before releasing its patches - add test_deploy_all_background_returns_joinable_thread to pin the contract, asserting the deploy mock absorbed every resource - assert the empty-list path returns None --- src/runpod_flash/core/deployment.py | 16 ++++++++++--- tests/unit/test_deployment.py | 37 +++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/src/runpod_flash/core/deployment.py b/src/runpod_flash/core/deployment.py index 93d27722..f925262a 100644 --- a/src/runpod_flash/core/deployment.py +++ b/src/runpod_flash/core/deployment.py @@ -6,7 +6,7 @@ from dataclasses import dataclass from datetime import datetime from enum import Enum -from typing import List +from typing import List, Optional from rich.console import Console from rich.progress import Progress, SpinnerColumn, TextColumn @@ -55,7 +55,9 @@ def __init__(self, max_concurrent: int = 3): self.manager = ResourceManager() self.results: List[DeploymentResult] = [] - def deploy_all_background(self, resources: List[DeployableResource]) -> None: + def deploy_all_background( + self, resources: List[DeployableResource] + ) -> Optional[threading.Thread]: """Deploy all resources in background thread. This method spawns a background thread to deploy resources without @@ -63,10 +65,17 @@ def deploy_all_background(self, resources: List[DeployableResource]) -> None: Args: resources: List of resources to deploy + + Returns: + The worker thread, or None when there was nothing to deploy. + Callers that need the deployment to finish within a bounded scope + (notably tests, which patch the deploy path) must join it — + otherwise the daemon thread outlives that scope and mutates + ResourceManager's class-level state afterwards. """ if not resources: console.print("[dim]No resources to deploy[/dim]") - return + return None def run_async_deployment(): """Run async deployment in background thread.""" @@ -90,6 +99,7 @@ def run_async_deployment(): console.print( f"[dim]Auto-provisioning {len(resources)} resource(s) in background...[/dim]" ) + return thread async def deploy_all( self, resources: List[DeployableResource], show_progress: bool = True diff --git a/tests/unit/test_deployment.py b/tests/unit/test_deployment.py index 081ca0b9..ad93c959 100644 --- a/tests/unit/test_deployment.py +++ b/tests/unit/test_deployment.py @@ -2,6 +2,7 @@ import pytest import asyncio +import threading from unittest.mock import MagicMock, AsyncMock, patch from runpod_flash.core.deployment import ( @@ -204,17 +205,43 @@ def test_deploy_all_background(self, mock_resources): mock_deploy.side_effect = mock_resources # Should not block - orchestrator.deploy_all_background(mock_resources) + thread = orchestrator.deploy_all_background(mock_resources) + + # Join inside the patch context so the worker cannot outlive the + # mock and reach the real deploy path. See the returns-a-thread + # test below for why this matters. + thread.join(timeout=10) + assert not thread.is_alive() + + def test_deploy_all_background_returns_joinable_thread(self, mock_resources): + """Callers must be able to await background deployment. + + Without a handle to join, the daemon thread outlives the caller's + patches and runs the real deploy path afterwards, writing mock + resources into the ResourceManager class-level state. That state is + shared process-wide, so an unrelated test later fails when it tries + to cloudpickle the leftover mock. + """ + orchestrator = DeploymentOrchestrator() + + with patch.object( + orchestrator.manager, "get_or_deploy_resource", new_callable=AsyncMock + ) as mock_deploy: + mock_deploy.side_effect = mock_resources + + thread = orchestrator.deploy_all_background(mock_resources) - # Background thread should be started - # (not much we can test here without waiting for thread) + assert isinstance(thread, threading.Thread) + thread.join(timeout=10) + assert not thread.is_alive() + assert mock_deploy.await_count == len(mock_resources) def test_deploy_all_background_empty_list(self): """Test background deployment with empty list.""" orchestrator = DeploymentOrchestrator() - # Should handle gracefully - orchestrator.deploy_all_background([]) + # Nothing to deploy means no thread to join. + assert orchestrator.deploy_all_background([]) is None @pytest.mark.asyncio async def test_deploy_all_raises_api_key_error_before_deploying( From dc6983a7b4800cbaf5be84f72af8a978039b6a7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dean=20Qui=C3=B1anola?= Date: Fri, 7 Aug 2026 14:53:07 -0700 Subject: [PATCH 2/2] chore(deps): resync uv.lock requires-python with pyproject pyproject.toml declares >=3.10,<3.14 and CI runs a 3.13 job, but the committed lock still pinned >=3.10,<3.13. Any `uv sync` on 3.13 regenerated the file, leaving a spurious diff in the working tree. Resolution is unchanged -- `uv lock` rewrites only the requires-python line, no package versions move. --- uv.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uv.lock b/uv.lock index a2e48d3f..c43632a7 100644 --- a/uv.lock +++ b/uv.lock @@ -1,6 +1,6 @@ version = 1 revision = 3 -requires-python = ">=3.10, <3.13" +requires-python = ">=3.10, <3.14" resolution-markers = [ "python_full_version >= '3.13'", "python_full_version < '3.13'",