chore: add tox.ini for local development#38
Conversation
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
|
Thanks for the pull request, @salman2013! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #38 +/- ##
=======================================
Coverage 82.17% 82.17%
=======================================
Files 48 48
Lines 1419 1419
Branches 110 110
=======================================
Hits 1166 1166
Misses 221 221
Partials 32 32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a tox.ini configuration to make it easy to run the repo’s existing linting, tests (with coverage), and docs build locally via tox, using uv-native tox environments (tox-uv).
Changes:
- Introduces
lint,test, anddocstox environments. - Uses
uv-venv-lock-runnerwith extras mapped to existing optional-dependency groups (dev,test,docs).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I think we can follow two approaches for command orchestration:
[tox]
envlist = lint, test, docs
requires =
tox-uv>=1
[testenv:lint]
runner = uv-venv-lock-runner
extras = dev
allowlist_externals = make
commands =
make lint
[testenv:test]
runner = uv-venv-lock-runner
extras = test
allowlist_externals = make
commands =
make test-with-coverage
[testenv:docs]
runner = uv-venv-lock-runner
extras = docs
allowlist_externals = make
commands =
make docs
I think we should follow second option: Reasons:
@feanil @kdmccormick @openedx/axim-aximprovements |
|
@farhan I agree, I think the 2nd option is the way to go. Tox for codifying the testing env and then makefile for convenience/consistency aliases for calling Tox. |
|
@salman2013 Let's go for second option then Please study xblocks-core tox.ini to run tests for different environments; different django versions; and keep it consistent in all the repositories. |
- Makefile and CI now delegate to `tox -e <env>` instead of running commands directly - Add `allowlist_externals = make` to [testenv:docs] for tox v4 compatibility - Add `uv tool install tox --with tox-uv` to `make requirements` Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
|
We should add all the environments in the tox supported by the repository. |
Adds PEP 735 dependency groups to test against Django 4.2 and 5.2 (matching the CI matrix), following the same pattern as openedx/XBlock. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
- Replace separate lint/test/docs CI jobs with a single matrix job
over [lint, docs, django42, django52] running in parallel
- Use uv sync --group ci instead of uv tool install tox
- Simplify tox envs from py312-django{42,52} to django{42,52}
using [testenv] default with dependency group factors
- Add ci dependency group to lock tox/tox-uv versions
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Matches XBlock's approach of using changedir = {toxinidir}/docs
with make html instead of make -C docs html.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
farhan
left a comment
There was a problem hiding this comment.
Some change requests, rest seems good,
We are close to merge I believe
| @@ -0,0 +1,26 @@ | |||
| [tox] | |||
| envlist = django{42,52}, lint, docs | |||
There was a problem hiding this comment.
I think we should add python version explicitly
py312-django{42,52}
There was a problem hiding this comment.
Its takes default python version so don't need to mention.
There was a problem hiding this comment.
alright i have updated it.
| test-with-coverage: ## Run tests with coverage reporting | ||
| uv run pytest --cov=$(SRC_DIRECTORY) --cov-report=xml | ||
| test: ## Run tests against all supported Python/Django combinations | ||
| tox -e "django{42,52}" |
There was a problem hiding this comment.
Same:
I think we should add python version explicitly
py312-django{42,52}
There was a problem hiding this comment.
Its takes default python version so don't need to mention.
- Set fail_ci_if_error: true so coverage upload failures surface in CI
- Remove explicit {group = "doc"} from uv conflicts; uv derives it
automatically since doc includes test via include-group
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
v6.0.1 had a stale GPG signing key causing CI failures. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>



Summary
Modernizes the local development and CI tooling to use tox as the single source of truth for running lint, tests, and docs — matching the approach used in the XBlock repo itself.
tox.iniwithlint,django42,django52, anddocsenvironments usinguv-venv-lock-runner[project.optional-dependencies]test/docs extras with[dependency-groups](test-base,test,django42,doc,ci) for proper Django version isolationcidependency group (tox+tox-uv) so CI installs only what it needs viauv sync --group cilint+test) to a singlerun_testsmatrix job over[lint, docs, django42, django52]make lint,make test,make docsall delegate to tox)fail_ci_if_error: trueon Codecov upload so coverage failures surface in CIcodecov/codecov-actionpin from v6.0.1 → v6.0.2 (fixes GPG key verification failure)uv.lockconflicts betweentestanddjango42groups (uv derives thedocconflict automatically viainclude-group)Test plan
make lintpasses (tox -e lint)make testpasses (tox -e django42,django52)make docspasses (tox -e docs)lint,docs,django42,django52