Skip to content

ci: test every Python version the package promises - #1342

Open
davidberenstein1957 wants to merge 4 commits into
masterfrom
scaling/06-ci-python-matrix
Open

ci: test every Python version the package promises#1342
davidberenstein1957 wants to merge 4 commits into
masterfrom
scaling/06-ci-python-matrix

Conversation

@davidberenstein1957

@davidberenstein1957 davidberenstein1957 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Part of #1338.

requires-python = ">=3.10" (pyproject.toml:9) and the classifiers advertise 3.10 through 3.14. The matrix ran ["3.12", "3.13", "3.14"]. Two of five promised versions were exercised by nothing — confirmed across all workflows; package-validation-reusable.yml pins 3.12 for both its jobs and nothing else touches the SDK matrix.

Honest framing: no user is broken today

I tried to produce a break on 3.10 and could not. A clean 3.10 venv resolved and installed from source with only declared runtime dependencies; import codecarbon worked; codecarbon --help worked; a real OfflineEmissionsTracker run returned emissions (5.95e-07 kg). Full non-integration suite on the same machine: 3.10 → 619 passed / 5 failed, 3.12 → 623 passed / 1 failed, with non-overlapping failure sets that all pass in isolation — macOS shared-state ordering flakes, not version incompatibilities. Same 624 tests collected on both. Grepping for the usual hazards (datetime.UTC, tomllib, StrEnum, ExceptionGroup, itertools.batched, match) found zero hits in codecarbon/.

So what is broken is the enforcement, not the code: nothing stops the first datetime.UTC from shipping. This is the cheapest moment to fix it, precisely because 3.10 is green and widening the matrix costs no fix-the-code work.

Changes

  • test-package.yml — matrix widened to ["3.10", "3.11", "3.12", "3.13", "3.14"], plus fail-fast: false so a single-version failure is diagnosable rather than confusing.
  • package-validation-reusable.yml — new MIN_PYTHON_VERSION: "3.10" and a bare-install-smoke job that installs only the built wheel into an empty 3.10 venv, then runs import, codecarbon --help, and a 2s OfflineEmissionsTracker. This closes a second real gap: the existing test-package-from-wheel job runs uv sync --group dev before installing the wheel, so a runtime dependency missing from [project.dependencies] but present in the lockfile goes unnoticed. It runs on PRs, since package.yml calls this workflow on pull_request.
  • tests/test_gpu_amd.py — out of the stated scope of this PR, flagged explicitly so nobody is surprised by it in the diff. It is not a matrix change; it is the fix required to make 3.10 and 3.11 green, and without it this PR cannot go in. _import_gpu_amd_with_amdsmi_error re-imports codecarbon.core.gpu_amd with a poisoned __import__ and then restores sys.modules. That is not enough: importlib.import_module also rebinds gpu_amd as an attribute of the parent codecarbon.core package, and on Python < 3.12 mock.patch("codecarbon.core.gpu_amd.<x>") resolves its target by walking that attribute chain rather than by sys.modules lookup. The stale binding therefore sent every later test's patches to the throwaway module, so the AMD tests failed on 3.10/3.11 while passing on 3.12+. The teardown now restores the parent attribute too. Nine lines, test-only, no runtime code touched.

bare-install-smoke asserts a result, not just the absence of a crash

The first revision only printed tracker.stop(). Since stop() returns None on every internal give-up path — tracker never started, another instance holding the lock, emissions engine failed to initialise — that job would have printed emissions: None and still exited 0, i.e. stayed green on a wheel whose tracker does not work at all. It now asserts the shape of the result and deliberately tolerates a zero:

assert isinstance(emissions, float), f'tracker.stop() returned {emissions!r}'
assert emissions >= 0.0, emissions

No magnitude assertion. A runner with no RAPL falls back to constant CPU power, and 0.0 over a 2-second window is a legitimate outcome there; asserting > 0 would be the flaky version of this check. Verified locally on a bare install: returns 5.46e-07, assertions pass.

Cost

Three matrix jobs become five. Roughly +12-18 CI minutes per triggering PR, plus ~2 min for the smoke job; wall clock unchanged since matrix jobs run in parallel, and this is a public repo so the minutes are free.

Choices against the original proposal

  • Widened the matrix rather than raising the floor to 3.12. Raising it is user-visible breakage justified by evidence I could not find — the code demonstrably runs on 3.10.
  • Skipped the Ruff UP rule change. ruff check --select UP --target-version py310 codecarbon/ reports 552 errors, 521 auto-fixable, essentially all cosmetic (Tupletuple) — and UP does not flag from datetime import UTC at all, so it would not catch the hazard used to motivate it. A 500-line stylistic diff that misses the stated threat isn't a cheap first line of defence.
  • Skipped the coverage include/primary split. Codecov merges matrix uploads rather than double-counting; the repo already uploads three times today.

Verified vs. unverified

Verified locally: both workflow files parse as YAML, the matrix expands to exactly five entries, the smoke script survives YAML block-scalar and python -c "…" shell quoting (round-tripped through yaml.safe_load), and its substance succeeds on a bare install here. Full suite 626 passed, 21 skipped; uv run pre-commit run --all-files passes.

Unverified (cannot run Actions locally): that uv python install 3.10/3.11 resolve on ubuntu-24.04 (very likely — uv ships its own builds); that the suite passes on 3.10/3.11 on Linux CI rather than macOS; and that the smoke job's tracker run succeeds on a runner with no RAPL. If 3.10 or 3.11 fails on Linux in a way it doesn't here, this becomes a fix-the-code task — fail-fast: false is what makes that legible. If five interpreters per PR proves too slow, moving 3.10/3.11 to push-and-nightly is the escape hatch; I did not pre-build it.

Still open

Deciding the requires-python floor before any extras split lands, and test-package.yml:43 installing dash/fire by hand instead of via the [carbonboard] extra, which lets packaging metadata drift from what CI actually installs.

🤖 Generated with Claude Code

requires-python is >=3.10 and the classifiers list 3.10-3.14, but
test-package.yml only exercised 3.12-3.14. Add 3.10 and 3.11 to the
matrix with fail-fast: false so one version's failure does not hide
the others.

Also add a bare-install smoke job to package-validation-reusable.yml:
the existing wheel job syncs the dev group before installing the
wheel, so it cannot catch a runtime dependency that is missing from
[project.dependencies]. The new job installs only the wheel, on the
floor interpreter, and runs an OfflineEmissionsTracker plus the CLI.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.43%. Comparing base (065d0e6) to head (027ad65).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1342      +/-   ##
==========================================
+ Coverage   91.39%   91.43%   +0.03%     
==========================================
  Files          49       49              
  Lines        5056     5056              
==========================================
+ Hits         4621     4623       +2     
+ Misses        435      433       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

davidberenstein1957 and others added 2 commits August 12, 2026 17:31
The amdsmi import-failure tests re-import codecarbon.core.gpu_amd with a
patched __import__ and restore sys.modules afterwards, but importlib also
rebinds gpu_amd on the codecarbon.core package, and that binding was left
pointing at the throwaway module whose amdsmi is None.

On Python < 3.12 mock.patch resolves a dotted target through getattr on the
parent package, so every later patch of codecarbon.core.gpu_amd.amdsmi hit
the throwaway module while the code under test still used the real one,
failing with AttributeError: 'NoneType' object has no attribute 'amdsmi_init'.
Python 3.12+ resolves via pkgutil.resolve_name and hid the leak.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tracker.stop() returns None on every internal give-up path, so printing it
left the smoke job green on a silently broken wheel. Assert the shape rather
than the magnitude, since 0.0 is legitimate on a runner without RAPL.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@davidberenstein1957
davidberenstein1957 marked this pull request as ready for review August 12, 2026 19:14
@davidberenstein1957
davidberenstein1957 requested a review from a team as a code owner August 12, 2026 19:14
The new bare-install-smoke job lives in a workflow the `package` paths
filter does not watch, so it never ran on this PR and would first
execute on master. Watch the two workflow files too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant