docs: tests/README.md and Makefile for the test suite#626
Conversation
Two ergonomic helpers around the existing pytest tree, no behavior change to any production code: tests/README.md Lays out the directory, explains how to run the full suite on a Linux+CPU box (no GPU / Mac / Metal needed), and documents how the hardware-aware suites simulate every accelerator family by spoofing torch.cuda.is_available, torch.xpu.is_available, torch.backends.mps.is_available, torch.version.hip, platform.system, platform.machine, and sys.modules['mlx']. Calls out the test_active_merge_device_matrix.py PROFILES dataclass as the extension point for adding new hardware combinations. Makefile - make install-test pip install -e . + pytest + CPU torch - make test full pytest run - make test-fast quiet run (one line per file) - make test-matrix accelerator cascade dispatch matrix only - make test-mlx-sim MLX-on-torch shim suite only - make test-merge pure-torch LoRA / MoE merge regression only - make format ruff format + kwarg spacing pipeline Both are pure additions: no existing file is modified.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5020f0290
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| install-test: | ||
| $(PYTHON) -m pip install --upgrade pip | ||
| $(PYTHON) -m pip install torch --index-url https://download.pytorch.org/whl/cpu | ||
| $(PYTHON) -m pip install -e . pytest |
There was a problem hiding this comment.
Install unsloth before running test targets
In a fresh clone that follows make install-test, this installs unsloth_zoo and pytest but not the separate unsloth package. The suite imports unsloth_zoo.__init__, which raises ImportError("Please install Unsloth...") when importlib.find_spec('unsloth') is absent, so the advertised make install-test && make test-matrix path still errors unless the developer already has unsloth installed outside this target.
Useful? React with 👍 / 👎.
| $(PYTHON) -m pytest tests/test_unsloth_zoo_lora_merge.py tests/test_forward_native_moe_loop_lora.py -v | ||
|
|
||
| format: | ||
| $(PYTHON) scripts/run_ruff_format.py $$(git ls-files '*.py') |
There was a problem hiding this comment.
Point format at an existing formatter
The new format target always fails because scripts/run_ruff_format.py is not present in the repository (I searched the tree for that filename), and running make format exits with Python's can't open file ... [Errno 2] No such file or directory. Since help advertises this as the canonical formatting command, contributors cannot use the documented target until it invokes an existing tool or the script is added.
Useful? React with 👍 / 👎.
| ## Quick start | ||
|
|
||
| ```bash | ||
| pip install -e . pytest torch --index-url https://download.pytorch.org/whl/cpu |
There was a problem hiding this comment.
Keep PyPI available when installing pytest
For a clean environment, this single pip command routes all non-local package lookups through the PyTorch CPU index; pip install --help documents --index-url as replacing the default PyPI base URL, with --extra-index-url needed to add another index. That means pytest and project dependencies that are not mirrored by download.pytorch.org cannot be resolved, so the quick-start install can fail before users ever run the tests.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request introduces a Makefile for task automation and a comprehensive README for the tests directory, detailing the hardware-aware simulation suite. Feedback suggests improving the 'format' target in the Makefile to recursively find Python files and correcting the pip installation instructions in the documentation to ensure dependencies are correctly resolved when using the PyTorch CPU index.
| $(PYTHON) -m pytest tests/test_unsloth_zoo_lora_merge.py tests/test_forward_native_moe_loop_lora.py -v | ||
|
|
||
| format: | ||
| $(PYTHON) scripts/run_ruff_format.py $$(git ls-files '*.py') |
There was a problem hiding this comment.
The current git ls-files '*.py' command only matches Python files in the root directory. To ensure the formatting script processes all Python files across the entire repository (including unsloth_zoo/ and tests/), you should use a recursive pathspec like ':/*.py'. Additionally, note that git ls-files will skip any new, untracked files.
$(PYTHON) scripts/run_ruff_format.py $$(git ls-files ':/*.py')
| ## Quick start | ||
|
|
||
| ```bash | ||
| pip install -e . pytest torch --index-url https://download.pytorch.org/whl/cpu |
There was a problem hiding this comment.
Using --index-url replaces the default PyPI index. Since pytest and the dependencies of unsloth_zoo (like transformers or bitsandbytes) are not hosted on the PyTorch CPU index, this combined command will fail to find them. It is safer to install torch separately or use --extra-index-url to allow fallback to PyPI.
| pip install -e . pytest torch --index-url https://download.pytorch.org/whl/cpu | |
| pip install torch --index-url https://download.pytorch.org/whl/cpu && pip install -e . pytest |
Summary
tests/README.mddocumenting the layout, the GPU-free harness, and how the hardware-aware suites (test_active_merge_device_matrix.py,test_mlx_torch_shim_smoke.py, thetest_pr_a_*family) simulate every accelerator family on a Linux+CPU box.Makefilewith one-liners for the common test invocations (test,test-matrix,test-mlx-sim,test-merge,format,install-test).Why
The post-#620
tests/tree now has 174 tests across pure-torch LoRA merge regression, MLX-on-torch simulation, the_active_merge_device()cascade matrix, and the Gemma-4 MoE LoRA extractor. Running them is a one-liner once you know the right invocation, but there is no documented entry point. This PR captures the conventions so reviewers can run the right subset without trial and error.The
Makefilealso pins the canonical install ladder (CPU torch from the official PyTorch index, thenpip install -e . pytest) so contributors with no GPU on hand can validate changes locally.Test plan
make helplists all targets.make testruns the full suite (174 tests, ~8s on CPU).make test-matrixruns the 9 cascade tests (~4s).make test-mlx-simruns the 105 MLX-shim PR-A tests (~7s).make test-mergeruns the 16 pure-torch LoRA / MoE merge regression tests (~6s).make formatis a no-op against the current tree (idempotent).tests/README.mdrenders correctly on GitHub.