unsloth/tests/studio/install/test_diffusers_pin.py
Daniel Han 6c124b6dd8
Run the repo CPU test suite on all four runner cores (#9019)
* Fix a 500 on dictation Unload before any backend is resident

stt_unload passes expected_model positionally:

    _, unload_stt = _stt_lifecycle()
    failed = await asyncio.to_thread(unload_stt, engines, model)

_stt_lifecycle() returns the orchestrator's unload_stt_model when a backend is
resident and stt_registry.unload when one is not. Only the first takes
expected_model positionally; on the registry it sits behind a `*`:

    def unload(engines = None, *, wait = True, expected_model = None)
    def unload_stt_model(self, engines = None, expected_model = None)

So with nothing loaded yet, which is what a fresh process is, Unload raises
TypeError and the route answers 500. Reproduced outside pytest:

    peek_inference_backend() fresh process: None
    stt_unload -> TypeError: unload() takes from 0 to 1 positional arguments
                             but 2 were given

Pass it by keyword, which both callables accept.

Found while profiling CI, not by the tests, and the tests are why: this file's
two unload tests passed only because an earlier test in the full suite had left
a backend resident. Standalone they failed, and in their own file they failed;
they went green only inside the full serial run. That order dependence hid a
live bug. The new test drives the no-backend path directly, so neither the
signature nor the call site can drift back.

    before: 2 failed, 53 passed   (file alone)
    after:  56 passed

* Run the repo CPU test suite on all four runner cores

Backend CI is the most expensive workflow in the repo at 126.7 runner-minutes
per push. Install is cached at ~1.2 minutes, so almost all of it is pytest.
ubuntu-latest has 4 vCPUs and this suite is CPU-bound and GPU-free, so it was
running on one core of four.

On a real runner the step goes 17.75 minutes to 7.67, and the failure set is
unchanged: the same single pre-existing failure, serial and parallel, on the
same tree in the same staging repo. Locally, on the dependency shape the
workflow installs, 806.1s to 219.7s with an identical 2 failed / 8066 passed /
178 skipped / 40 subtests result.

The three small pytest steps in this job stay serial: the hardware-spoof step
exists because those files mutate hardware.py module globals, and all three
already measure under 0.1 minutes.

The four-interpreter matrix is deliberately left alone. It is the bigger prize
at ~79 runner-minutes, but measured under -n 4 on a real runner it fails tests
serial does not, in more than one way and not the same way twice, so it needs
its order dependencies found and fixed first rather than a flag.

* Pin the two unload signatures against the route's one call site

The bug fixed in the previous commit exists because _stt_lifecycle returns two
different callables and the route has a single call site:

  def unload(engines = None, *, wait = True, expected_model = None)   # stt_registry
  def unload_stt_model(self, engines = None, expected_model = None)   # orchestrator

A call that suits one is a TypeError on the other, and which one runs depends on
whether a backend happens to be resident, so the broken half only appears on a
fresh process. That will recur the next time either signature is edited.

This binds BOTH real signatures against the arguments the route actually passes.
It does not demand they be identical, only that one call site can serve both.

Checked against two mutations:

  registry grows a keyword-only param  -> passes (benign, and it should)
  registry drops expected_model        -> fails, "got an unexpected keyword
                                          argument 'expected_model'"

Paired with the test beside it: that one covers the call site (route -> registry
on a cold process), this one covers the two callees staying compatible.

57 passed.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Give each node harness run its own script file

Reported on this PR, and reproduced before changing anything. Every _run in
test_chat_preset_builtin_invariants.py wrote the same TEMP/run.mts and then
executed "node run.mts" from TEMP. Under pytest-xdist's default load
distribution two of this file's nine tests can land on different workers, so one
worker executes the script the other just wrote over the top of it.

Pinned to four cores, running the file at -n 4:

  before: 5 failed / 6 failed / 5 failed / 5 failed / 6 failed   (of 9)
  after:  9 passed, six runs out of six

Serial is unchanged at 9 passed either way, which is why the full-suite runs
that back this PR did not show it: at ~8000 tests across four workers these nine
rarely collide, so it is a latent flake rather than a reliable failure. That is
worse, not better, and enabling -n 4 is what would have started rolling the dice.

A unique NAME rather than a per-call directory, which is what
tests/studio/_node_harness.py::run_harness does for every other node harness
here. Those scripts reach the frontend sources by a relative path counted from
TEMP, so an extra directory level breaks every import with ERR_MODULE_NOT_FOUND
(tried it: 9 failed). register.mjs and loader.mjs stay shared because their
contents are fixed, so a concurrent rewrite writes identical bytes.

Swept the other five fixed TEMP roots under tests/studio: all of them already
allocate per call through tempfile.mkdtemp, directly or via run_harness. This
file was the only one rolling its own.

* Shorten the harness comment

* Stop two more shared-state races the parallel repo job would expose

Both reported on this PR, both real, both confirmed by reading the code.

1. tests/studio/test_chat_preset_builtin_invariants.py still rewrote the shared
   register.mjs and loader.mjs on every _run. Only run.mts became unique in the
   previous commit. write_text truncates before it writes, so a worker rewriting
   one of those while another worker's node process is importing it can hand that
   process an empty or partial module. The contents are constant, which is why I
   wrongly called a concurrent rewrite harmless: identical bytes still arrive
   after a truncation. They are now written through a temp file and os.replace,
   so every reader sees one whole version.

2. tests/python/test_no_torch_filtering.py::TestRealRequirementsFiltering had an
   autouse fixture that snapshotted the requirements directory and, at teardown,
   deleted every filtered file that had appeared since. Those files land in the
   REAL requirements directory, not tmp_path, so under xdist one test's teardown
   removes a file another worker is still reading. It now records the paths it
   creates and removes only those.

  test_chat_preset_builtin_invariants.py:  9 passed serial, 9 passed on four -n 4 runs
  test_no_torch_filtering.py:             65 passed serial, 65 passed on four -n 4 runs
  no filtered files left behind

* Run the event-loop latency tests outside the parallel invocation

Reported on this PR. tests/studio/load_freeze asserts upper bounds on real
elapsed time -- a /health burst under 250 ms while a 600 ms blocking probe runs,
and a 100-request burst under 350 ms -- and those bounds ARE the contract, so
they cannot be loosened without the tests ceasing to test anything. A pytest
worker descheduled by the other three inflates them.

Measured before changing anything, under 3x CPU oversubscription (8 spinners
pinned to the same 4 cores): 8 ms against the 250 ms bound, 38 ms against the
350 ms one, 3 runs, no failures. So the margin is wide on this box. It is not
wide enough to leave alone: a runner core is several times slower, which puts
the 350 ms bound within reach of an unlucky schedule. The directory is now
ignored from the -n 4 run and runs in its own serial step, 20s against the
~10 minutes -n 4 saves on this job.

The ignore and the step are two edits held together by nothing, and losing the
step is silent -- the job stays green while 23 tests stop running. New guard
tests/studio/test_backend_ci_parallel_isolation.py fails if an isolated path is
missing from the parallel run's ignores, or ignored with no serial step running
it, for load_freeze and for the three hardware-spoof files that were already in
this shape. Deleting either half of the load_freeze change fails it; a third
test pins the command scan so it cannot pass by matching nothing.

  load_freeze:            23 passed in 20.14s serial
  new guard:               9 passed, and 1 failed under each of the two mutations
  test_ci_shell_suite_coverage.py + test_xpu_spoof_pipeline.py: 58 passed

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Raise the matrix hang limit past the suite it was cancelling

Not a review item; found while triaging why every PR in this stack carries a red
check. The Backend CI matrix limit was 30 minutes, set when the step measured
14.1 to 15.0. It now measures 22.5 to 29.2, and it is cancelling: across the six
open PRs on this stack, five had exactly one leg cancelled at 30m9s while its
siblings passed at 25 to 29 minutes. Which leg loses is luck, so the red says
nothing about the PR, and a cancelled leg burns the full 30 minutes for no
signal.

45, which is what the Windows UI job already uses. This is a hang guard, not a
budget, and it costs nothing until a job needs it. The real fix is running this
matrix on all four cores, worth ~79 runner-minutes a push, and it stays blocked
on the order dependencies this stack is working through one at a time.

* Give each PowerShell probe its own script path

Reported on this PR, real, and reproduced directly. _run_capturing_bytes wrote to
tests/python/_{stem}_probe_{shape}.ps1, a name several tests share, so under
xdist one case unlinks the script another has written before that one's pwsh
child opens it. pwsh is installed on ubuntu-latest, so these do not skip there.

Reverting to the shared name and running -n 4 four times: 1 failed each time.
With a uuid in the name:

  35 passed, 8 skipped serial
  35 passed, 8 skipped on each of four -n 4 runs
  no probe scripts left behind

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Stop the pin scanner reading a generated filtered file as a second pin source

Reported on this PR and reproduced. test_no_torch_filtering.py exercises
install_python_stack._filter_requirements, which writes
`.{stem}-filtered-XXXX.txt` BESIDE the source on purpose, so relative -r/-c
includes still resolve. test_diffusers_pin.py::test_only_the_pin_file_names_diffusers
scans REQ_ROOT.rglob("*.txt"), so under xdist it can see one worker's copy of
diffusers-pin.txt and report it as a second source of the pin.

Calling _filter_requirements on diffusers-pin.txt and then running that test
fails exactly as described. It is not only an xdist problem: the function passes
delete=False, so the file also survives a real install on any machine.

The scan now skips dotfiles. That matches what it means -- no CHECKED-IN
requirements file other than the pin may name diffusers -- rather than narrowing
it: a real second source still fails it (verified with a probe file), and the
production write path is untouched, since writing beside the source is the
behaviour that makes includes resolve.

  5 passed with a filtered file present, 1 failed with a real second source

* Exempt only the generated filter files, not every dotfile

Reported on this PR and right: a checked-in hidden requirements file such as
.constraints.txt is a real place the pin could be overridden from, and the
blanket dotfile skip took it out of the scan.

Matched by the shape _filter_requirements actually writes now: a dot, the source
stem, "-filtered-", and NamedTemporaryFile's 8-character suffix.

  generated filtered file present:      5 passed
  checked-in .constraints.txt naming diffusers: 1 failed

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2026-08-16 21:46:01 -07:00

139 lines
6 KiB
Python

# SPDX-License-Identifier: AGPL-3.0-only
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
"""The pinned Diffusers revision has to survive a fresh install.sh, not just an update.
MiniMax-H3 needs a Diffusers revision newer than any published release, and Studio
refuses to load it otherwise. The pin originally lived in
studio/backend/requirements/base.txt, which did not reach fresh install.sh installs at
the time. base.txt now reaches those installs as an independent shared phase, but it
still runs too early to hold this pin safely.
These tests pin the shape that fixes it: exactly one file names diffusers, and the step
that installs it sits outside every skip.
"""
from __future__ import annotations
import ast
import pathlib
import re
REPO_ROOT = pathlib.Path(__file__).resolve().parents[3]
REQ_ROOT = REPO_ROOT / "studio" / "backend" / "requirements"
PIN_FILE = REQ_ROOT / "diffusers-pin.txt"
# The shape install_python_stack._filter_requirements writes: a dot, the source stem,
# "-filtered-", then tempfile's random suffix. NamedTemporaryFile's suffixes are
# [A-Za-z0-9_]{8}, so this cannot swallow a checked-in file that merely starts with a dot.
_GENERATED_FILTER = re.compile(r"\.[\w.-]+-filtered-\w{8}\.txt")
STACK = REPO_ROOT / "studio" / "install_python_stack.py"
INSTALL_SH = REPO_ROOT / "install.sh"
def _requirements(path: pathlib.Path) -> list[str]:
"""Requirement lines only: comments and flag lines dropped."""
out = []
for line in path.read_text(encoding = "utf-8").splitlines():
text = line.split("#", 1)[0].strip()
if text and not text.startswith("-"):
out.append(text)
return out
def test_the_pin_file_exists_and_names_an_exact_revision():
assert PIN_FILE.is_file(), f"{PIN_FILE} is missing"
lines = _requirements(PIN_FILE)
urls = [line for line in lines if "://" in line]
assert len(urls) == 1, f"expected exactly one pinned URL, got {urls}"
# A branch or tag would move under us; only a 40-char commit sha is reproducible.
assert re.search(
r"/archive/[0-9a-f]{40}\.zip", urls[0]
), f"the diffusers pin must name a full commit sha, not a moving ref: {urls[0]}"
assert 'python_version >= "3.10"' in urls[0], (
"diffusers dropped Python 3.9 in 0.38, so the archive needs a >= 3.10 marker or "
"the resolver has no candidate at all on a 3.9 host"
)
def test_only_the_pin_file_names_diffusers():
"""One source of truth. A second entry anywhere is how a release creeps back in:
whichever step runs last wins, and the step order is not obvious from any one file."""
offenders = {}
for path in sorted(REQ_ROOT.rglob("*.txt")):
if path == PIN_FILE:
continue
# install_python_stack._filter_requirements writes `.{stem}-filtered-XXXX.txt`
# BESIDE the source on purpose, so relative -r/-c includes still resolve, and it
# does not delete it. So a copy of the pin file can be sitting here while this
# runs -- transiently under pytest-xdist, where another worker is exercising that
# function, and durably on any machine that has run a real install. It is a
# generated temp, not a second source of the pin.
# Matched by that exact shape rather than by "starts with a dot": a checked-in
# hidden file such as .constraints.txt is a real requirements file and a real
# place the pin could be overridden from, so it stays in the scan.
if _GENERATED_FILTER.fullmatch(path.name):
continue
named = [line for line in _requirements(path) if line.lower().startswith("diffusers")]
if named:
offenders[str(path.relative_to(REPO_ROOT))] = named
assert not offenders, (
f"diffusers is requirement-listed outside diffusers-pin.txt: {offenders}. "
f"Move it into the pin file so the dedicated late step remains authoritative."
)
def test_the_pin_step_is_not_gated_by_skip_base_or_no_torch():
"""The pin must sit at function top level so it reaches every install path."""
tree = ast.parse(STACK.read_text(encoding = "utf-8"))
def _installs_pin(node: ast.AST) -> bool:
for call in ast.walk(node):
if not isinstance(call, ast.Call):
continue
if getattr(call.func, "id", None) != "pip_install":
continue
for kw in call.keywords:
if kw.arg == "req" and "diffusers-pin.txt" in ast.dump(kw.value):
return True
return False
found = False
for func in ast.walk(tree):
if not isinstance(func, ast.FunctionDef):
continue
for stmt in func.body: # top level of the function only, no if/else nesting
if _installs_pin(stmt):
found = True
assert found, (
"no unconditional pip_install of diffusers-pin.txt found at the top level of any "
"function in install_python_stack.py. Nested under an `if`, the pin can miss an "
"install path."
)
def test_the_pin_step_runs_after_every_other_requirements_install():
"""Ordering matters: a later `uv pip install -r ...` can re-resolve diffusers back to a
release. Keeping the pin last means nothing is left that could walk it forward."""
source = STACK.read_text(encoding = "utf-8")
pin_at = source.index("diffusers-pin.txt")
later = [
name
for name in (
"extras.txt",
"extras-no-deps.txt",
"studio.txt",
"base.txt",
"no-torch-runtime.txt",
"data-designer-deps.txt",
"data-designer.txt",
)
if source.rfind(name) > pin_at
]
assert not later, f"these requirements files are installed after the diffusers pin: {later}"
def test_install_sh_still_delegates_the_core_package_skip():
"""The handoff flag skips core packages while allowing other base entries through."""
assert 'SKIP_STUDIO_BASE="$_SKIP_BASE"' in INSTALL_SH.read_text(encoding = "utf-8")
assert "_SKIP_BASE=1" in INSTALL_SH.read_text(encoding = "utf-8")