Skip to content

tox: stop py3-unit re-pinning to Python 3.11, let the CI matrix actually run - #712

Open
UgaTheDev wants to merge 1 commit into
instructlab:mainfrom
UgaTheDev:fix/tox-py3-unit-basepython
Open

tox: stop py3-unit re-pinning to Python 3.11, let the CI matrix actually run#712
UgaTheDev wants to merge 1 commit into
instructlab:mainfrom
UgaTheDev:fix/tox-py3-unit-basepython

Conversation

@UgaTheDev

@UgaTheDev UgaTheDev commented Aug 24, 2026

Copy link
Copy Markdown

unit.yaml runs tox -e py3-unit across a matrix.python of 3.11, 3.12,
and 3.13 (three separate jobs, each with actions/setup-python installing
the matching interpreter). But [testenv:py3-unit] in tox.ini sets
basepython = python3.11, so tox re-pins every leg back to 3.11 regardless
of which interpreter setup-python put on PATH. All three matrix legs
currently exercise the same interpreter — the 3.12/3.13 legs are decorative.

This removes that pin. With no basepython override, tox's uv-venv-runner
uses the interpreter already on PATH for that job — i.e. whatever
actions/setup-python set up for the current matrix leg — so the three CI
jobs actually diverge.

Scope is intentionally narrow: only py3-unit. py3-smoke (tox.ini line 53)
keeps its basepython = python3.11 pin — it isn't part of the version matrix
in smoke.yaml/smoke-py312.yaml and shouldn't silently start floating.

Verified locally: tox -l still lists py3-unit with the same commands;
only the interpreter selection changes. No other env is touched.

Originally flagged in #705 (comment from 2026-07-29): "the 3.11/3.12/3.13 matrix is decorative: [testenv:py3-unit] in tox.ini sets basepython = python3.11, so tox re-pins the interpreter to 3.11 on all three legs no matter what setup-python installed."

Fixes the tox side of the CI-matrix finding in #705.

Summary by CodeRabbit

  • Chores
    • Updated the Python 3 unit-test environment to use tox’s default interpreter selection.

unit.yaml runs tox -e py3-unit across a matrix.python of 3.11/3.12/3.13,
but [testenv:py3-unit] pinned basepython to python3.11, silently
re-pinning every leg back to 3.11 regardless of what setup-python
installed. Removing the pin lets py3-unit use whatever interpreter is
already on PATH for that job, so the three CI legs actually diverge.
py3-smoke keeps its own basepython pin unchanged.

Ref: instructlab#705

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Kush Zingade <kush.zingade@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 50a42213-f3b2-4d65-80ba-d7815450ad7f

📥 Commits

Reviewing files that changed from the base of the PR and between da76404 and d1123df.

📒 Files selected for processing (1)
  • tox.ini
💤 Files with no reviewable changes (1)
  • tox.ini

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The py3-unit tox environment no longer sets basepython to Python 3.11. Tox now selects the interpreter by default.

Changes

Tox interpreter selection

Layer / File(s) Summary
Use tox default interpreter selection
tox.ini
The py3-unit environment no longer explicitly selects Python 3.11. Tox uses its default interpreter selection.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to d1123

The change allows each py3-unit CI matrix job to use its configured Python version while leaving other environments unchanged; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: removing the Python 3.11 pin so the CI matrix can use each configured Python version.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mergify mergify Bot added CI/CD Affects CI/CD configuration testing Relates to testing labels Aug 24, 2026
@UgaTheDev

Copy link
Copy Markdown
Author

All four of these (#712, #713, #714, #715) are mergeable with zero human reviews — and CodeRabbit rate-limited on three of them, so they've had no review at all. Since the constraint looks like review bandwidth rather than the changes themselves: want me to consolidate the four into a single PR so it's one review pass instead of four? They're independent commits, so either shape works — say the word and I'll have it up the same day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI/CD Affects CI/CD configuration testing Relates to testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant