From 259c527cfb907e121649390bd6c30535f47471b1 Mon Sep 17 00:00:00 2001 From: runpod-Henrik Date: Wed, 26 Aug 2026 16:46:02 -0700 Subject: [PATCH 01/11] fix(ci): run each matrix leg on its own Python version 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 --- .github/workflows/ci.yml | 11 +++++++++++ uv.lock | 6 ++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 522ff5ac7..61a6aa6f1 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/uv.lock b/uv.lock index a2e48d3f1..4db98f5a6 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'", @@ -824,7 +824,7 @@ name = "exceptiongroup" version = "1.3.1" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "typing-extensions" }, + { name = "typing-extensions", marker = "python_full_version < '3.13'" }, ] sdist = { url = "https://files.pythonhosted.org/packages/50/79/66800aadf48771f6b62f7eb014e352e5d06856655206165d775e675a02c9/exceptiongroup-1.3.1.tar.gz", hash = "sha256:8b412432c6055b0b7d14c310000ae93352ed6754f70fa8f7c34141f91c4e3219", size = 30371, upload-time = "2025-11-21T23:01:54.787Z" } wheels = [ @@ -2669,6 +2669,7 @@ dependencies = [ { name = "rich" }, { name = "runpod" }, { name = "tomli", marker = "python_full_version < '3.11'" }, + { name = "tomlkit" }, { name = "typer" }, ] @@ -2698,6 +2699,7 @@ requires-dist = [ { name = "rich", specifier = ">=14.0.0" }, { name = "runpod", git = "https://github.com/runpod/runpod-python?rev=main" }, { name = "tomli", marker = "python_full_version < '3.11'", specifier = ">=2.0.0" }, + { name = "tomlkit", specifier = ">=0.13.0" }, { name = "typer", specifier = ">=0.12.0" }, ] From 95d3b2df4ea8d1465cdeb90d599270c39e984ff6 Mon Sep 17 00:00:00 2001 From: runpod-Henrik Date: Wed, 26 Aug 2026 16:46:17 -0700 Subject: [PATCH 02/11] fix(tests): stop test_deploy_all_background leaking a thread 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 ``. 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 --- tests/unit/test_deployment.py | 37 +++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_deployment.py b/tests/unit/test_deployment.py index 081ca0b96..15c479c69 100644 --- a/tests/unit/test_deployment.py +++ b/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.""" From 0625cc9386a999ead1729f546973a93cc32f929a Mon Sep 17 00:00:00 2001 From: runpod-Henrik Date: Wed, 26 Aug 2026 16:58:52 -0700 Subject: [PATCH 03/11] fix(tests): repair three tests that only ever ran on 3.11 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 == {'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 --- .../cli/commands/test_run_server_helpers.py | 32 +++++++++++++++++-- .../test_resource_manager_extended.py | 19 ++++++++++- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/tests/unit/cli/commands/test_run_server_helpers.py b/tests/unit/cli/commands/test_run_server_helpers.py index 60ae9a14d..80a3feb60 100644 --- a/tests/unit/cli/commands/test_run_server_helpers.py +++ b/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 == {'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() diff --git a/tests/unit/core/resources/test_resource_manager_extended.py b/tests/unit/core/resources/test_resource_manager_extended.py index 06f2ac1eb..fe24a8f85 100644 --- a/tests/unit/core/resources/test_resource_manager_extended.py +++ b/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) From c9c0b8838c95e62233fb2977277db3822f754137 Mon Sep 17 00:00:00 2001 From: runpod-Henrik Date: Tue, 25 Aug 2026 11:06:32 -0700 Subject: [PATCH 04/11] fix: include serial tests in coverage.xml and upload it from CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coverage run is split in two: parallel tests with --cov-report=xml, then serial tests with --cov-append but only --cov-report=term-missing. The second run appends to .coverage without re-emitting the XML, so coverage.xml was left holding the parallel run alone. Measured locally: 85.1% (8085/9503) before the append is re-emitted vs 85.6% (8132/9503) after — 47 covered lines from the 19 serial-marked tests were being dropped from the report. Also upload coverage.xml so the weekly coverage report can read the number from the newest successful run on main instead of it being collected by hand. `if-no-files-found: error` so a silently-missing report fails the step rather than publishing an empty artifact. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 11 +++++++++++ Makefile | 8 ++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 61a6aa6f1..da79bad11 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -270,6 +270,17 @@ jobs: name: test-results-${{ matrix.python-version }} path: pytest-results-*.xml + # Consumed by the weekly coverage report, which reads the artifact from + # the newest successful run on main. Name is per-matrix-version because + # upload-artifact@v4 requires unique names within a run. + - name: Upload coverage report + uses: actions/upload-artifact@v4 + if: always() + with: + name: coverage-${{ matrix.python-version }} + path: coverage.xml + if-no-files-found: error + build: name: Build Package runs-on: ubuntu-latest diff --git a/Makefile b/Makefile index c764d4d48..acdbe8c65 100644 --- a/Makefile +++ b/Makefile @@ -73,7 +73,9 @@ test-integration-serial: # Run integration tests serially (for debugging) 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 + # 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-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 @@ -127,7 +129,9 @@ ci-quality-github: # Quality checks with GitHub Actions formatting (parallel by 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 @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. + uv run pytest tests/ --junitxml=pytest-results-serial.xml -v -m "serial" --cov=runpod_flash --cov-append --cov-report=term-missing --cov-report=xml @echo "::endgroup::" ci-quality-github-serial: # Serial quality checks for GitHub Actions (for debugging) From 8474be9bba760faecf87bc7ce2f0b406de1a6e69 Mon Sep 17 00:00:00 2001 From: runpod-Henrik Date: Tue, 25 Aug 2026 15:56:34 -0700 Subject: [PATCH 05/11] chore: render coverage numbers into the job summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coverage report was only readable by downloading the artifact. Write a Test Results + Coverage Summary table to $GITHUB_STEP_SUMMARY so the numbers show up on the run page, matching what the ai-api component workflow does. Reads both pytest-results-parallel.xml and pytest-results-serial.xml, so the counts cover the whole suite rather than one pass. Stdlib only, so there is no extra install step, and `if: always()` means the summary still renders when tests fail — which is when it is most useful. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index da79bad11..23c7b51e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -263,6 +263,62 @@ jobs: - name: Quality checks run: make ci-quality-github + - name: Coverage summary + if: always() + run: | + python - coverage.xml 'pytest-results-*.xml' <<'PY' >> "$GITHUB_STEP_SUMMARY" + import glob, sys, xml.etree.ElementTree as ET + from pathlib import Path + + cov, junit = sys.argv[1], sys.argv[2:] + o = ["## Coverage Report", ""] + + t = f = s = 0 + seen = False + for pat in junit: + for path in sorted(glob.glob(pat)): + try: + r = ET.parse(path).getroot() + except ET.ParseError: + continue + seen = True + for su in (r.iter("testsuite") if r.tag == "testsuites" else [r]): + t += int(su.get("tests") or 0) + f += int(su.get("failures") or 0) + int(su.get("errors") or 0) + s += int(su.get("skipped") or 0) + if seen: + o += ["### Test Results", "", + "| Status | Passed | Failed | Skipped | Total |", + "|---|---:|---:|---:|---:|", + f"| {'✅ Passed' if not f else '❌ Failed'} | {t - f - s} | {f} | {s} | {t} |", ""] + + p = Path(cov) + if not p.exists(): + o += [f"> No coverage report at `{cov}`."] + else: + r = ET.parse(p).getroot() + c, v = int(r.get("lines-covered") or 0), int(r.get("lines-valid") or 0) + o += ["### Coverage Summary", "", "| Metric | Value |", "|---|---:|", + f"| Line coverage | **{(100.0 * c / v if v else 0):.1f}%** |", + f"| Lines covered | {c:,} / {v:,} |", + f"| Lines missing | {v - c:,} |", ""] + rows = [] + for cl in r.iter("class"): + ls = list(cl.iter("line")) + if ls: + h = sum(1 for x in ls if int(x.get("hits") or 0) > 0) + rows.append((100.0 * h / len(ls), + cl.get("filename") or cl.get("name") or "?", h, len(ls))) + rows.sort() + if rows: + o += ["
", + f"Per-file coverage ({len(rows)} files, least covered first)", + "", "| File | Coverage | Covered / Total |", "|---|---:|---:|"] + o += [f"| `{n}` | {q:.1f}% | {h} / {tt} |" for q, n, h, tt in rows] + o += ["", "
", ""] + print("\n".join(o)) + PY + - name: Upload test results uses: actions/upload-artifact@v4 if: always() From 44b4bcbe344cfa2389818bfeef2bc11b1b32ac42 Mon Sep 17 00:00:00 2001 From: runpod-Henrik Date: Wed, 26 Aug 2026 08:28:42 -0700 Subject: [PATCH 06/11] fix: harden the coverage summary step and document the coverage gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses all four review comments on this PR. Summary step (same three defects, all reachable because the step runs under `if: always()`): * the coverage-XML parse was unguarded while the junit parse beside it was. A report truncated by a timeout, OOM or crashed xdist worker made ET.parse raise, so the step exited non-zero and stacked a spurious failure on top of the real one. Verified: the old script exits 1 on a truncated report, the new one exits 0 and reports it in the summary. * attributes were parsed with bare int(), so a malformed value raised rather than degrading. * the status cell treated `0 failures` as passing even when no tests ran. Upload now uses `if-no-files-found: warn`. With `error`, a run that never produced coverage.xml — pytest erroring at collection before pytest-cov writes anything — failed the upload too, masking the root cause. This upload hangs off the PR-gating job, so it should not fail a PR on its own. Makefile: documented the two asymmetries flagged in review. * the serial line, not the parallel one, is the real coverage gate: the parallel pass passes --cov-fail-under=0 while the serial pass inherits 65 from pyproject addopts. Aligning the two flags would quietly disable the gate, so that is now written down. * make stops at the first failing recipe line, so a failing parallel pass means the serial re-emit never runs and coverage.xml holds the parallel subset. Left as-is deliberately rather than guarding with `-`/`|| true`: that would let a red parallel suite exit 0, which is worse than an undercount on a run that already failed. The weekly report only reads runs whose status is success, so nothing consumes that undercount. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 60 +++++++++++++++++++++++++++++----------- Makefile | 12 ++++++++ 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 23c7b51e8..16f6e75b3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -270,51 +270,79 @@ jobs: import glob, sys, xml.etree.ElementTree as ET from pathlib import Path + MAX_ROWS = 300 cov, junit = sys.argv[1], sys.argv[2:] o = ["## Coverage Report", ""] + + def num(v, default=0): + """Attribute values are text, and a malformed one must degrade rather than + raise: this step runs under `if: always()`, so an exception here would + stack a spurious failure on top of whatever actually went wrong.""" + try: + return int(v) + except (TypeError, ValueError): + return default + + + def parse(path): + try: + return ET.parse(path).getroot(), None + except (ET.ParseError, OSError) as e: + return None, e + + t = f = s = 0 seen = False for pat in junit: for path in sorted(glob.glob(pat)): - try: - r = ET.parse(path).getroot() - except ET.ParseError: + r, _ = parse(path) + if r is None: continue seen = True for su in (r.iter("testsuite") if r.tag == "testsuites" else [r]): - t += int(su.get("tests") or 0) - f += int(su.get("failures") or 0) + int(su.get("errors") or 0) - s += int(su.get("skipped") or 0) + t += num(su.get("tests")) + f += num(su.get("failures")) + num(su.get("errors")) + s += num(su.get("skipped")) if seen: + # A suite dying at import reports errors>0 with tests possibly 0, so + # "no failures AND something actually ran" is what keeps a crashed run + # from rendering as passed. + ok = f == 0 and t > 0 o += ["### Test Results", "", "| Status | Passed | Failed | Skipped | Total |", "|---|---:|---:|---:|---:|", - f"| {'✅ Passed' if not f else '❌ Failed'} | {t - f - s} | {f} | {s} | {t} |", ""] + f"| {'✅ Passed' if ok else '❌ Failed'} | {t - f - s} | {f} | {s} | {t} |", ""] p = Path(cov) - if not p.exists(): - o += [f"> No coverage report at `{cov}`."] + root, err = (None, None) if not p.exists() else parse(p) + if root is None: + o += [f"> No coverage report at `{cov}`." if err is None + else f"> Coverage report at `{cov}` could not be parsed: {err}"] else: - r = ET.parse(p).getroot() - c, v = int(r.get("lines-covered") or 0), int(r.get("lines-valid") or 0) + c, v = num(root.get("lines-covered")), num(root.get("lines-valid")) o += ["### Coverage Summary", "", "| Metric | Value |", "|---|---:|", f"| Line coverage | **{(100.0 * c / v if v else 0):.1f}%** |", f"| Lines covered | {c:,} / {v:,} |", f"| Lines missing | {v - c:,} |", ""] rows = [] - for cl in r.iter("class"): + for cl in root.iter("class"): ls = list(cl.iter("line")) if ls: - h = sum(1 for x in ls if int(x.get("hits") or 0) > 0) + h = sum(1 for x in ls if num(x.get("hits")) > 0) rows.append((100.0 * h / len(ls), cl.get("filename") or cl.get("name") or "?", h, len(ls))) - rows.sort() + rows.sort(key=lambda r: (r[0], r[1])) if rows: + shown = rows[:MAX_ROWS] o += ["
", f"Per-file coverage ({len(rows)} files, least covered first)", "", "| File | Coverage | Covered / Total |", "|---|---:|---:|"] - o += [f"| `{n}` | {q:.1f}% | {h} / {tt} |" for q, n, h, tt in rows] + o += [f"| `{n}` | {q:.1f}% | {h} / {tt} |" for q, n, h, tt in shown] + # Capped so a growing repo cannot push the summary past GitHub's 1 MiB + # limit, which would truncate it silently. + if len(rows) > MAX_ROWS: + o += [f"| _…and {len(rows) - MAX_ROWS} more files_ | | |"] o += ["", "
", ""] print("\n".join(o)) PY @@ -335,7 +363,7 @@ jobs: with: name: coverage-${{ matrix.python-version }} path: coverage.xml - if-no-files-found: error + if-no-files-found: warn build: name: Build Package diff --git a/Makefile b/Makefile index acdbe8c65..28644c24f 100644 --- a/Makefile +++ b/Makefile @@ -131,6 +131,18 @@ ci-quality-github: # Quality checks with GitHub Actions formatting (parallel by @echo "::group::Test suite with coverage (serial pass)" # 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-append --cov-report=term-missing --cov-report=xml @echo "::endgroup::" From 7600d9c6ad4ca68f19bb32dd24804db0faae3dc3 Mon Sep 17 00:00:00 2001 From: runpod-Henrik Date: Wed, 26 Aug 2026 12:56:25 -0700 Subject: [PATCH 07/11] feat: collect branch coverage Branch coverage was never collected, so the job summary could only ever show line coverage. Adds --cov-branch. Purely additive: line coverage is unchanged (verified per repo), so the weekly trend, which reads line coverage, is unaffected. The Cobertura report now carries branches-valid/covered, which the summary renders as its own row. Note --cov-fail-under gates on coverage.py total, which now blends lines and branches, so that number drops even though line coverage does not. Measured before committing: this repo stays comfortably above its gate. Co-Authored-By: Claude Opus 5 (1M context) --- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 28644c24f..46b253b39 100644 --- a/Makefile +++ b/Makefile @@ -72,10 +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 -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-append --cov-report=term-missing --cov-report=xml + 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 @@ -126,7 +126,7 @@ 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)" # Re-emit the XML after appending the serial tests, or coverage.xml is left @@ -143,7 +143,7 @@ ci-quality-github: # Quality checks with GitHub Actions formatting (parallel by # `-`/`|| 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-append --cov-report=term-missing --cov-report=xml + 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) From 7d22164ac834e72c056e0f7650e8044996f5c325 Mon Sep 17 00:00:00 2001 From: runpod-Henrik Date: Wed, 26 Aug 2026 13:49:41 -0700 Subject: [PATCH 08/11] refactor: use the shared coverage-summary action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the inline summary script with runpod/internal-workflows/.github/actions/coverage-summary, which now owns both the summary rendering and the artifact upload. The same ~90-line script had been copy-pasted into six repos, and review found the same bugs in every copy — four rounds of a single class where "passed" was the default state and each new way of losing a report had to be patched out of it separately. The shared version computes it from positive evidence instead, and has 59 unit tests plus a smoke job behind it. Pinned by commit rather than tag, so a change to the action cannot reach this repo until someone bumps the SHA. Behaviour was proven on runpod/github-image-builder first: green run, artifact uploaded, and the weekly collector parsed it back at the expected number before the remaining repos followed. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 104 +++------------------------------------ 1 file changed, 7 insertions(+), 97 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 16f6e75b3..8e0d0eba3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -263,106 +263,16 @@ jobs: - name: Quality checks run: make ci-quality-github + # Shared with the other repos' coverage jobs. Pinned by commit rather + # than tag: a change to the action cannot reach us until we bump it. - name: Coverage summary if: always() - run: | - python - coverage.xml 'pytest-results-*.xml' <<'PY' >> "$GITHUB_STEP_SUMMARY" - import glob, sys, xml.etree.ElementTree as ET - from pathlib import Path - - MAX_ROWS = 300 - cov, junit = sys.argv[1], sys.argv[2:] - o = ["## Coverage Report", ""] - - - def num(v, default=0): - """Attribute values are text, and a malformed one must degrade rather than - raise: this step runs under `if: always()`, so an exception here would - stack a spurious failure on top of whatever actually went wrong.""" - try: - return int(v) - except (TypeError, ValueError): - return default - - - def parse(path): - try: - return ET.parse(path).getroot(), None - except (ET.ParseError, OSError) as e: - return None, e - - - t = f = s = 0 - seen = False - for pat in junit: - for path in sorted(glob.glob(pat)): - r, _ = parse(path) - if r is None: - continue - seen = True - for su in (r.iter("testsuite") if r.tag == "testsuites" else [r]): - t += num(su.get("tests")) - f += num(su.get("failures")) + num(su.get("errors")) - s += num(su.get("skipped")) - if seen: - # A suite dying at import reports errors>0 with tests possibly 0, so - # "no failures AND something actually ran" is what keeps a crashed run - # from rendering as passed. - ok = f == 0 and t > 0 - o += ["### Test Results", "", - "| Status | Passed | Failed | Skipped | Total |", - "|---|---:|---:|---:|---:|", - f"| {'✅ Passed' if ok else '❌ Failed'} | {t - f - s} | {f} | {s} | {t} |", ""] - - p = Path(cov) - root, err = (None, None) if not p.exists() else parse(p) - if root is None: - o += [f"> No coverage report at `{cov}`." if err is None - else f"> Coverage report at `{cov}` could not be parsed: {err}"] - else: - c, v = num(root.get("lines-covered")), num(root.get("lines-valid")) - o += ["### Coverage Summary", "", "| Metric | Value |", "|---|---:|", - f"| Line coverage | **{(100.0 * c / v if v else 0):.1f}%** |", - f"| Lines covered | {c:,} / {v:,} |", - f"| Lines missing | {v - c:,} |", ""] - rows = [] - for cl in root.iter("class"): - ls = list(cl.iter("line")) - if ls: - h = sum(1 for x in ls if num(x.get("hits")) > 0) - rows.append((100.0 * h / len(ls), - cl.get("filename") or cl.get("name") or "?", h, len(ls))) - rows.sort(key=lambda r: (r[0], r[1])) - if rows: - shown = rows[:MAX_ROWS] - o += ["
", - f"Per-file coverage ({len(rows)} files, least covered first)", - "", "| File | Coverage | Covered / Total |", "|---|---:|---:|"] - o += [f"| `{n}` | {q:.1f}% | {h} / {tt} |" for q, n, h, tt in shown] - # Capped so a growing repo cannot push the summary past GitHub's 1 MiB - # limit, which would truncate it silently. - if len(rows) > MAX_ROWS: - o += [f"| _…and {len(rows) - MAX_ROWS} more files_ | | |"] - o += ["", "
", ""] - print("\n".join(o)) - PY - - - name: Upload test results - uses: actions/upload-artifact@v4 - if: always() - with: - name: test-results-${{ matrix.python-version }} - path: pytest-results-*.xml - - # Consumed by the weekly coverage report, which reads the artifact from - # the newest successful run on main. Name is per-matrix-version because - # upload-artifact@v4 requires unique names within a run. - - name: Upload coverage report - uses: actions/upload-artifact@v4 - if: always() + uses: runpod/internal-workflows/.github/actions/coverage-summary@a9dcd07aabffbe72fbb6a12d7baeb9fa8150a0bf with: - name: coverage-${{ matrix.python-version }} - path: coverage.xml + format: cobertura + coverage-file: coverage.xml + results: pytest-results-*.xml + artifact-name: coverage-${{ matrix.python-version }} if-no-files-found: warn build: From eb12ff83566bc802c0c580d7acba42adc3227c49 Mon Sep 17 00:00:00 2001 From: runpod-Henrik Date: Wed, 26 Aug 2026 13:53:42 -0700 Subject: [PATCH 09/11] revert: keep the inline coverage summary here MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A public repository cannot consume an action from a private one, and runpod/internal-workflows is private. Every matrix leg failed at "Set up job" with: Unable to resolve action `runpod/internal-workflows`, not found This reverts the migration to the shared action for this repo. The inline script is restored, including all the hardening from review. The shared action still applies to the private repos — github-image-builder is migrated and green, main-ui and RunPod follow. Sharing it here needs internal-workflows to be public, or the action published somewhere public. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 104 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 97 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8e0d0eba3..16f6e75b3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -263,16 +263,106 @@ jobs: - name: Quality checks run: make ci-quality-github - # Shared with the other repos' coverage jobs. Pinned by commit rather - # than tag: a change to the action cannot reach us until we bump it. - name: Coverage summary if: always() - uses: runpod/internal-workflows/.github/actions/coverage-summary@a9dcd07aabffbe72fbb6a12d7baeb9fa8150a0bf + run: | + python - coverage.xml 'pytest-results-*.xml' <<'PY' >> "$GITHUB_STEP_SUMMARY" + import glob, sys, xml.etree.ElementTree as ET + from pathlib import Path + + MAX_ROWS = 300 + cov, junit = sys.argv[1], sys.argv[2:] + o = ["## Coverage Report", ""] + + + def num(v, default=0): + """Attribute values are text, and a malformed one must degrade rather than + raise: this step runs under `if: always()`, so an exception here would + stack a spurious failure on top of whatever actually went wrong.""" + try: + return int(v) + except (TypeError, ValueError): + return default + + + def parse(path): + try: + return ET.parse(path).getroot(), None + except (ET.ParseError, OSError) as e: + return None, e + + + t = f = s = 0 + seen = False + for pat in junit: + for path in sorted(glob.glob(pat)): + r, _ = parse(path) + if r is None: + continue + seen = True + for su in (r.iter("testsuite") if r.tag == "testsuites" else [r]): + t += num(su.get("tests")) + f += num(su.get("failures")) + num(su.get("errors")) + s += num(su.get("skipped")) + if seen: + # A suite dying at import reports errors>0 with tests possibly 0, so + # "no failures AND something actually ran" is what keeps a crashed run + # from rendering as passed. + ok = f == 0 and t > 0 + o += ["### Test Results", "", + "| Status | Passed | Failed | Skipped | Total |", + "|---|---:|---:|---:|---:|", + f"| {'✅ Passed' if ok else '❌ Failed'} | {t - f - s} | {f} | {s} | {t} |", ""] + + p = Path(cov) + root, err = (None, None) if not p.exists() else parse(p) + if root is None: + o += [f"> No coverage report at `{cov}`." if err is None + else f"> Coverage report at `{cov}` could not be parsed: {err}"] + else: + c, v = num(root.get("lines-covered")), num(root.get("lines-valid")) + o += ["### Coverage Summary", "", "| Metric | Value |", "|---|---:|", + f"| Line coverage | **{(100.0 * c / v if v else 0):.1f}%** |", + f"| Lines covered | {c:,} / {v:,} |", + f"| Lines missing | {v - c:,} |", ""] + rows = [] + for cl in root.iter("class"): + ls = list(cl.iter("line")) + if ls: + h = sum(1 for x in ls if num(x.get("hits")) > 0) + rows.append((100.0 * h / len(ls), + cl.get("filename") or cl.get("name") or "?", h, len(ls))) + rows.sort(key=lambda r: (r[0], r[1])) + if rows: + shown = rows[:MAX_ROWS] + o += ["
", + f"Per-file coverage ({len(rows)} files, least covered first)", + "", "| File | Coverage | Covered / Total |", "|---|---:|---:|"] + o += [f"| `{n}` | {q:.1f}% | {h} / {tt} |" for q, n, h, tt in shown] + # Capped so a growing repo cannot push the summary past GitHub's 1 MiB + # limit, which would truncate it silently. + if len(rows) > MAX_ROWS: + o += [f"| _…and {len(rows) - MAX_ROWS} more files_ | | |"] + o += ["", "
", ""] + print("\n".join(o)) + PY + + - name: Upload test results + uses: actions/upload-artifact@v4 + if: always() + with: + name: test-results-${{ matrix.python-version }} + path: pytest-results-*.xml + + # Consumed by the weekly coverage report, which reads the artifact from + # the newest successful run on main. Name is per-matrix-version because + # upload-artifact@v4 requires unique names within a run. + - name: Upload coverage report + uses: actions/upload-artifact@v4 + if: always() with: - format: cobertura - coverage-file: coverage.xml - results: pytest-results-*.xml - artifact-name: coverage-${{ matrix.python-version }} + name: coverage-${{ matrix.python-version }} + path: coverage.xml if-no-files-found: warn build: From 6b9046b74faf070860ee22053b198eb00a9d6fa7 Mon Sep 17 00:00:00 2001 From: runpod-Henrik Date: Thu, 27 Aug 2026 10:30:00 -0700 Subject: [PATCH 10/11] refactor: use the shared coverage-summary action from its public repo Replaces the ~90-line inline heredoc summary with the shared action, now that there is a host this repo can actually consume. The first attempt at this pointed at runpod/internal-workflows, which is private, and failed at `Set up job` on every matrix leg: Unable to resolve action `runpod/internal-workflows`, not found No tests ran at all, so it was reverted. A public repository cannot resolve an action from a private one, and this repo is public. The action now lives in runpod/coverage-summary-action, which is public, and the same commit is already green in three private consumers. Two things differ from the reverted attempt: * pinned to the public repo's commit rather than internal-workflows' * the "Upload test results" step is kept. The earlier attempt dropped it, which would have silently stopped publishing test-results- artifacts. The action uploads one artifact; coverage and test results are two, so that step stays separate. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 111 +++++++-------------------------------- 1 file changed, 19 insertions(+), 92 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 16f6e75b3..0ae3f8682 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -263,89 +263,27 @@ 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() - run: | - python - coverage.xml 'pytest-results-*.xml' <<'PY' >> "$GITHUB_STEP_SUMMARY" - import glob, sys, xml.etree.ElementTree as ET - from pathlib import Path - - MAX_ROWS = 300 - cov, junit = sys.argv[1], sys.argv[2:] - o = ["## Coverage Report", ""] - - - def num(v, default=0): - """Attribute values are text, and a malformed one must degrade rather than - raise: this step runs under `if: always()`, so an exception here would - stack a spurious failure on top of whatever actually went wrong.""" - try: - return int(v) - except (TypeError, ValueError): - return default - - - def parse(path): - try: - return ET.parse(path).getroot(), None - except (ET.ParseError, OSError) as e: - return None, e - - - t = f = s = 0 - seen = False - for pat in junit: - for path in sorted(glob.glob(pat)): - r, _ = parse(path) - if r is None: - continue - seen = True - for su in (r.iter("testsuite") if r.tag == "testsuites" else [r]): - t += num(su.get("tests")) - f += num(su.get("failures")) + num(su.get("errors")) - s += num(su.get("skipped")) - if seen: - # A suite dying at import reports errors>0 with tests possibly 0, so - # "no failures AND something actually ran" is what keeps a crashed run - # from rendering as passed. - ok = f == 0 and t > 0 - o += ["### Test Results", "", - "| Status | Passed | Failed | Skipped | Total |", - "|---|---:|---:|---:|---:|", - f"| {'✅ Passed' if ok else '❌ Failed'} | {t - f - s} | {f} | {s} | {t} |", ""] - - p = Path(cov) - root, err = (None, None) if not p.exists() else parse(p) - if root is None: - o += [f"> No coverage report at `{cov}`." if err is None - else f"> Coverage report at `{cov}` could not be parsed: {err}"] - else: - c, v = num(root.get("lines-covered")), num(root.get("lines-valid")) - o += ["### Coverage Summary", "", "| Metric | Value |", "|---|---:|", - f"| Line coverage | **{(100.0 * c / v if v else 0):.1f}%** |", - f"| Lines covered | {c:,} / {v:,} |", - f"| Lines missing | {v - c:,} |", ""] - rows = [] - for cl in root.iter("class"): - ls = list(cl.iter("line")) - if ls: - h = sum(1 for x in ls if num(x.get("hits")) > 0) - rows.append((100.0 * h / len(ls), - cl.get("filename") or cl.get("name") or "?", h, len(ls))) - rows.sort(key=lambda r: (r[0], r[1])) - if rows: - shown = rows[:MAX_ROWS] - o += ["
", - f"Per-file coverage ({len(rows)} files, least covered first)", - "", "| File | Coverage | Covered / Total |", "|---|---:|---:|"] - o += [f"| `{n}` | {q:.1f}% | {h} / {tt} |" for q, n, h, tt in shown] - # Capped so a growing repo cannot push the summary past GitHub's 1 MiB - # limit, which would truncate it silently. - if len(rows) > MAX_ROWS: - o += [f"| _…and {len(rows) - MAX_ROWS} more files_ | | |"] - o += ["", "
", ""] - print("\n".join(o)) - PY + 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 @@ -354,17 +292,6 @@ jobs: name: test-results-${{ matrix.python-version }} path: pytest-results-*.xml - # Consumed by the weekly coverage report, which reads the artifact from - # the newest successful run on main. Name is per-matrix-version because - # upload-artifact@v4 requires unique names within a run. - - name: Upload coverage report - uses: actions/upload-artifact@v4 - if: always() - with: - name: coverage-${{ matrix.python-version }} - path: coverage.xml - if-no-files-found: warn - build: name: Build Package runs-on: ubuntu-latest From b920a4e0c59a8ca338eee3e90c2e086d8db86562 Mon Sep 17 00:00:00 2001 From: Justin Date: Mon, 26 Oct 2026 19:08:51 -0400 Subject: [PATCH 11/11] fix(ci): measure branch coverage from config and keep Makefile comments out of logs - Set branch = true in [tool.coverage.run] and drop the four --cov-branch flags, so every target, including the -serial ones, measures the same thing against the 65% gate. - Move the coverage comment blocks above test-coverage and ci-quality-github. Tab-indented, make echoed all of them into every CI log. --- Makefile | 40 ++++++++++++++++++++-------------------- pyproject.toml | 2 +- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/Makefile b/Makefile index 46b253b39..5907bd546 100644 --- a/Makefile +++ b/Makefile @@ -71,11 +71,11 @@ test-integration: # Run integration tests in parallel (auto-detect cores) test-integration-serial: # Run integration tests serially (for debugging) uv run pytest tests/integration/ -v -m integration +# The serial pass re-emits the XML after appending, or coverage.xml is left +# holding only the parallel run and undercounts. 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-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 + 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 --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 @@ -118,6 +118,20 @@ quality-check-strict: format-check lint typecheck test-coverage # Strict quality quality-check-serial: ci-quality-github-serial # Serial quality gate for debugging # GitHub Actions specific target -- canonical CI quality gate. +# The serial pass re-emits the XML after appending, or coverage.xml is left +# holding only the parallel run and undercounts. +# +# The serial pass, not the parallel one, is the real coverage gate: the +# parallel invocation passes --cov-fail-under=0 to suppress the partial +# number, while the serial one inherits --cov-fail-under=65 from pyproject +# addopts. Do not "align" the two flags -- that quietly disables the gate. +# +# make stops at the first failing recipe line, so if the parallel pass fails +# the re-emit never runs and coverage.xml holds the parallel subset. That is +# deliberate: guarding the parallel line with `-`/`|| true` so the serial pass +# 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. ci-quality-github: # Quality checks with GitHub Actions formatting (parallel by default) @echo "::group::Code formatting check" uv run ruff format --check . @@ -126,24 +140,10 @@ 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-branch --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-report=xml --cov-fail-under=0 @echo "::endgroup::" @echo "::group::Test suite with coverage (serial pass)" - # 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 + uv run pytest tests/ --junitxml=pytest-results-serial.xml -v -m "serial" --cov=runpod_flash --cov-append --cov-report=term-missing --cov-report=xml @echo "::endgroup::" ci-quality-github-serial: # Serial quality checks for GitHub Actions (for debugging) diff --git a/pyproject.toml b/pyproject.toml index 30eac1bd2..ff5743c43 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -119,7 +119,7 @@ ignore_missing_imports = true [tool.coverage.run] parallel = true -branch = false +branch = true source = ["runpod_flash"] omit = ["*/tests/*", "*/test_*.py", "*/__pycache__/*", "*/site-packages/*"]