Repository navigation
test: fix two tests broken by pytest 9.2.0 - #4222
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
|
| Layer / File(s) | Summary |
|---|---|
Dynamo limit setup and checks tests/core/trainers/diffusion_lm/test_encoder_canvas.py |
The test uses torch._dynamo.config.patch(recompile_limit=8) and retains checks for dense and flex-attention construction, including preservation of a manually set limit. |
Quantized loss comparison
| Layer / File(s) | Summary |
|---|---|
Scalar loss comparison tests/utils/test_moun8bit.py |
The trajectory test compares quant_loss.item() with the dense loss using the existing approximate tolerance. |
Priority:
Estimated code review effort: 2 (Simple) | ~5 minutes
Change: Bug fix
Possibly related PRs
- axolotl-ai-cloud/axolotl#4220: It addresses pytest failures in tests exercised by this pull request’s CI.
Suggested reviewers: ved1beta
Merge Risk: 
Minimal · previous review da3c5
-
The test updates are ready to merge; reported CI coverage passed with the fixes.
Pre-merge checks: 
5
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
pytest 9.2's approx iterates the expected value to check for Decimal tolerances, which raises on the 0-d tensors _step returns.
From pytest 9.2, monkeypatch reads the old value statically, finds nothing on torch._dynamo.config (whose keys go through __getattr__) and undoes the patch with delattr, which hides the key: every later compile on that xdist worker failed with "torch._dynamo.config.recompile_limit does not exist" (test_chunked_xentropy on CI). Dynamo's own config.patch restores it.
The 9.1.1 pin from #4221 was a stopgap for the two tests fixed in this branch. With them passing under pytest 9.2, CI goes back to the latest pytest.
da3c56d to
2c756e7
Compare
Description
pytest 9.2.0, released 2026-10-27, breaks two of main's tests. CI installs pytest unpinned; main's last green runs used 9.1.1.
tests/utils/test_moun8bit.py::test_trajectory_tracks_dist_muon: pytest 9.2'sapproxiterates the expected value to look forDecimaltolerances. The test passed 0-d tensors, so it raisedTypeError: iteration over a 0-d tensor. It now compares the losses as floats (.item()).tests/core/trainers/diffusion_lm/test_encoder_canvas.py::test_flex_backend_raises_the_dynamo_recompile_budget: from pytest 9.2,monkeypatchreads the old value statically. It finds nothing ontorch._dynamo.config, whose keys go through__getattr__, so its undo callsdelattr, and torch'sConfigModule.__delattr__hides the key. Every latertorch.compileon that xdist worker then failed withtorch._dynamo.config.recompile_limit does not exist. On CI that showed up intests/test_chunked_xentropy.py::test_chunked_forward. The test now uses dynamo's ownconfig.patch(recompile_limit=8), which restores the value.Besides the two test files, the last commit removes the
pytest==9.1.1pin that #4221 added as a stopgap (pyproject.toml'stestgroup, the CPU workflows and both Dockerfiles), so CI runs the latest pytest again. The branch is rebased onto main after #4221.Motivation and Context
This is maintainer work (Wing). Both failures surfaced while getting CI on #4220 green, and they hit main and every open PR once CI picks up pytest 9.2.0. The same two fixes are on #4220; this PR lands them on main without waiting for it. #4221 pinned
pytest==9.1.1as a stopgap; this PR removes that pin.How has this been tested?
Locally, on macOS with Python 3.11 and torch 2.14.0, in a venv with pytest 9.2.0:
torch._dynamo.configstill hasrecompile_limit. With main's version of the test, the check fails; with this branch's, it passes andrecompile_limitis back at its default. With pytest 8.4.2, neither version of the test hides the key.approxon tensors: under pytest 9.2.0,tensor(0.501) == pytest.approx(tensor(0.5), rel=1e-2)raises the sameTypeError, and the float comparison passes.Not run locally:
test_trajectory_tracks_dist_muonandtest_chunked_xentropyneed inductor's C++ compile, which fails on this Mac. On #4220, CI ran both with these fixes under pytest 9.2.0 on Python 3.12 / torch 2.13 and Python 3.14 / torch 2.14, and they passed.AI Usage Disclaimer
Do not open this PR, including as a draft, until a human has reviewed the complete diff. The contributor may perform this review; a separate reviewer is not required. This also applies to maintainer-directed work.
Yes. Claude Code (Claude Opus) found the two failures in #4220's CI, traced them to pytest 9.2.0, wrote and verified the fixes, and prepared this branch. Wing reviewed the diff.
Screenshots (if appropriate)
Types of changes
Social Handles (Optional)
Summary by CodeRabbit