Commit graph

5 commits

Author SHA1 Message Date
yuneng-jiang
a734afca32
feat(ci): gate patching of SDK internals in tests as TQ008 (#37787)
* feat(ci): gate patching of SDK internals in tests as TQ008

TQ002 catches the narrowest symptom of the suite's dominant mocking idiom,
patch X then assert only that X was called. The idiom itself is wider: tests
reach for litellm's own functions instead of faking the wire, so they pin how
the code is wired rather than what it does, and a test that patches internals
but makes weak real assertions trips nothing today.

TQ008 counts patch targets rooted at `litellm`, both the dotted string form and
the attribute chain handed to patch.object, and ratchets like every other rule.
Mocking anything outside the SDK is untouched: respx, httpx transports and
third-party clients do not trip it, which is the point, since those are the
patterns this is meant to move the suite toward.

Seeded at 9,643, in line with the ~9.4k patch sites an independent grep found
in the mirror. The burn-down horizon is long; the value here is stopping the
flow rather than clearing the stock.

Five existing rule tests patched `litellm.completion` incidentally and now
report TQ008 alongside what they were pinning. Their expected values are
updated to the accurate pair rather than loosened, so they keep failing on a
regression in either rule.

* test: add TQ008 to the shipped-budget rule canary

* fix(ci): resolve imported SDK names in TQ008

patch.object(handler.OpenAIChatCompletion, ...) after a from-import reaches the
same internal as the dotted string form, but the rule only saw the bare local
name and let it through. Import bindings are now resolved to the path they
stand for, so the aliased, renamed and from-imported forms all read alike and
the reported target is the real one.

That is 1,496 patches the ratchet could not see, so the TQ008 limit moves from
9,643 to 11,139. Third-party names and locals with no SDK import behind them
stay unflagged.
2026-08-22 22:54:30 -07:00
yuneng-jiang
6e23288b47
perf(ci): fan the budget checkers out across cores (#37784)
* perf(ci): fan the budget checkers out across cores

check_type_discipline.py and check_test_quality.py each walk a few thousand
files and parse every one, single-threaded. In the lint job those two steps
measure 2.3 and 1.6 minutes, second and third behind dependency install, and
lint is the slowest required check on 9 of the last 10 merged staging PRs.

check_file is already pure per-file work, so the walk fans out over a process
pool with no change to what either rule reports. Callers sort, which is what
keeps output order stable when results land out of order. Runs below
PARALLEL_MIN_PATHS stay serial rather than pay for process startup, and the
worker count is capped so a large runner does not oversubscribe.

Measured locally over the same trees, output byte-identical both times:
type-discipline 17.8s -> 3.0s over litellm/ (78,768 report lines), test-quality
14.4s -> 2.3s over tests/ (6,321 report lines), per-rule counts unchanged.

* test(ci): type the fan-out helpers and skip the comparison on one core
2026-08-22 22:44:58 -07:00
yuneng-jiang
4af66657f9
feat(ci): freeze the conftest save/restore inventory so it can only shrink (#37621)
* feat(ci): freeze the conftest save/restore inventory so it can only shrink

* fix(ci): resolve the named constant a conftest save loop iterates

* fix(ci): match the snapshot shape instead of a list of blessed dict names

* feat(ci): fail a branch that clears TQ violations without lowering the ceiling

A limit that only ever falls is not the same as one that falls when it can.
Clearing violations and leaving the ceiling above the new count let the same
violations return later under a limit nobody moved, so the gate now fails on
that and names `make lint-budget-update` as the fix. It needs both head below
base and head below limit, so headroom already in the base is never blamed on
the branch that happens to run next.

Drops the seeded-rule exemption from the ratchet along with it. Its stated
reason was that the base tree predates a rule introduced on this branch, but
base counts are measured with the current checker, so such a rule is counted at
the base too and its grandfathered total was never at risk of reading as fixed.
Removing the exemption is what lets a newly seeded rule ratchet like the six
that came before it.

The base scan is skipped when the branch touches neither the test tree nor the
checker, since neither count can have moved.
2026-08-20 21:39:59 +00:00
yuneng-jiang
569dcf435d
feat(ci): ratchet tests that skip themselves when a credential is absent (#37612)
* feat(ci): ratchet tests that skip themselves when a credential is absent

* docs(ci): name the new rule where the gate's rules are listed

* fix(ci): require the condition to test for absence before TQ006 fires
2026-08-20 10:59:35 -07:00
yuneng-jiang
ffab5a39d0
feat(ci): ratchet the test suite's zero-assert, mock-echo and global-state debt (#37588)
* feat(ci): ratchet the test suite's zero-assert, mock-echo and global-state debt

The suite's dominant failure mode is tests that cannot fail for the reason anyone
would want them to. The testing-strategy audit measured five shapes of it, and
nothing mechanical stops any of them from reproducing, so they keep reproducing.

`scripts/check_test_quality.py` is an AST checker for those five, emitting the
same `path:line: CODE message` contract as `scripts/check_type_discipline.py`:

  TQ001  a collectible test with no assertion of any kind
  TQ002  mock-echo, where every assertion only inspects the mock that was patched
  TQ003  sys.path.insert inside the test tree
  TQ004  raw `os.environ[...] =`, which leaks into whatever runs next
  TQ005  `litellm.<attr> =`, the process-wide leak the 491-line conftest undoes

`scripts/test_quality_gate.py` caps each rule against test-quality-budget.json,
seeded at exactly today's count, and fails only when a rule is both over its
limit and higher than the base being merged into, so a change is blamed for what
it adds and never for drift already in the base. `--update` lowers a limit by
what a branch cleared, so the ceilings only ever fall. It runs in the existing
required lint job, which means it enforces without a ruleset change.

TQ001 follows assertions into helpers defined in the same module, transitively.
Without that it flagged 111 tests in tests/e2e, the harness this program holds up
as the reference, because that suite factors its assertions into shared helpers
(`assert_auth_denied(result, ...)`). Following them leaves 25, all of which reach
their assertions across a module boundary; those are grandfathered and documented
rather than papered over.

The seeded counts land within about 10% of the audit's independent numbers for
every rule measured on the same subtree, which is the cross-check that the
definitions here match the ones the audit pinned.

* fix(ci): resolve test helpers per scope, not by bare name

The helper walk keyed every function in a module by its bare name, so two
same-named helpers in different classes collided and the last one parsed won.
A test calling `self._check()` could be cleared by a `_check` belonging to a
different class, or flagged because of one.

Resolution is now scoped: a bare name looks up the module-level functions, and
`self.<name>` looks up the enclosing class's own methods and no other class's.
Recursion is tracked by function identity rather than by name, so the cycle
guard cannot be confused by the same collision.

This surfaced one real zero-assert test that a same-named helper elsewhere had
been clearing, so TQ001 seeds at 750 rather than 749.

The test module has to register itself in sys.modules before exec_module:
`@dataclass(slots=True)` rebuilds its class through `sys.modules[__module__]`,
and Scope fails to construct without it. Recorded at the call site, since it
reads like avoidable global mutation otherwise.

* fix: register test-quality-budget.json with the ratchet alarm

The repo keeps one census over its budget files: every *-budget.json on disk
must appear in DEFAULT_BUDGETS, or its ceilings can be raised with no signal.
tests/test_litellm/test_budget_ratchet_check.py asserts that set equality and
caught the new budget on the way in.

Registering it also turns the alarm on for TQ001-TQ005, so a later PR cannot
quietly raise a test-quality ceiling. The file already uses the {limit: N}
schema the ratchet reads, so no other change was needed.
2026-08-20 10:08:49 -07:00