fix: include serial tests in coverage.xml and upload it from CI

.github/workflows/ci.yml

@@ -231,6 +231,17 @@ jobs:
         python-version: ['3.10', '3.11', '3.12', '3.13']
     timeout-minutes: 15
 
+    # Without this the matrix is decorative. `make dev` runs a bare `uv sync`,
+    # and uv resolves its interpreter from .python-version (pinned to 3.11)
+    # rather than from whatever actions/setup-python just installed -- so every
+    # leg built a 3.11 venv and all four ran the same interpreter. UV_PYTHON
+    # takes precedence over the pin file, which restores real version coverage.
+    #
+    # Set at job level, not on the install step: `make ci-quality-github` runs
+    # `uv run pytest`, which resolves the interpreter again.
+    env:
+      UV_PYTHON: ${{ matrix.python-version }}
+
     steps:
       - name: Checkout code
         uses: actions/checkout@v4
@@ -252,6 +263,28 @@ jobs:
       - name: Quality checks
         run: make ci-quality-github
 
+      # Shared with the other repos' coverage jobs. It lives in its own public
+      # repo because a public repository -- this one -- cannot resolve an action
+      # from a private one. An earlier attempt to consume it from the private
+      # internal-workflows repo failed at `Set up job` on every matrix leg with
+      # "Unable to resolve action, not found", so no tests ran at all.
+      #
+      # Pinned by commit rather than tag: a tag can be repointed at new code,
+      # and pinning means a change to the action cannot reach us until we bump
+      # it deliberately.
+      #
+      # This owns the coverage artifact upload. The test-results upload below
+      # stays separate -- the action uploads one artifact, and these are two.
+      - name: Coverage summary
+        if: always()
+        uses: runpod/coverage-summary-action@65b35a32ecfd9c2f0dc2352394eca1decfba4915
+        with:
+          format: cobertura
+          coverage-file: coverage.xml
+          results: pytest-results-*.xml
+          artifact-name: coverage-${{ matrix.python-version }}
+          if-no-files-found: warn
+
       - name: Upload test results
         uses: actions/upload-artifact@v4
         if: always()

Makefile

@@ -72,8 +72,10 @@ test-integration-serial: # Run integration tests serially (for debugging)
 	uv run pytest tests/integration/ -v -m integration
 
 test-coverage: # Run tests with coverage report (parallel non-serial, then serial pass for state isolation)
-	uv run pytest tests/ -v -n auto -m "not serial" --cov=runpod_flash --cov-report=xml
-	uv run pytest tests/ -v -m "serial" --cov=runpod_flash --cov-append --cov-report=term-missing
+	uv run pytest tests/ -v -n auto -m "not serial" --cov=runpod_flash --cov-branch --cov-report=xml
+	# Re-emit the XML after appending the serial tests, or coverage.xml is left
+	# holding only the parallel run and undercounts.
+	uv run pytest tests/ -v -m "serial" --cov=runpod_flash --cov-branch --cov-append --cov-report=term-missing --cov-report=xml
 
 test-coverage-serial: # Run tests with coverage report (serial execution)
 	uv run pytest tests/ -v --cov=runpod_flash --cov-report=term-missing
@@ -124,10 +126,24 @@ ci-quality-github: # Quality checks with GitHub Actions formatting (parallel by
 	uv run ruff check . --output-format=github
 	@echo "::endgroup::"
 	@echo "::group::Test suite with coverage (parallel non-serial)"
-	uv run pytest tests/ --junitxml=pytest-results-parallel.xml -v -n auto -m "not serial" --cov=runpod_flash --cov-report=xml --cov-fail-under=0
+	uv run pytest tests/ --junitxml=pytest-results-parallel.xml -v -n auto -m "not serial" --cov=runpod_flash --cov-branch --cov-report=xml --cov-fail-under=0
 	@echo "::endgroup::"
 	@echo "::group::Test suite with coverage (serial pass)"
-	uv run pytest tests/ --junitxml=pytest-results-serial.xml -v -m "serial" --cov=runpod_flash --cov-append --cov-report=term-missing
+	# Re-emit the XML after appending the serial tests, or coverage.xml is left
+	# holding only the parallel run and undercounts.
+	#
+	# This line, not the parallel one above, is the real coverage gate: the
+	# parallel invocation passes --cov-fail-under=0 to suppress the partial
+	# number, while this one inherits --cov-fail-under=65 from pyproject
+	# addopts. Do not "align" the two flags — that quietly disables the gate.
+	#
+	# Note also that make stops at the first failing recipe line, so if the
+	# parallel pass fails this re-emit never runs and coverage.xml holds the
+	# parallel subset. That is deliberate: guarding the parallel line with
+	# `-`/`|| true` so this always ran would let a red parallel suite exit 0.
+	# The weekly report only reads runs whose status is success, so an
+	# undercount on an already-failing run is not consumed by anything.
+	uv run pytest tests/ --junitxml=pytest-results-serial.xml -v -m "serial" --cov=runpod_flash --cov-branch --cov-append --cov-report=term-missing --cov-report=xml
 	@echo "::endgroup::"
 
 ci-quality-github-serial: # Serial quality checks for GitHub Actions (for debugging)

tests/unit/cli/commands/test_run_server_helpers.py

@@ -336,12 +336,34 @@ async def test_allows_populated_fields(self):
         assert result == {"echo": "hello"}
         func.assert_called_once_with(msg="hello")
 
+    # These two wrap the mock in a real async def instead of passing the mock
+    # straight to call_with_body.
+    #
+    # 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 -- the failure is
+    # in signature(), not in spec resolution.
+    #
+    # call_with_body catches Exception and converts it into a 500 JSONResponse,
+    # so on 3.10 these failed as `assert <JSONResponse object> == {'ok': True}`,
+    # with the real TypeError visible only inside the response body.
+    #
+    # The wrapper declares (*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 the call assertions.
+
     @pytest.mark.asyncio
     async def test_allows_plain_dict_body(self):
         """Non-empty plain dict passes through unchanged (no model_fields_set)."""
         body = {"key": "value"}
 
-        func = AsyncMock(return_value={"ok": True})
+        mock = AsyncMock(return_value={"ok": True})
+
+        async def func(*args, **kwargs):
+            return await mock(*args, **kwargs)
+
         result = await call_with_body(func, body)
 
         assert result == {"ok": True}
@@ -354,8 +376,12 @@ async def test_allows_empty_plain_dict_body(self):
         """
         body = {}
 
-        func = AsyncMock(return_value={"ok": True})
+        mock = AsyncMock(return_value={"ok": True})
+
+        async def func(*args, **kwargs):
+            return await mock(*args, **kwargs)
+
         result = await call_with_body(func, body)
 
         assert result == {"ok": True}
-        func.assert_called_once_with()
+        mock.assert_called_once_with()

tests/unit/core/resources/test_resource_manager_extended.py

@@ -9,6 +9,17 @@
 )
 
 
+class _LegacyResource:
+    """Minimal picklable stand-in for a resource in the pre-1.12 state file.
+
+    Module-level, not defined inside a test: cloudpickle serialises a class
+    defined in a function body by value, which is the thing being avoided here.
+    """
+
+    def __init__(self, config_hash: str):
+        self.config_hash = config_hash
+
+
 @pytest.fixture(autouse=True)
 def reset_manager(isolate_resource_state_file, reset_singletons):
     """Reset singleton and state file between tests."""
@@ -56,7 +67,13 @@ def test_loads_legacy_dict_format(self, tmp_path):
         state_file = tmp_path / ".runpod" / "resources.pkl"
         state_file.parent.mkdir(parents=True)
 
-        resources = {"key1": MagicMock(config_hash="hash1")}
+        # A plain object rather than MagicMock: whether cloudpickle can pickle a
+        # Mock is version-dependent, and on 3.10 this failed with
+        # "Could not pickle object as excessively deep recursion required".
+        # Only .config_hash is read here (via _refresh_config_hashes), and the
+        # absence of get_resource_key is what keeps the legacy key un-migrated,
+        # which is exactly what this test is asserting.
+        resources = {"key1": _LegacyResource("hash1")}
         with open(state_file, "wb") as f:
             cloudpickle.dump(resources, f)
 

tests/unit/test_deployment.py

@@ -195,19 +195,44 @@ async def slow_deploy(resource):
             assert results[0].duration >= 0.09
 
     def test_deploy_all_background(self, mock_resources):
-        """Test background deployment doesn't block."""
+        """Test background deployment starts a daemon thread without blocking."""
         orchestrator = DeploymentOrchestrator()
 
-        with patch.object(
-            orchestrator.manager, "get_or_deploy_resource", new_callable=AsyncMock
-        ) as mock_deploy:
+        # threading.Thread is stubbed rather than left to run for real.
+        #
+        # deploy_all_background() starts a daemon thread and returns
+        # immediately, so a real thread outlives this test: the patch below
+        # would be lifted while the thread was still starting, and the thread
+        # would then call the *unpatched* manager and register these
+        # MagicMock(spec=ServerlessResource) objects into the ResourceManager
+        # singleton -- during whichever unrelated test happened to be running
+        # at that moment. ResourceManager._save_resources() cloudpickles its
+        # whole state on every registration, and a MagicMock cannot be pickled,
+        # so an arbitrary later test died with
+        #
+        #   _pickle.PicklingError: args[0] from __newobj__ args has the wrong class
+        #
+        # Which test got hit depended on thread scheduling and on xdist's
+        # worker assignment, which is what made it flaky rather than simply
+        # broken. Stubbing the thread keeps the assertion this test actually
+        # makes -- that the call is non-blocking and spawns a daemon thread --
+        # and lets nothing escape the test.
+        with (
+            patch("runpod_flash.core.deployment.threading.Thread") as mock_thread_cls,
+            patch.object(
+                orchestrator.manager, "get_or_deploy_resource", new_callable=AsyncMock
+            ) as mock_deploy,
+        ):
             mock_deploy.side_effect = mock_resources
 
             # Should not block
             orchestrator.deploy_all_background(mock_resources)
 
-            # Background thread should be started
-            # (not much we can test here without waiting for thread)
+            # A daemon thread was started, and nothing ran inline.
+            mock_thread_cls.assert_called_once()
+            assert mock_thread_cls.call_args.kwargs["daemon"] is True
+            mock_thread_cls.return_value.start.assert_called_once()
+            mock_deploy.assert_not_called()
 
     def test_deploy_all_background_empty_list(self):
         """Test background deployment with empty list."""