mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
ci(lint): gate every rule on its merge-base count and cap Anys at a fixed total (#40193)
* ci(lint): derive gate ceilings from merge-base counts and drop the budget files The four lint gates (ruff strict, type discipline, basedpyright, test quality) now fail a branch only when a rule's codebase count grows past its count at the merge-base with litellm_internal_staging plus a fixed per-rule headroom, which is zero everywhere except the LIT010/LIT011 and reportAny/reportExplicitAny seeds. Base counts come from a disk cache, then the CI artifact the renamed publish-lint-base-counts workflow uploads for every staging push (all four checkers, one artifact per checker and sha), then a scan of the base worktree. The four *-budget.json files, make lint-budget-update, budget_ratchet_check.py, and the unratcheted check are gone, so no PR carries a budget edit again. * test(lint-gates): assert checker identities by behavior and type the new gate tests * test(lint-gates): resolve base points over an injected git so unit tests stay in-process --------- Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
This commit is contained in:
parent
fbbb54c841
commit
d6e453f74b
29 changed files with 1319 additions and 2802 deletions
|
|
@ -1,65 +0,0 @@
|
|||
name: Publish basedpyright base counts
|
||||
|
||||
# Every commit on main can become a future merge-base.
|
||||
# Publishing its per-rule basedpyright counts as an artifact lets
|
||||
# scripts/type_check_gate.py download them in seconds instead of paying a
|
||||
# 60-110s second basedpyright pass on every fresh worktree or moved merge-base.
|
||||
# No concurrency group on purpose: runs must never cancel each other, because
|
||||
# every sha's artifact matters (any of them can become a merge-base).
|
||||
|
||||
on:
|
||||
push:
|
||||
branches:
|
||||
- main
|
||||
workflow_dispatch:
|
||||
inputs:
|
||||
ref:
|
||||
description: "Ref to compute and publish base counts for (defaults to the workflow run's commit)"
|
||||
required: false
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
publish:
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 20
|
||||
|
||||
steps:
|
||||
- uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
|
||||
with:
|
||||
ref: ${{ inputs.ref || github.sha }}
|
||||
clean: true
|
||||
persist-credentials: false
|
||||
|
||||
- name: Set up Python
|
||||
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
|
||||
with:
|
||||
python-version: "3.12"
|
||||
|
||||
- name: Set up uv
|
||||
uses: ./.github/actions/setup-uv-with-retries
|
||||
with:
|
||||
version: "0.10.9"
|
||||
|
||||
- name: Cache the Rust build
|
||||
uses: ./.github/actions/cache-cargo-build
|
||||
|
||||
- name: Cache Prisma binaries
|
||||
uses: ./.github/actions/cache-prisma-binaries
|
||||
|
||||
# The gate provisions its own measurement env (.venv-typecheck: a frozen
|
||||
# uv sync of its canonical dependency groups plus a generated Prisma
|
||||
# client), so no install step here can drift from what local runs measure.
|
||||
- name: Emit basedpyright counts for HEAD
|
||||
run: |
|
||||
python scripts/type_check_gate.py --emit-counts-dir "$RUNNER_TEMP/basedpyright-counts"
|
||||
counts_file=$(ls "$RUNNER_TEMP"/basedpyright-counts/basedpyright-counts-*.json)
|
||||
echo "COUNTS_ARTIFACT_NAME=$(basename "$counts_file" .json)" >> "$GITHUB_ENV"
|
||||
|
||||
- name: Upload counts artifact
|
||||
uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
|
||||
with:
|
||||
name: ${{ env.COUNTS_ARTIFACT_NAME }}
|
||||
path: ${{ runner.temp }}/basedpyright-counts/
|
||||
if-no-files-found: error
|
||||
96
.github/workflows/publish-lint-base-counts.yml
vendored
Normal file
96
.github/workflows/publish-lint-base-counts.yml
vendored
Normal file
|
|
@ -0,0 +1,96 @@
|
|||
name: Publish lint base counts
|
||||
|
||||
# Every commit on main can become a future merge-base.
|
||||
# Publishing its per-rule counts for each lint gate (strict ruff, type discipline,
|
||||
# test quality, basedpyright) as an artifact lets the gates download them through
|
||||
# scripts/lint_base_counts.py in seconds instead of scanning the merge-base tree in
|
||||
# a throwaway worktree on every fresh checkout or moved merge-base.
|
||||
# No concurrency group on purpose: runs must never cancel each other, because
|
||||
# every sha's artifact matters (any of them can become a merge-base).
|
||||
|
||||
on:
|
||||
push:
|
||||
branches:
|
||||
- main
|
||||
workflow_dispatch:
|
||||
inputs:
|
||||
ref:
|
||||
description: "Ref to compute and publish base counts for (defaults to the workflow run's commit)"
|
||||
required: false
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
publish:
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 20
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
include:
|
||||
- checker: ruff-strict
|
||||
script: scripts/ruff_strict_gate.py
|
||||
- checker: type-discipline
|
||||
script: scripts/type_discipline_gate.py
|
||||
- checker: test-quality
|
||||
script: scripts/test_quality_gate.py
|
||||
- checker: basedpyright
|
||||
script: scripts/type_check_gate.py
|
||||
|
||||
steps:
|
||||
- uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
|
||||
with:
|
||||
ref: ${{ inputs.ref || github.sha }}
|
||||
clean: true
|
||||
persist-credentials: false
|
||||
|
||||
- name: Set up Python
|
||||
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
|
||||
with:
|
||||
python-version: "3.12"
|
||||
|
||||
- name: Set up uv
|
||||
uses: ./.github/actions/setup-uv-with-retries
|
||||
with:
|
||||
version: "0.10.9"
|
||||
|
||||
- name: Cache the Rust build
|
||||
if: matrix.checker == 'basedpyright'
|
||||
uses: ./.github/actions/cache-cargo-build
|
||||
|
||||
- name: Cache Prisma binaries
|
||||
if: matrix.checker == 'basedpyright'
|
||||
uses: ./.github/actions/cache-prisma-binaries
|
||||
|
||||
# The three source scanners only need the pinned dev tools (ruff and the
|
||||
# stdlib checkers), the same versions test-linting.yml's lint job runs.
|
||||
- name: Install the dev tools
|
||||
if: matrix.checker != 'basedpyright'
|
||||
run: |
|
||||
uv sync --frozen --only-group dev --no-install-project
|
||||
|
||||
- name: Emit ${{ matrix.checker }} counts for HEAD
|
||||
if: matrix.checker != 'basedpyright'
|
||||
run: |
|
||||
uv run --no-sync python ${{ matrix.script }} --emit-counts-dir "$RUNNER_TEMP/lint-counts"
|
||||
|
||||
# The basedpyright gate provisions its own measurement env (.venv-typecheck:
|
||||
# a frozen uv sync of its canonical dependency groups plus a generated Prisma
|
||||
# client), so no install step here can drift from what local runs measure.
|
||||
- name: Emit basedpyright counts for HEAD
|
||||
if: matrix.checker == 'basedpyright'
|
||||
run: |
|
||||
python scripts/type_check_gate.py --emit-counts-dir "$RUNNER_TEMP/lint-counts"
|
||||
|
||||
- name: Name the counts artifact
|
||||
run: |
|
||||
counts_file=$(ls "$RUNNER_TEMP"/lint-counts/${{ matrix.checker }}-counts-*.json)
|
||||
echo "COUNTS_ARTIFACT_NAME=$(basename "$counts_file" .json)" >> "$GITHUB_ENV"
|
||||
|
||||
- name: Upload counts artifact
|
||||
uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
|
||||
with:
|
||||
name: ${{ env.COUNTS_ARTIFACT_NAME }}
|
||||
path: ${{ runner.temp }}/lint-counts/
|
||||
if-no-files-found: error
|
||||
54
.github/workflows/test-linting.yml
vendored
54
.github/workflows/test-linting.yml
vendored
|
|
@ -17,9 +17,9 @@ jobs:
|
|||
lint:
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 15
|
||||
# actions: read lets scripts/type_check_gate.py download the base-counts
|
||||
# artifact published by publish-basedpyright-base-counts.yml instead of
|
||||
# re-running basedpyright over the merge-base tree.
|
||||
# actions: read lets the four lint gates download the base-counts artifacts
|
||||
# published by publish-lint-base-counts.yml instead of re-scanning the
|
||||
# merge-base tree in a throwaway worktree.
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: read
|
||||
|
|
@ -144,18 +144,24 @@ jobs:
|
|||
run: |
|
||||
uv run --no-sync ruff check --config ruff-tests.toml tests
|
||||
|
||||
- name: Check strict-rule budget (delta vs base)
|
||||
- name: Check strict ruff rules (delta vs merge-base counts)
|
||||
if: steps.changes.outputs.decision != 'skip'
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
run: |
|
||||
uv run --no-sync python scripts/ruff_strict_gate.py --base "$GATE_BASE_SHA"
|
||||
|
||||
- name: Check type-discipline budget (mutable collections / casts / type guards / kwargs / unexplained suppressions, delta vs base)
|
||||
- name: Check type discipline (mutable collections / casts / type guards / kwargs / unexplained suppressions, delta vs merge-base counts)
|
||||
if: steps.changes.outputs.decision != 'skip'
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
run: |
|
||||
uv run --no-sync python scripts/type_discipline_gate.py --base "$GATE_BASE_SHA"
|
||||
|
||||
- name: Check test-quality budget (zero-assert / mock-echo tests, sys.path.insert, raw env writes, litellm global mutation, credential-gated skips, conftest snapshot inventory, delta vs base)
|
||||
- name: Check test quality (zero-assert / mock-echo tests, sys.path.insert, raw env writes, litellm global mutation, credential-gated skips, conftest snapshot inventory, delta vs merge-base counts)
|
||||
if: steps.changes.outputs.decision != 'skip'
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
run: |
|
||||
uv run --no-sync python scripts/test_quality_gate.py --base "$GATE_BASE_SHA"
|
||||
|
||||
|
|
@ -164,7 +170,7 @@ jobs:
|
|||
run: |
|
||||
uv run --no-sync python -c "import openai; print(f'OpenAI version: {openai.__version__}')"
|
||||
|
||||
- name: Check basedpyright budget (delta vs base)
|
||||
- name: Check basedpyright (delta vs merge-base counts)
|
||||
if: steps.changes.outputs.decision != 'skip'
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
|
|
@ -206,40 +212,6 @@ jobs:
|
|||
run: |
|
||||
uv run --no-sync python -c "from litellm import *" || (echo '🚨 import failed, this means you introduced unprotected imports! 🚨'; exit 1)
|
||||
|
||||
# Intentionally NON-GATING. This job turns red when a *-budget.json ceiling is
|
||||
# raised (or a rule/budget is dropped) so a loosening is obvious in review, but it
|
||||
# must be kept OUT of the branch-protection required-checks list so a justified
|
||||
# bump can still be merged by a human who has seen and accepted the red.
|
||||
budget-ratchet:
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
steps:
|
||||
- uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
|
||||
with:
|
||||
fetch-depth: 1
|
||||
persist-credentials: false
|
||||
|
||||
- name: Fetch ratchet base
|
||||
env:
|
||||
BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
run: |
|
||||
retry() { "$@" || { sleep 15; "$@"; } || { sleep 30; "$@"; }; }
|
||||
retry git fetch --no-tags --depth=1 origin "$BASE_SHA"
|
||||
|
||||
- name: Set up Python
|
||||
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
|
||||
with:
|
||||
python-version: "3.12"
|
||||
|
||||
- name: Ratchet check (budgets may only decrease; non-gating)
|
||||
env:
|
||||
BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
run: |
|
||||
python scripts/budget_ratchet_check.py --base "$BASE_SHA"
|
||||
|
||||
secret-scan:
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 5
|
||||
|
|
|
|||
|
|
@ -54,13 +54,13 @@ Don't hesitate to use values in .env to get needed API keys and other secrets, a
|
|||
|
||||
Python max line length is 120, not 88
|
||||
|
||||
Never edit or commit `ruff-strict-budget.json`, `type-discipline-budget.json`, `basedpyright-code-budget.json`, or `test-quality-budget.json` on a PR branch, and don't run `make lint-budget-update` there. A scheduled Devin automation lowers the limits on the default branch in its own PR by exactly what landed since the last ratchet, so concurrent PRs don't fight over the same `"limit"` lines. Keep the hosted automation's target in sync when the repository default changes. If your branch already carries a budget edit, drop it before opening the PR
|
||||
The four lint gates (`scripts/ruff_strict_gate.py`, `scripts/type_discipline_gate.py`, `scripts/test_quality_gate.py`, `scripts/type_check_gate.py`) compare each rule's codebase count on your branch against the count at its merge-base with the default branch, and a rule may not grow. There are no budget files to edit or ratchet: when a gate fails, fix the violations the branch introduced or remove at least as many of that rule elsewhere in the tree. The one exception is `reportAny` / `reportExplicitAny`, which share a fixed codebase-wide cap in `ANY_CAPS` in `scripts/type_check_gate.py` because Any spreads past the lines you touch. A branch may add Anys while the total stays under that cap. Only lower `ANY_CAPS`, and only in its own PR to the default branch, never raise it on a feature branch
|
||||
|
||||
`make check` (f.k.a. `make pre-commit`, which still works identically as an alias) saves its complete output to a log file in .git (overwriting previous logs) and prints that path as its first and last output lines. To inspect a run, read or grep that log instead of re-running the multi-minute checks just to see a different slice
|
||||
|
||||
`make check`, `make lint`, `scripts/pre_commit_lint.sh`, and the standalone budget gates (`scripts/ruff_strict_gate.py`, `scripts/type_discipline_gate.py`, `scripts/type_check_gate.py`) each hold one of 2 machine-wide slots, so when other sessions or worktrees on the same box are already running heavy work, yours prints "all N machine-wide slots are busy; queueing" and then stays quiet until a slot frees. Give the command a long timeout and let it wait rather than killing it, retrying it, or assuming it hung. Don't change the # of machine-wide slots or make it unlimited by setting `LITELLM_GATE_SLOTS=0`
|
||||
`make check`, `make lint`, `scripts/pre_commit_lint.sh`, and the standalone lint gates (`scripts/ruff_strict_gate.py`, `scripts/type_discipline_gate.py`, `scripts/type_check_gate.py`) each hold one of 2 machine-wide slots, so when other sessions or worktrees on the same box are already running heavy work, yours prints "all N machine-wide slots are busy; queueing" and then stays quiet until a slot frees. Give the command a long timeout and let it wait rather than killing it, retrying it, or assuming it hung. Don't change the # of machine-wide slots or make it unlimited by setting `LITELLM_GATE_SLOTS=0`
|
||||
|
||||
If you're trying to create a new function that relies on untyped stuff, instead of adding more Any's and pushing `reportAny` / `reportExplicitAny` closer to their basedpyright ceilings, just validate it in the caller with Pydantic (a model or `TypeAdapter` that returns the typed thing or raises will do) and then pass the now typed variable in
|
||||
If you're trying to create a new function that relies on untyped stuff, instead of adding more Any's and pushing `reportAny` / `reportExplicitAny` closer to their cap in `ANY_CAPS`, just validate it in the caller with Pydantic (a model or `TypeAdapter` that returns the typed thing or raises will do) and then pass the now typed variable in
|
||||
|
||||
If you get an LIT001 fail, refactor the code to follow functional programming best practices rather than introducing mutable data structures. For example, build values in one shot with comprehensions or generators wrapped in `tuple()` / `MappingProxyType()` / `frozenset()` instead of seeding an empty `list`/`dict`/`set` and mutating it over time. Ideally, `# mutable-ok` is never used; reach for it only as a genuine last resort when an immutable rewrite is truly impossible, and always pair it with a real reason
|
||||
|
||||
|
|
@ -130,4 +130,4 @@ Before implementing:
|
|||
|
||||
Ask yourself: "Would a senior engineer say this is overcomplicated?" If yes, simplify
|
||||
|
||||
Before requesting maintainer review, verify the current PR tip passes required CI and code coverage, meets Greptile confidence of at least 4/5, and has acceptable Veria and Bugbot reviews. Inspect warnings and findings, fix actionable issues, and rerun the affected checks and reviewers after changes. Record evidence for any false positive or unavailable review; never treat a pending or missing bot result as a pass. Do not lower coverage thresholds or lint budgets to satisfy a check
|
||||
Before requesting maintainer review, verify the current PR tip passes required CI and code coverage, meets Greptile confidence of at least 4/5, and has acceptable Veria and Bugbot reviews. Inspect warnings and findings, fix actionable issues, and rerun the affected checks and reviewers after changes. Record evidence for any false positive or unavailable review; never treat a pending or missing bot result as a pass. Do not lower coverage thresholds or raise `ANY_CAPS` to satisfy a check
|
||||
|
|
|
|||
39
Makefile
39
Makefile
|
|
@ -6,9 +6,8 @@
|
|||
test-proxy-unit-a test-proxy-unit-b test-integration test-unit-helm \
|
||||
test-rust-extension rust-sqlx-prepare lens-dev \
|
||||
info lint lint-inner lint-dev lint-checks format \
|
||||
lint-basedpyright lint-e2e-basedpyright lint-basedpyright-budget-update lint-type-discipline lint-type-discipline-budget-update \
|
||||
lint-ruff-budget lint-ruff-budget-update lint-budget-update lint-gate \
|
||||
lint-test-quality lint-test-quality-budget-update \
|
||||
lint-basedpyright lint-e2e-basedpyright lint-type-discipline \
|
||||
lint-ruff-strict lint-gate lint-test-quality \
|
||||
install-dev install-proxy-dev install-test-deps install-hooks \
|
||||
install-helm-unittest check-circular-imports check-import-safety check check-inner pre-commit \
|
||||
lint-install lint-fetch-base bootstrap
|
||||
|
|
@ -32,13 +31,10 @@ help:
|
|||
@echo " make lint-ruff - Run Ruff linting only"
|
||||
@echo " make lint-basedpyright - Run basedpyright strict, gated by per-rule error counts"
|
||||
@echo " make lint-e2e-basedpyright - Run basedpyright over tests/e2e and tests/e2e_harness (zero errors allowed)"
|
||||
@echo " make lint-basedpyright-budget-update - Ratchet basedpyright limits down by what this branch fixed"
|
||||
@echo " make lint-format - Check ruff format formatting (matches CI)"
|
||||
@echo " make lint-ruff-budget - Gate the codebase total of each strict ruff rule against its limit"
|
||||
@echo " make lint-ruff-strict - Gate each strict ruff rule's codebase total against its merge-base count"
|
||||
@echo " make lint-gate - Strict ruff gate in CI-parity mode (fetches the default branch, simulates the merge)"
|
||||
@echo " make lint-ruff-budget-update - Ratchet ruff-strict-budget.json limits down by what this branch fixed"
|
||||
@echo " make lint-test-quality - Gate the test suite against test-quality-budget.json"
|
||||
@echo " make lint-budget-update - Ratchet all budgets down (ruff + type-discipline + test quality + basedpyright)"
|
||||
@echo " make lint-test-quality - Gate the test suite's TQ counts against their merge-base counts"
|
||||
@echo " make check-circular-imports - Check for circular imports"
|
||||
@echo " make check-import-safety - Check import safety"
|
||||
@echo " make test - Run all tests"
|
||||
|
|
@ -143,7 +139,7 @@ lint-fetch-base:
|
|||
|
||||
# Mirror test-linting.yml's lint job environment: the proxy-dev group plus a generated
|
||||
# Prisma client, so `basedpyright tests/e2e` resolves the same modules CI does. The
|
||||
# budget gate itself no longer measures here (scripts/type_check_gate.py provisions its
|
||||
# basedpyright gate itself no longer measures here (scripts/type_check_gate.py provisions its
|
||||
# own .venv-typecheck). --inexact tops up the venv instead of pruning the proxy extras
|
||||
# gen:api and the running proxy need.
|
||||
lint-install:
|
||||
|
|
@ -213,25 +209,20 @@ lint-basedpyright: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE)
|
|||
lint-e2e-basedpyright: $(LINT_E2E_DEP_INSTALL)
|
||||
$(UV_RUN) basedpyright tests/e2e tests/e2e_harness
|
||||
|
||||
# Type-discipline budget (mutable collections / casts / type guards / kwargs /
|
||||
# Type-discipline gate (mutable collections / casts / type guards / kwargs /
|
||||
# unexplained suppressions), the test-linting.yml step `make lint` used to omit.
|
||||
lint-type-discipline: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE)
|
||||
$(UV_RUN) python scripts/type_discipline_gate.py --base "$(BASE_REF)"
|
||||
|
||||
# Test-quality budget (zero-assert / mock-echo tests, sys.path.insert, raw env writes,
|
||||
# Test-quality gate (zero-assert / mock-echo tests, sys.path.insert, raw env writes,
|
||||
# litellm module-global mutation, credential-gated skips, conftest snapshot
|
||||
# inventory), counted across tests/ the same delta-vs-base way.
|
||||
lint-test-quality: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE)
|
||||
$(UV_RUN) python scripts/test_quality_gate.py --base "$(BASE_REF)"
|
||||
|
||||
# --update lowers each limit by what this branch fixed since its branch point, so
|
||||
# it needs the base ref fetched to resolve the merge-base.
|
||||
lint-basedpyright-budget-update: install-dev
|
||||
$(UV_RUN) python scripts/type_check_gate.py --update --base "$(BASE_REF)"
|
||||
|
||||
lint-format: format-check
|
||||
|
||||
lint-ruff-budget: install-dev
|
||||
lint-ruff-strict: install-dev
|
||||
$(UV_RUN) python scripts/ruff_strict_gate.py --base "$(BASE_REF)"
|
||||
|
||||
# Strict gate, invoked the same way CI does in test-linting.yml so a local pass
|
||||
|
|
@ -239,18 +230,6 @@ lint-ruff-budget: install-dev
|
|||
lint-gate: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE)
|
||||
$(UV_RUN) python scripts/ruff_strict_gate.py --base "$(BASE_REF)"
|
||||
|
||||
lint-ruff-budget-update: install-dev
|
||||
$(UV_RUN) python scripts/ruff_strict_gate.py --update --base "$(BASE_REF)"
|
||||
|
||||
lint-type-discipline-budget-update: install-dev
|
||||
$(UV_RUN) python scripts/type_discipline_gate.py --update --base "$(BASE_REF)"
|
||||
|
||||
lint-test-quality-budget-update: install-dev
|
||||
$(UV_RUN) python scripts/test_quality_gate.py --update --base "$(BASE_REF)"
|
||||
|
||||
# Ratchet all budgets in one shot (ruff strict + type-discipline + test quality + basedpyright)
|
||||
lint-budget-update: lint-ruff-budget-update lint-type-discipline-budget-update lint-test-quality-budget-update lint-basedpyright-budget-update
|
||||
|
||||
check-circular-imports: $(LINT_DEP_INSTALL)
|
||||
cd litellm && $(UV_RUN) python ../tests/documentation_tests/test_circular_imports.py && cd ..
|
||||
|
||||
|
|
@ -260,7 +239,7 @@ check-import-safety: $(LINT_DEP_INSTALL)
|
|||
# Combined linting, isomorphic to test-linting.yml's lint job so a local pass means a
|
||||
# green CI lint: it installs the same env (proxy-dev + generated Prisma client) and then
|
||||
# runs the diff-scoped ruff format check, whole-tree ruff check, the strict-rule /
|
||||
# type-discipline / basedpyright budgets as a delta vs the base, then the circular-import
|
||||
# type-discipline / basedpyright gates as a delta vs the base, then the circular-import
|
||||
# and import-safety checks. Steps that compare against the base resolve it the same way CI
|
||||
# does (merge-base with origin's current default branch). Setup (env sync, Prisma client,
|
||||
# base fetch) runs once up front; the checks themselves are independent, so a sub-make
|
||||
|
|
|
|||
|
|
@ -1,146 +0,0 @@
|
|||
{
|
||||
"reportAny": {
|
||||
"limit": 13429
|
||||
},
|
||||
"reportArgumentType": {
|
||||
"limit": 2198
|
||||
},
|
||||
"reportAssignmentType": {
|
||||
"limit": 319
|
||||
},
|
||||
"reportAttributeAccessIssue": {
|
||||
"limit": 480
|
||||
},
|
||||
"reportCallIssue": {
|
||||
"limit": 112
|
||||
},
|
||||
"reportConstantRedefinition": {
|
||||
"limit": 40
|
||||
},
|
||||
"reportDeprecated": {
|
||||
"limit": 209
|
||||
},
|
||||
"reportDuplicateImport": {
|
||||
"limit": 19
|
||||
},
|
||||
"reportExplicitAny": {
|
||||
"limit": 3369
|
||||
},
|
||||
"reportFunctionMemberAccess": {
|
||||
"limit": 7
|
||||
},
|
||||
"reportGeneralTypeIssues": {
|
||||
"limit": 101
|
||||
},
|
||||
"reportIncompatibleMethodOverride": {
|
||||
"limit": 56
|
||||
},
|
||||
"reportIncompatibleVariableOverride": {
|
||||
"limit": 8
|
||||
},
|
||||
"reportInconsistentOverload": {
|
||||
"limit": 12
|
||||
},
|
||||
"reportIndexIssue": {
|
||||
"limit": 24
|
||||
},
|
||||
"reportInvalidTypeForm": {
|
||||
"limit": 30
|
||||
},
|
||||
"reportInvalidTypeVarUse": {
|
||||
"limit": 1
|
||||
},
|
||||
"reportMatchNotExhaustive": {
|
||||
"limit": 0
|
||||
},
|
||||
"reportMissingParameterType": {
|
||||
"limit": 5570
|
||||
},
|
||||
"reportMissingTypeArgument": {
|
||||
"limit": 15281
|
||||
},
|
||||
"reportMissingTypeStubs": {
|
||||
"limit": 40
|
||||
},
|
||||
"reportOperatorIssue": {
|
||||
"limit": 0
|
||||
},
|
||||
"reportOptionalCall": {
|
||||
"limit": 0
|
||||
},
|
||||
"reportOptionalIterable": {
|
||||
"limit": 0
|
||||
},
|
||||
"reportOptionalMemberAccess": {
|
||||
"limit": 0
|
||||
},
|
||||
"reportOptionalOperand": {
|
||||
"limit": 0
|
||||
},
|
||||
"reportOptionalSubscript": {
|
||||
"limit": 0
|
||||
},
|
||||
"reportPossiblyUnboundVariable": {
|
||||
"limit": 56
|
||||
},
|
||||
"reportPrivateUsage": {
|
||||
"limit": 1804
|
||||
},
|
||||
"reportRedeclaration": {
|
||||
"limit": 8
|
||||
},
|
||||
"reportReturnType": {
|
||||
"limit": 180
|
||||
},
|
||||
"reportTypedDictNotRequiredAccess": {
|
||||
"limit": 22
|
||||
},
|
||||
"reportUndefinedVariable": {
|
||||
"limit": 0
|
||||
},
|
||||
"reportUnknownArgumentType": {
|
||||
"limit": 44802
|
||||
},
|
||||
"reportUnknownLambdaType": {
|
||||
"limit": 109
|
||||
},
|
||||
"reportUnknownMemberType": {
|
||||
"limit": 38269
|
||||
},
|
||||
"reportUnknownParameterType": {
|
||||
"limit": 19584
|
||||
},
|
||||
"reportUnknownVariableType": {
|
||||
"limit": 29814
|
||||
},
|
||||
"reportUnnecessaryCast": {
|
||||
"limit": 110
|
||||
},
|
||||
"reportUnnecessaryComparison": {
|
||||
"limit": 687
|
||||
},
|
||||
"reportUnnecessaryContains": {
|
||||
"limit": 4
|
||||
},
|
||||
"reportUnnecessaryIsInstance": {
|
||||
"limit": 816
|
||||
},
|
||||
"reportUntypedBaseClass": {
|
||||
"limit": 0
|
||||
},
|
||||
"reportUntypedFunctionDecorator": {
|
||||
"limit": 27
|
||||
},
|
||||
"reportUnusedClass": {
|
||||
"limit": 21
|
||||
},
|
||||
"reportUnusedFunction": {
|
||||
"limit": 136
|
||||
},
|
||||
"reportUnusedImport": {
|
||||
"limit": 542
|
||||
},
|
||||
"reportUnusedVariable": {
|
||||
"limit": 137
|
||||
}
|
||||
}
|
||||
|
|
@ -378,6 +378,7 @@ version_files = [
|
|||
]
|
||||
|
||||
[tool.pytest.ini_options]
|
||||
pythonpath = ["scripts"]
|
||||
asyncio_mode = "auto"
|
||||
asyncio_default_fixture_loop_scope = "session"
|
||||
markers = [
|
||||
|
|
|
|||
|
|
@ -1,260 +0,0 @@
|
|||
{
|
||||
"ANN001": {
|
||||
"limit": 2918
|
||||
},
|
||||
"ANN002": {
|
||||
"limit": 71
|
||||
},
|
||||
"ANN003": {
|
||||
"limit": 806
|
||||
},
|
||||
"ANN201": {
|
||||
"limit": 1965
|
||||
},
|
||||
"ANN202": {
|
||||
"limit": 829
|
||||
},
|
||||
"ANN204": {
|
||||
"limit": 683
|
||||
},
|
||||
"ANN205": {
|
||||
"limit": 112
|
||||
},
|
||||
"ANN206": {
|
||||
"limit": 133
|
||||
},
|
||||
"ANN401": {
|
||||
"limit": 119
|
||||
},
|
||||
"ASYNC230": {
|
||||
"limit": 11
|
||||
},
|
||||
"B004": {
|
||||
"limit": 2
|
||||
},
|
||||
"B006": {
|
||||
"limit": 176
|
||||
},
|
||||
"B008": {
|
||||
"limit": 503
|
||||
},
|
||||
"B009": {
|
||||
"limit": 52
|
||||
},
|
||||
"B010": {
|
||||
"limit": 187
|
||||
},
|
||||
"B018": {
|
||||
"limit": 2
|
||||
},
|
||||
"B019": {
|
||||
"limit": 1
|
||||
},
|
||||
"B021": {
|
||||
"limit": 1
|
||||
},
|
||||
"B026": {
|
||||
"limit": 3
|
||||
},
|
||||
"BLE001": {
|
||||
"limit": 2914
|
||||
},
|
||||
"C401": {
|
||||
"limit": 8
|
||||
},
|
||||
"C404": {
|
||||
"limit": 1
|
||||
},
|
||||
"C405": {
|
||||
"limit": 19
|
||||
},
|
||||
"C408": {
|
||||
"limit": 11
|
||||
},
|
||||
"C414": {
|
||||
"limit": 4
|
||||
},
|
||||
"C419": {
|
||||
"limit": 1
|
||||
},
|
||||
"C901": {
|
||||
"limit": 306
|
||||
},
|
||||
"D419": {
|
||||
"limit": 6
|
||||
},
|
||||
"DTZ001": {
|
||||
"limit": 2
|
||||
},
|
||||
"DTZ003": {
|
||||
"limit": 24
|
||||
},
|
||||
"DTZ005": {
|
||||
"limit": 233
|
||||
},
|
||||
"DTZ006": {
|
||||
"limit": 10
|
||||
},
|
||||
"DTZ007": {
|
||||
"limit": 6
|
||||
},
|
||||
"DTZ011": {
|
||||
"limit": 3
|
||||
},
|
||||
"EXE001": {
|
||||
"limit": 4
|
||||
},
|
||||
"EXE002": {
|
||||
"limit": 3
|
||||
},
|
||||
"F401": {
|
||||
"limit": 12
|
||||
},
|
||||
"LOG015": {
|
||||
"limit": 5
|
||||
},
|
||||
"N999": {
|
||||
"limit": 1
|
||||
},
|
||||
"PERF102": {
|
||||
"limit": 21
|
||||
},
|
||||
"PERF401": {
|
||||
"limit": 12
|
||||
},
|
||||
"PERF403": {
|
||||
"limit": 33
|
||||
},
|
||||
"PIE804": {
|
||||
"limit": 18
|
||||
},
|
||||
"PIE810": {
|
||||
"limit": 43
|
||||
},
|
||||
"PLC0206": {
|
||||
"limit": 26
|
||||
},
|
||||
"PLC0414": {
|
||||
"limit": 46
|
||||
},
|
||||
"PLR0124": {
|
||||
"limit": 1
|
||||
},
|
||||
"PLR0206": {
|
||||
"limit": 1
|
||||
},
|
||||
"PLR1704": {
|
||||
"limit": 1
|
||||
},
|
||||
"PLR1714": {
|
||||
"limit": 253
|
||||
},
|
||||
"PLW0127": {
|
||||
"limit": 57
|
||||
},
|
||||
"PLW0602": {
|
||||
"limit": 215
|
||||
},
|
||||
"PLW0603": {
|
||||
"limit": 190
|
||||
},
|
||||
"PLW1508": {
|
||||
"limit": 190
|
||||
},
|
||||
"PLW1510": {
|
||||
"limit": 2
|
||||
},
|
||||
"PYI036": {
|
||||
"limit": 3
|
||||
},
|
||||
"RET504": {
|
||||
"limit": 173
|
||||
},
|
||||
"RUF012": {
|
||||
"limit": 239
|
||||
},
|
||||
"RUF015": {
|
||||
"limit": 8
|
||||
},
|
||||
"RUF019": {
|
||||
"limit": 27
|
||||
},
|
||||
"RUF046": {
|
||||
"limit": 4
|
||||
},
|
||||
"RUF059": {
|
||||
"limit": 66
|
||||
},
|
||||
"RUF100": {
|
||||
"limit": 0
|
||||
},
|
||||
"S110": {
|
||||
"limit": 207
|
||||
},
|
||||
"S112": {
|
||||
"limit": 22
|
||||
},
|
||||
"SIM101": {
|
||||
"limit": 56
|
||||
},
|
||||
"SIM102": {
|
||||
"limit": 310
|
||||
},
|
||||
"SIM103": {
|
||||
"limit": 119
|
||||
},
|
||||
"SIM113": {
|
||||
"limit": 3
|
||||
},
|
||||
"SIM115": {
|
||||
"limit": 2
|
||||
},
|
||||
"SIM117": {
|
||||
"limit": 6
|
||||
},
|
||||
"SIM201": {
|
||||
"limit": 1
|
||||
},
|
||||
"SIM210": {
|
||||
"limit": 8
|
||||
},
|
||||
"SIM211": {
|
||||
"limit": 1
|
||||
},
|
||||
"SIM222": {
|
||||
"limit": 1
|
||||
},
|
||||
"SIM401": {
|
||||
"limit": 11
|
||||
},
|
||||
"TC004": {
|
||||
"limit": 5
|
||||
},
|
||||
"TID251": {
|
||||
"limit": 1035
|
||||
},
|
||||
"TRY002": {
|
||||
"limit": 524
|
||||
},
|
||||
"TRY004": {
|
||||
"limit": 96
|
||||
},
|
||||
"TRY201": {
|
||||
"limit": 401
|
||||
},
|
||||
"TRY203": {
|
||||
"limit": 109
|
||||
},
|
||||
"TRY300": {
|
||||
"limit": 852
|
||||
},
|
||||
"UP028": {
|
||||
"limit": 2
|
||||
},
|
||||
"UP031": {
|
||||
"limit": 2
|
||||
},
|
||||
"UP036": {
|
||||
"limit": 1
|
||||
}
|
||||
}
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
lint.ignore = ["F405", "E402", "F403"]
|
||||
# The second group is the strict gate's graduates: rules the codebase already has zero
|
||||
# violations of, so they hard-fail here instead of being ratcheted in ruff-strict-budget.json.
|
||||
# violations of, so they hard-fail here instead of being counted by the strict gate.
|
||||
# That gives editors and `ruff check --fix` the diagnostic, which the gate script cannot.
|
||||
lint.extend-select = [
|
||||
"T20", "PGH004", "RUF008", "RUF009", "RUF100",
|
||||
|
|
|
|||
|
|
@ -1,260 +0,0 @@
|
|||
#!/usr/bin/env python3
|
||||
"""Non-gating ratchet guard: budget limits may only fall, never rise.
|
||||
|
||||
Every `*-budget.json` file (ruff-strict, type-discipline, basedpyright-code) is a
|
||||
one-way ratchet: each rule's ceiling is its `limit`, and that limit is meant to be
|
||||
driven DOWN over time. This check compares every budget file against its own
|
||||
content at the merge-base with the target branch and fails (exits 1, red) if:
|
||||
|
||||
* a rule's `limit` went up,
|
||||
* a rule was dropped from a budget (its ceiling effectively became infinite) while
|
||||
its checker still emits it, or
|
||||
* an entire budget file was deleted.
|
||||
|
||||
New rules and lowered/equal limits are fine. So is a rule that graduated: once a
|
||||
paired config (ruff.toml for the ruff-strict budget) selects the rule outright it
|
||||
hard-fails at the first violation, which is stricter than any ceiling the budget
|
||||
could hold, so dropping its entry tightens the guard rather than removing it.
|
||||
Likewise a retired rule: once the paired checker (check_test_quality.py for the
|
||||
test-quality budget) no longer emits a code, its entry has no ceiling left to
|
||||
loosen.
|
||||
|
||||
This is deliberately NOT a gating check. It should turn the run red so that a
|
||||
loosening is impossible to miss in review, but it must stay OUT of the
|
||||
branch-protection required-checks list: a justified bump (e.g. banning a new API,
|
||||
which mechanically raises a baseline) can then still be merged by a human who has
|
||||
seen the red and accepted it.
|
||||
|
||||
Usage:
|
||||
python scripts/budget_ratchet_check.py [--base REF] [budget.json ...]
|
||||
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import importlib.util
|
||||
import json
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from types import MappingProxyType, ModuleType
|
||||
from typing import Final, NamedTuple
|
||||
|
||||
if sys.version_info >= (3, 11):
|
||||
import tomllib
|
||||
else:
|
||||
import tomli as tomllib
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
DEFAULT_BUDGETS: tuple[str, ...] = (
|
||||
"ruff-strict-budget.json",
|
||||
"type-discipline-budget.json",
|
||||
"basedpyright-code-budget.json",
|
||||
"test-quality-budget.json",
|
||||
)
|
||||
GRADUATION_CONFIGS = MappingProxyType({"ruff-strict-budget.json": "ruff.toml"})
|
||||
RETIREMENT_SOURCES = MappingProxyType({"test-quality-budget.json": "check_test_quality"})
|
||||
|
||||
|
||||
class Regression(NamedTuple):
|
||||
budget: str
|
||||
rule: str
|
||||
detail: str
|
||||
|
||||
|
||||
def _run(cmd: list[str]) -> subprocess.CompletedProcess[str]:
|
||||
return subprocess.run(cmd, cwd=REPO_ROOT, capture_output=True, text=True)
|
||||
|
||||
|
||||
def _merge_base(base: str) -> str:
|
||||
"""The common ancestor of `base` and HEAD, so unrelated base drift is ignored."""
|
||||
proc = _run(["git", "merge-base", base, "HEAD"])
|
||||
return proc.stdout.strip() or base
|
||||
|
||||
|
||||
def _load_head(rel: str) -> dict | None:
|
||||
path = REPO_ROOT / rel
|
||||
if not path.exists():
|
||||
return None
|
||||
return json.loads(path.read_text())
|
||||
|
||||
|
||||
def _ref_is_commit(ref: str) -> bool:
|
||||
return (
|
||||
_run(
|
||||
["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"]
|
||||
).returncode
|
||||
== 0
|
||||
)
|
||||
|
||||
|
||||
def _load_base(rel: str, ref: str) -> dict | None:
|
||||
"""Budget content at `ref`, or None when the file did not exist there.
|
||||
|
||||
`ref` is verified as a real commit by the caller, so a non-zero `git show` here means
|
||||
the path was absent at that commit, not that the ref itself is unresolvable.
|
||||
"""
|
||||
proc = _run(["git", "show", f"{ref}:{rel}"])
|
||||
if proc.returncode != 0:
|
||||
return None
|
||||
return json.loads(proc.stdout)
|
||||
|
||||
|
||||
def _ceiling(spec: dict) -> int:
|
||||
"""A rule's ceiling: its `limit`, or legacy `baseline + slack`.
|
||||
|
||||
The base side of the diff can predate the `limit` migration, so a spec is read
|
||||
under either schema and the two are compared on the same footing.
|
||||
"""
|
||||
if "limit" in spec:
|
||||
return int(spec["limit"])
|
||||
return int(spec.get("baseline", 0)) + int(spec.get("slack", 0))
|
||||
|
||||
|
||||
def _limits(budget: dict) -> dict[str, int]:
|
||||
"""Map each rule to its ceiling; skip malformed specs."""
|
||||
return {
|
||||
rule: _ceiling(spec)
|
||||
for rule, spec in budget.items()
|
||||
if isinstance(spec, dict)
|
||||
}
|
||||
|
||||
|
||||
def selectors_hard_failed_by(lint: dict) -> tuple[str, ...]:
|
||||
"""A ruff `[lint]` table's selected codes, minus anything `ignore` turns back off.
|
||||
|
||||
`lint.ignore` wins over `lint.extend-select` in ruff, so an ignored code is not
|
||||
actually enforced and must not count as a graduation.
|
||||
"""
|
||||
ignored = tuple(lint.get("ignore", ()))
|
||||
return tuple(
|
||||
selector
|
||||
for selector in lint.get("extend-select", ())
|
||||
if not (ignored and selector.startswith(ignored))
|
||||
)
|
||||
|
||||
|
||||
def graduated_selectors(rel: str) -> tuple[str, ...]:
|
||||
"""Selectors the budget's paired ruff config hard-fails, so its ceiling is moot."""
|
||||
config = GRADUATION_CONFIGS.get(rel)
|
||||
if config is None or not (REPO_ROOT / config).exists():
|
||||
return ()
|
||||
return selectors_hard_failed_by(
|
||||
tomllib.loads((REPO_ROOT / config).read_text()).get("lint", {})
|
||||
)
|
||||
|
||||
|
||||
def _load_script(name: str) -> ModuleType:
|
||||
if name in sys.modules:
|
||||
return sys.modules[name]
|
||||
spec: Final = importlib.util.spec_from_file_location(name, REPO_ROOT / "scripts" / f"{name}.py")
|
||||
assert spec is not None and spec.loader is not None
|
||||
module: Final = importlib.util.module_from_spec(spec)
|
||||
sys.modules[name] = module
|
||||
spec.loader.exec_module(module)
|
||||
return module
|
||||
|
||||
|
||||
def retired_rules(rel: str, base: dict[str, object]) -> frozenset[str]:
|
||||
"""Rules in the base budget that the paired checker can no longer emit, so there is no ceiling to loosen."""
|
||||
source: Final = RETIREMENT_SOURCES.get(rel)
|
||||
if source is None:
|
||||
return frozenset()
|
||||
return frozenset(_limits(base)) - _load_script(source).RULE_CODES
|
||||
|
||||
|
||||
def _regression_detail(
|
||||
rule: str,
|
||||
base_limits: dict[str, int],
|
||||
head_limits: dict[str, int],
|
||||
graduated: tuple[str, ...],
|
||||
retired: frozenset[str] = frozenset(),
|
||||
) -> str | None:
|
||||
"""Why `rule` regressed vs base, or None when it held flat, fell, or left the budget legitimately.
|
||||
|
||||
A dropped rule is terminal unless it graduated or retired; otherwise the only
|
||||
loosening left is a raised limit.
|
||||
"""
|
||||
base_limit = base_limits[rule]
|
||||
if rule not in head_limits:
|
||||
if rule in retired or (graduated and rule.startswith(graduated)):
|
||||
return None
|
||||
return f"rule dropped (limit {base_limit} -> removed)"
|
||||
if head_limits[rule] > base_limit:
|
||||
return f"limit raised {base_limit} -> {head_limits[rule]}"
|
||||
return None
|
||||
|
||||
|
||||
def regressions_for(
|
||||
rel: str,
|
||||
base: dict | None,
|
||||
head: dict | None,
|
||||
graduated: tuple[str, ...] = (),
|
||||
retired: frozenset[str] = frozenset(),
|
||||
) -> list[Regression]:
|
||||
if base is None:
|
||||
return [] # new budget file: nothing to ratchet against yet
|
||||
if head is None:
|
||||
return [Regression(rel, "*", "budget file was deleted (every limit removed)")]
|
||||
|
||||
base_limits, head_limits = _limits(base), _limits(head)
|
||||
return [
|
||||
Regression(rel, rule, detail)
|
||||
for rule in sorted(base_limits)
|
||||
if (detail := _regression_detail(rule, base_limits, head_limits, graduated, retired)) is not None
|
||||
]
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--base", help="Comparison ref (default: origin's current default branch)")
|
||||
parser.add_argument("budgets", nargs="*", help="budget files to check")
|
||||
args = parser.parse_args()
|
||||
from default_branch import resolve_base_ref
|
||||
|
||||
base_ref: Final = resolve_base_ref(args.base, REPO_ROOT)
|
||||
budgets = args.budgets or list(DEFAULT_BUDGETS)
|
||||
|
||||
ref = _merge_base(base_ref)
|
||||
if not _ref_is_commit(ref):
|
||||
print(
|
||||
f"FAIL: base ref {ref!r} does not resolve to a commit, so the ratchet has nothing "
|
||||
f"to compare against; refusing to pass vacuously (check the --base / BASE_SHA value)",
|
||||
file=sys.stderr,
|
||||
)
|
||||
return 1
|
||||
|
||||
regressions: list[Regression] = []
|
||||
checked: list[str] = []
|
||||
for rel in budgets:
|
||||
base = _load_base(rel, ref)
|
||||
head = _load_head(rel)
|
||||
if base is None and head is None:
|
||||
continue
|
||||
if base is None:
|
||||
print(f"skip {rel}: new file (no base at {base_ref} to ratchet against)")
|
||||
continue
|
||||
checked.append(rel)
|
||||
regressions.extend(regressions_for(rel, base, head, graduated_selectors(rel), retired_rules(rel, base)))
|
||||
|
||||
if regressions:
|
||||
print(
|
||||
f"FAIL: budget limit(s) loosened vs base {base_ref} (merge-base {ref[:12]}):"
|
||||
)
|
||||
for reg in regressions:
|
||||
print(f" {reg.budget} {reg.rule}: {reg.detail}")
|
||||
print(
|
||||
"Budgets are one-way ratchets and may only go down or stay flat. This "
|
||||
"check is non-gating: if the increase is justified (e.g. a newly banned "
|
||||
"API), a human can merge over the red after acknowledging it."
|
||||
)
|
||||
return 1
|
||||
|
||||
suffix = f" ({', '.join(checked)})" if checked else ""
|
||||
print(f"OK: no budget limit increased vs base {base_ref}{suffix}")
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
raise SystemExit(main())
|
||||
|
|
@ -4,8 +4,8 @@
|
|||
Sibling of scripts/check_type_discipline.py, same output contract
|
||||
(``path:line: CODE message``) and same stdlib-only constraint, aimed at the test
|
||||
tree instead of the package. Each rule is a shape the testing-strategy audit
|
||||
measured and named; scripts/test_quality_gate.py caps the codebase total of each
|
||||
one against test-quality-budget.json so the counts can only ratchet down.
|
||||
measured and named; scripts/test_quality_gate.py fails any change that grows the
|
||||
codebase total of one past its merge-base count.
|
||||
|
||||
Rules
|
||||
-----
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
#!/usr/bin/env python3
|
||||
"""Machine-wide slot lock for this repo's heavy entrypoints.
|
||||
|
||||
`make check`, `make lint`, and the standalone budget gates
|
||||
`make check`, `make lint`, and the standalone lint gates
|
||||
(scripts/ruff_strict_gate.py, scripts/type_discipline_gate.py,
|
||||
scripts/type_check_gate.py) each hold one of N machine-wide slots while they
|
||||
run, so however many sessions and worktrees share one machine, at most N of
|
||||
|
|
|
|||
327
scripts/lint_base_counts.py
Normal file
327
scripts/lint_base_counts.py
Normal file
|
|
@ -0,0 +1,327 @@
|
|||
#!/usr/bin/env python3
|
||||
"""Merge-base counts for the delta-vs-base lint gates.
|
||||
|
||||
Each gate (scripts/ruff_strict_gate.py, scripts/type_discipline_gate.py,
|
||||
scripts/type_check_gate.py, scripts/test_quality_gate.py) counts its rules
|
||||
across the whole tree at HEAD and at the merge-base with the branch the change
|
||||
merges into, and fails only when a rule grew past its ceiling: the merge-base
|
||||
count, or the rule's fixed codebase-wide cap when the gate sets one and it is
|
||||
higher. There is no committed budget, so a ceiling moves only when the base
|
||||
branch does or when someone lowers a cap on it.
|
||||
|
||||
The merge-base counts come from, in order, the disk cache under the git common
|
||||
dir, the CI artifact publish-lint-base-counts.yml uploads for every push to the
|
||||
default branch, and a scan of the base tree in a temporary worktree. Every
|
||||
entry is keyed by the merge-base commit plus the checker's fingerprints (its
|
||||
config, its rule logic, its tool version), so counts measured under a different
|
||||
rule set are never matched, only recomputed.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import io
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import zipfile
|
||||
from collections.abc import Callable, Mapping, Sequence
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from types import MappingProxyType
|
||||
from typing import Final, NamedTuple, TypeAlias
|
||||
|
||||
REPO_ROOT: Final = Path(__file__).resolve().parents[1]
|
||||
CACHE_DIR_NAME: Final = "litellm-lint-cache"
|
||||
CACHE_KEEP_ENTRIES: Final = 8
|
||||
GH_TIMEOUT_SECONDS: Final = 10
|
||||
|
||||
_ORIGIN_SLUG: Final = re.compile(r"(?:git@github\.com:|https://github\.com/)([^/]+/[^/]+?)(?:\.git)?/?")
|
||||
|
||||
Counts: TypeAlias = Mapping[str, int]
|
||||
GhOutput: TypeAlias = Callable[[Sequence[str]], bytes | None]
|
||||
NO_CAPS: Final[Counts] = MappingProxyType({})
|
||||
|
||||
|
||||
class Breach(NamedTuple):
|
||||
rule: str
|
||||
total: int
|
||||
ceiling: int
|
||||
added: int
|
||||
|
||||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
class Checker:
|
||||
name: str
|
||||
fingerprints: tuple[str, ...]
|
||||
|
||||
def key(self, base_point: str) -> str:
|
||||
return cache_key(base_point, self.fingerprints)
|
||||
|
||||
def artifact_name(self, base_point: str) -> str:
|
||||
return f"{self.name}-counts-{self.key(base_point)}"
|
||||
|
||||
def cache_file_name(self, base_point: str) -> str:
|
||||
return f"{self.name}-base-{self.key(base_point)}.json"
|
||||
|
||||
def cache_glob(self) -> str:
|
||||
return f"{self.name}-base-*.json"
|
||||
|
||||
|
||||
Fetch: TypeAlias = Callable[[Checker, str], Counts | None]
|
||||
|
||||
|
||||
def sha256_of(path: Path) -> str:
|
||||
return hashlib.sha256(path.read_bytes()).hexdigest()
|
||||
|
||||
|
||||
def cache_key(base_point: str, fingerprints: Sequence[str]) -> str:
|
||||
return hashlib.sha256("|".join((base_point, *fingerprints)).encode()).hexdigest()[:16]
|
||||
|
||||
|
||||
Git: TypeAlias = Callable[[Sequence[str]], str]
|
||||
|
||||
|
||||
def git_in(cwd: Path) -> Git:
|
||||
def run(args: Sequence[str]) -> str:
|
||||
proc: Final = subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True)
|
||||
if proc.returncode not in (0, 1):
|
||||
sys.stderr.write(proc.stderr)
|
||||
raise SystemExit(f"git exited {proc.returncode}")
|
||||
return proc.stdout
|
||||
|
||||
return run
|
||||
|
||||
|
||||
REPO_GIT: Final = git_in(REPO_ROOT)
|
||||
|
||||
|
||||
def head_sha(git: Git = REPO_GIT) -> str:
|
||||
return git(["rev-parse", "HEAD"]).strip()
|
||||
|
||||
|
||||
def resolve_base_point(base_ref: str, git: Git = REPO_GIT) -> str:
|
||||
"""The snapshot commit base counts are measured at: merge-base(base_ref, HEAD),
|
||||
made aware of an in-progress merge. Mid-merge, HEAD is still the pre-merge tip,
|
||||
so its merge-base is the old branch point and every violation the base gained
|
||||
since then would be blamed on this change. While MERGE_HEAD exists, prefer
|
||||
merge-base(base_ref, MERGE_HEAD) whenever it is the newer of the two."""
|
||||
head_point: Final = git(["merge-base", base_ref, "HEAD"]).strip()
|
||||
if not head_point:
|
||||
return base_ref
|
||||
merge_head: Final = git(["rev-parse", "--verify", "--quiet", "MERGE_HEAD"]).strip()
|
||||
if not merge_head:
|
||||
return head_point
|
||||
merge_point: Final = git(["merge-base", base_ref, merge_head]).strip()
|
||||
if not merge_point:
|
||||
return head_point
|
||||
older: Final = git(["merge-base", head_point, merge_point]).strip()
|
||||
return merge_point if older == head_point else head_point
|
||||
|
||||
|
||||
def default_cache_dir(git: Git = REPO_GIT) -> Path:
|
||||
return Path(git(["rev-parse", "--path-format=absolute", "--git-common-dir"]).strip()) / CACHE_DIR_NAME
|
||||
|
||||
|
||||
def validated_counts(data: object) -> Counts | None:
|
||||
counts: Final = data.get("counts") if isinstance(data, dict) else None
|
||||
if not isinstance(counts, dict):
|
||||
return None
|
||||
if not all(
|
||||
isinstance(code, str) and isinstance(total, int) and not isinstance(total, bool)
|
||||
for code, total in counts.items()
|
||||
):
|
||||
return None
|
||||
return counts
|
||||
|
||||
|
||||
def load_cached_counts(path: Path) -> Counts | None:
|
||||
try:
|
||||
data: Final = json.loads(path.read_text())
|
||||
except (OSError, json.JSONDecodeError):
|
||||
return None
|
||||
return validated_counts(data)
|
||||
|
||||
|
||||
def scratch_path(path: Path) -> Path:
|
||||
return path.with_name(f".{path.name}.{os.getpid()}.tmp")
|
||||
|
||||
|
||||
def counts_payload(base_point: str, counts: Counts) -> str:
|
||||
return json.dumps({"base_point": base_point, "counts": dict(sorted(counts.items()))}, indent=2) + "\n"
|
||||
|
||||
|
||||
def entry_recency(path: Path) -> float:
|
||||
try:
|
||||
return path.stat().st_mtime
|
||||
except OSError:
|
||||
return 0.0
|
||||
|
||||
|
||||
def evicted_beyond_cap(entries: Sequence[Path], keep: int) -> tuple[Path, ...]:
|
||||
newest_first: Final = sorted(entries, key=entry_recency, reverse=True)
|
||||
return tuple(newest_first[keep:])
|
||||
|
||||
|
||||
def store_counts(directory: Path, checker: Checker, base_point: str, counts: Counts) -> Path:
|
||||
directory.mkdir(parents=True, exist_ok=True)
|
||||
path: Final = directory / checker.cache_file_name(base_point)
|
||||
scratch: Final = scratch_path(path)
|
||||
scratch.write_text(counts_payload(base_point, counts))
|
||||
scratch.replace(path)
|
||||
siblings: Final = tuple(entry for entry in directory.glob(checker.cache_glob()) if entry != path)
|
||||
for stale in evicted_beyond_cap(siblings, CACHE_KEEP_ENTRIES - 1):
|
||||
stale.unlink(missing_ok=True)
|
||||
return path
|
||||
|
||||
|
||||
def parse_origin_slug(url: str) -> str | None:
|
||||
match: Final = _ORIGIN_SLUG.fullmatch(url.strip())
|
||||
return match.group(1) if match else None
|
||||
|
||||
|
||||
def origin_slug(cwd: Path = REPO_ROOT) -> str | None:
|
||||
proc: Final = subprocess.run(["git", "remote", "get-url", "origin"], cwd=cwd, capture_output=True, text=True)
|
||||
return parse_origin_slug(proc.stdout) if proc.returncode == 0 else None
|
||||
|
||||
|
||||
def gh_output(args: Sequence[str]) -> bytes | None:
|
||||
try:
|
||||
proc: Final = subprocess.run(["gh", *args], capture_output=True, timeout=GH_TIMEOUT_SECONDS)
|
||||
except (OSError, subprocess.SubprocessError):
|
||||
return None
|
||||
return proc.stdout if proc.returncode == 0 else None
|
||||
|
||||
|
||||
def _parsed_json(raw: bytes) -> object | None:
|
||||
try:
|
||||
return json.loads(raw)
|
||||
except ValueError:
|
||||
return None
|
||||
|
||||
|
||||
def _artifact_download_url(listing: object) -> str | None:
|
||||
artifacts: Final = listing.get("artifacts") if isinstance(listing, dict) else None
|
||||
if not isinstance(artifacts, list) or not artifacts:
|
||||
return None
|
||||
newest: Final = artifacts[0]
|
||||
if not isinstance(newest, dict) or newest.get("expired"):
|
||||
return None
|
||||
url: Final = newest.get("archive_download_url")
|
||||
return url if isinstance(url, str) else None
|
||||
|
||||
|
||||
def _counts_json_from_zip(zip_bytes: bytes) -> object | None:
|
||||
try:
|
||||
with zipfile.ZipFile(io.BytesIO(zip_bytes)) as archive:
|
||||
members: Final = tuple(name for name in archive.namelist() if name.endswith(".json"))
|
||||
if len(members) != 1:
|
||||
return None
|
||||
return json.loads(archive.read(members[0]))
|
||||
except (zipfile.BadZipFile, ValueError, OSError):
|
||||
return None
|
||||
|
||||
|
||||
def counts_for_base(payload: object, base_point: str) -> Counts | None:
|
||||
if not isinstance(payload, dict) or payload.get("base_point") != base_point:
|
||||
return None
|
||||
counts: Final = validated_counts(payload)
|
||||
return counts if counts else None
|
||||
|
||||
|
||||
def _fetch_fallback(reason: str) -> None:
|
||||
sys.stderr.write(f"{reason}; computing base counts locally\n")
|
||||
|
||||
|
||||
def fetch_ci_base_counts(
|
||||
checker: Checker,
|
||||
base_point: str,
|
||||
gh: GhOutput = gh_output,
|
||||
cwd: Path = REPO_ROOT,
|
||||
) -> Counts | None:
|
||||
"""Base counts from the CI artifact published for `base_point`, or None.
|
||||
|
||||
Every failure mode (no gh, no auth, offline, expired or missing artifact,
|
||||
malformed payload, counts for a different commit) returns None so the
|
||||
caller falls back to the local base scan; the fetch is an optimization and
|
||||
must never make the gate less available than local compute alone."""
|
||||
slug: Final = origin_slug(cwd)
|
||||
if slug is None:
|
||||
return _fetch_fallback("origin remote is not a github.com URL")
|
||||
name: Final = checker.artifact_name(base_point)
|
||||
listing: Final = gh(["api", f"repos/{slug}/actions/artifacts?name={name}&per_page=1"])
|
||||
if listing is None:
|
||||
return _fetch_fallback(f"could not list CI artifacts named {name}")
|
||||
url: Final = _artifact_download_url(_parsed_json(listing))
|
||||
if url is None:
|
||||
return _fetch_fallback(f"no usable CI artifact named {name}")
|
||||
zip_bytes: Final = gh(["api", url])
|
||||
if zip_bytes is None:
|
||||
return _fetch_fallback(f"download failed for CI artifact {name}")
|
||||
counts: Final = counts_for_base(_counts_json_from_zip(zip_bytes), base_point)
|
||||
if counts is None:
|
||||
return _fetch_fallback(f"CI artifact {name} is not valid base counts for {base_point[:12]}")
|
||||
sys.stderr.write(f"base counts fetched from CI artifact {name}\n")
|
||||
return counts
|
||||
|
||||
|
||||
def base_counts_cached(
|
||||
checker: Checker,
|
||||
base_point: str,
|
||||
compute: Callable[[str], Counts],
|
||||
cache_dir: Path | None = None,
|
||||
fetch: Fetch = fetch_ci_base_counts,
|
||||
) -> Counts:
|
||||
"""`compute` memoized on disk. The base tree at a given commit is immutable,
|
||||
so its counts are a pure function of the merge-base plus the checker's
|
||||
fingerprints in the cache key; an empty result is never stored because it is
|
||||
the signature of a crashed pass, not a clean tree. On a disk miss the counts
|
||||
CI already published for the merge-base are fetched before the expensive
|
||||
local base scan; a fetch miss of any kind computes locally."""
|
||||
directory: Final = default_cache_dir() if cache_dir is None else cache_dir
|
||||
cached: Final = load_cached_counts(directory / checker.cache_file_name(base_point))
|
||||
if cached is not None:
|
||||
return cached
|
||||
fetched: Final = fetch(checker, base_point)
|
||||
if fetched:
|
||||
store_counts(directory, checker, base_point, fetched)
|
||||
return fetched
|
||||
counts: Final = compute(base_point)
|
||||
if counts:
|
||||
store_counts(directory, checker, base_point, counts)
|
||||
return counts
|
||||
|
||||
|
||||
def emit_counts(checker: Checker, counts: Counts, directory: Path, head_point: str) -> Path:
|
||||
"""Write HEAD's per-rule counts as the file the publisher workflow uploads.
|
||||
|
||||
The filename stem is exactly the artifact name `fetch_ci_base_counts` will
|
||||
later look up for this commit, so emit and fetch cannot drift apart. Empty
|
||||
counts are refused: a pass that produced nothing almost certainly crashed,
|
||||
and publishing it would poison every branch that fetches it."""
|
||||
if not counts:
|
||||
print(
|
||||
f"FAIL: {checker.name} produced no violations; refusing to publish empty base "
|
||||
"counts because the pass almost certainly crashed or emitted nothing."
|
||||
)
|
||||
raise SystemExit(1)
|
||||
name: Final = checker.artifact_name(head_point)
|
||||
directory.mkdir(parents=True, exist_ok=True)
|
||||
path: Final = directory / f"{name}.json"
|
||||
path.write_text(counts_payload(head_point, counts))
|
||||
print(f"Emitted base counts for {head_point} as {name}.json ({sum(counts.values())} violations total)")
|
||||
return path
|
||||
|
||||
|
||||
def ceiling(rule: str, base: Counts, caps: Counts) -> int:
|
||||
return max(base.get(rule, 0), caps.get(rule, 0))
|
||||
|
||||
|
||||
def evaluate(head: Counts, base: Counts, caps: Counts = NO_CAPS) -> tuple[Breach, ...]:
|
||||
return tuple(
|
||||
Breach(rule, total, ceiling(rule, base, caps), total - base.get(rule, 0))
|
||||
for rule, total in sorted(head.items())
|
||||
if total > ceiling(rule, base, caps)
|
||||
)
|
||||
|
|
@ -13,10 +13,10 @@
|
|||
# - tests/e2e and tests/e2e_harness Python
|
||||
# -> `make lint-e2e-basedpyright` (test-linting.yml's e2e type-check step)
|
||||
# + raw HTTP client ban (test-code-quality.yml's check_e2e_no_raw_requests)
|
||||
# - tests/ Python, ruff-tests.toml, test-quality-budget.json, scripts/check_test_quality.py,
|
||||
# - tests/ Python, ruff-tests.toml, scripts/check_test_quality.py,
|
||||
# scripts/test_quality_gate.py
|
||||
# -> ruff over ruff-tests.toml + `make lint-test-quality` (test-linting.yml's
|
||||
# test-tree ruff and test-quality budget steps)
|
||||
# test-tree ruff and test-quality gate steps)
|
||||
# - dashboard -> prettier + eslint + lint budgets (test-litellm-ui-build.yml's frontend-lint)
|
||||
# - proxy/types -> regenerate the lazy OpenAPI snapshot and dashboard API types, fail on drift (check-ui-api-types.yml)
|
||||
#
|
||||
|
|
@ -33,7 +33,7 @@ set -eu
|
|||
# before anything else, so N parallel `make check` runs across worktrees execute two
|
||||
# at a time instead of thrashing the machine. The wrapper exports
|
||||
# LITELLM_GATE_SLOT_HELD, so this re-exec happens exactly once and everything this
|
||||
# script spawns (make lint, the budget gates) skips its own acquisition.
|
||||
# script spawns (make lint, the lint gates) skips its own acquisition.
|
||||
script_dir=$(python3 -c 'import os, sys; print(os.path.dirname(os.path.realpath(sys.argv[1])))' "$0")
|
||||
if [ -z "${LITELLM_GATE_SLOT_HELD:-}" ]; then
|
||||
exec python3 "$script_dir/gate_slot_lock.py" "$0" "$@"
|
||||
|
|
@ -97,7 +97,7 @@ existing_files() {
|
|||
|
||||
litellm_py_pattern='^litellm/.*\.py$'
|
||||
e2e_py_pattern='^tests/e2e(_harness)?/.*\.py$'
|
||||
test_tree_pattern='^(tests/.*\.py|ruff-tests\.toml|test-quality-budget\.json|scripts/(check_test_quality|test_quality_gate)\.py)$'
|
||||
test_tree_pattern='^(tests/.*\.py|ruff-tests\.toml|scripts/(check_test_quality|test_quality_gate)\.py)$'
|
||||
spec_pattern='^(litellm/(proxy|types)/.*|ui/litellm-dashboard/(scripts/gen-api-types\.mjs|package\.json|package-lock\.json|src/lib/http/schema\.d\.ts))$'
|
||||
ui_prettier_pattern='^ui/litellm-dashboard/.*\.(js|jsx|ts|tsx|mjs|cjs|json|css|scss|md|mdx|yml|yaml|html)$'
|
||||
ui_eslint_pattern='^ui/litellm-dashboard/.*\.(js|jsx|ts|tsx|mjs|cjs)$'
|
||||
|
|
@ -147,7 +147,7 @@ if [ -n "$staged" ]; then
|
|||
}
|
||||
warn_skipped "Python lint (make lint)" "$litellm_py_pattern" "$litellm_py_files"
|
||||
warn_skipped "tests/e2e checks (basedpyright + raw HTTP client ban)" "$e2e_py_pattern" "$e2e_py_files"
|
||||
warn_skipped "test-tree lint (ruff-tests.toml + test-quality budget)" "$test_tree_pattern" "$test_tree_files"
|
||||
warn_skipped "test-tree lint (ruff-tests.toml + test-quality gate)" "$test_tree_pattern" "$test_tree_files"
|
||||
warn_skipped "dashboard lint (prettier + eslint + lint budgets)" "$ui_prettier_pattern" "$ui_prettier_changed"
|
||||
warn_skipped "dashboard API-type sync (npm run gen:api)" "$spec_pattern" "$spec_files"
|
||||
fi
|
||||
|
|
@ -304,9 +304,9 @@ if [ -n "$test_tree_files" ] && [ -z "$litellm_py_files" ]; then
|
|||
echo "check: linting the test tree (ruff check --config ruff-tests.toml tests)"
|
||||
uv run --no-sync ruff check --config ruff-tests.toml tests \
|
||||
|| { echo "✗ Test-tree ruff failed. Fix the errors above, then re-run make check." >&2; status=1; }
|
||||
echo "check: checking the test-quality budget (make lint-test-quality)"
|
||||
echo "check: checking the test-quality gate (make lint-test-quality)"
|
||||
make lint-test-quality \
|
||||
|| { echo "✗ Test-quality budget failed. Fix the errors above, then re-run make check." >&2; status=1; }
|
||||
|| { echo "✗ Test-quality gate failed. Fix the errors above, then re-run make check." >&2; status=1; }
|
||||
fi
|
||||
|
||||
if [ -n "${python_pid:-}" ]; then
|
||||
|
|
@ -334,7 +334,7 @@ summary_item() {
|
|||
echo "check: summary"
|
||||
summary_item "Python lint (make lint)" "$litellm_py_files" "no litellm/ Python files in scope"
|
||||
summary_item "tests/e2e checks (basedpyright + raw HTTP client ban)" "$e2e_py_files" "no tests/e2e Python files in scope"
|
||||
summary_item "test-tree lint (ruff-tests.toml + test-quality budget)" "$test_tree_files" \
|
||||
summary_item "test-tree lint (ruff-tests.toml + test-quality gate)" "$test_tree_files" \
|
||||
"no tests/ Python files or test-tree lint inputs in scope"
|
||||
summary_item "dashboard lint (prettier + eslint + lint budgets)" "$ui_prettier_changed$ui_eslint_changed" "no dashboard files in scope"
|
||||
summary_item "dashboard API-type sync (npm run gen:api)" "$spec_files" "no litellm/proxy, litellm/types, or generator files in scope"
|
||||
|
|
|
|||
|
|
@ -1,12 +1,17 @@
|
|||
#!/usr/bin/env python3
|
||||
"""Total-count gate for the strict ruff rules in ruff-strict.toml.
|
||||
"""Delta-vs-base gate for the strict ruff rules in ruff-strict.toml.
|
||||
|
||||
Each rule has a hard ``limit`` in ruff-strict-budget.json. The gate counts each
|
||||
rule across the whole tree and fails when a rule is both over its limit and
|
||||
higher than the base it merges into, so a change is blamed for the violations it
|
||||
adds, never for drift that already exists in the base. ``--update`` ratchets each
|
||||
rule's limit down by the number of violations this branch fixed relative to its
|
||||
branch point (the merge-base).
|
||||
Each rule is counted across the whole tree at HEAD and at the merge-base with
|
||||
the branch this change merges into, and the gate fails only when a rule grew
|
||||
past the merge-base count, so a change is blamed for the violations it adds,
|
||||
never for drift that already sits in the base. There is no committed budget: the merge-base count is the
|
||||
ceiling, so it moves only when the base branch does.
|
||||
|
||||
The merge-base counts come from scripts/lint_base_counts.py: the disk cache,
|
||||
then the CI artifact published for that commit, then a ruff pass over a
|
||||
detached worktree at the merge-base under the current ruff configs.
|
||||
``--emit-counts-dir`` writes HEAD's counts as the file that artifact is built
|
||||
from.
|
||||
"""
|
||||
|
||||
import argparse
|
||||
|
|
@ -17,15 +22,26 @@ import subprocess
|
|||
import sys
|
||||
import tempfile
|
||||
from collections import Counter
|
||||
from collections.abc import Callable, Mapping, Sequence
|
||||
from pathlib import Path
|
||||
from typing import Final, NamedTuple
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
STRICT_CONFIG = REPO_ROOT / "ruff-strict.toml"
|
||||
BUDGET_PATH = REPO_ROOT / "ruff-strict-budget.json"
|
||||
TARGET = "litellm"
|
||||
from lint_base_counts import (
|
||||
Checker,
|
||||
base_counts_cached,
|
||||
emit_counts,
|
||||
evaluate,
|
||||
head_sha,
|
||||
resolve_base_point,
|
||||
sha256_of,
|
||||
)
|
||||
|
||||
_HUNK = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@")
|
||||
REPO_ROOT: Final = Path(__file__).resolve().parent.parent
|
||||
STRICT_CONFIG: Final = REPO_ROOT / "ruff-strict.toml"
|
||||
BASE_CONFIG: Final = REPO_ROOT / "ruff.toml"
|
||||
TARGET: Final = "litellm"
|
||||
|
||||
_HUNK: Final = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@")
|
||||
|
||||
|
||||
class Violation(NamedTuple):
|
||||
|
|
@ -34,38 +50,22 @@ class Violation(NamedTuple):
|
|||
code: str
|
||||
|
||||
|
||||
class Breach(NamedTuple):
|
||||
rule: str
|
||||
total: int
|
||||
cap: int
|
||||
added: int
|
||||
|
||||
|
||||
def _run(cmd: list, cwd: Path = REPO_ROOT) -> str:
|
||||
proc = subprocess.run(cmd, cwd=cwd, capture_output=True, text=True)
|
||||
def _run(cmd: Sequence[str], cwd: Path = REPO_ROOT) -> str:
|
||||
proc: Final = subprocess.run(cmd, cwd=cwd, capture_output=True, text=True)
|
||||
if proc.returncode not in (0, 1):
|
||||
sys.stderr.write(proc.stderr)
|
||||
raise SystemExit(f"{cmd[0]} exited {proc.returncode}")
|
||||
return proc.stdout
|
||||
|
||||
|
||||
def resolve_base_point(base_ref: str, cwd: Path = REPO_ROOT) -> str:
|
||||
"""The snapshot commit base counts are measured at: merge-base(base_ref, HEAD),
|
||||
made aware of an in-progress merge. Mid-merge, HEAD is still the pre-merge tip,
|
||||
so its merge-base is the old branch point and every violation the base gained
|
||||
since then would be blamed on this change. While MERGE_HEAD exists, prefer
|
||||
merge-base(base_ref, MERGE_HEAD) whenever it is the newer of the two."""
|
||||
head_point: Final = _run(["git", "merge-base", base_ref, "HEAD"], cwd=cwd).strip()
|
||||
if not head_point:
|
||||
return base_ref
|
||||
merge_head: Final = _run(["git", "rev-parse", "--verify", "--quiet", "MERGE_HEAD"], cwd=cwd).strip()
|
||||
if not merge_head:
|
||||
return head_point
|
||||
merge_point: Final = _run(["git", "merge-base", base_ref, merge_head], cwd=cwd).strip()
|
||||
if not merge_point:
|
||||
return head_point
|
||||
older: Final = _run(["git", "merge-base", head_point, merge_point], cwd=cwd).strip()
|
||||
return merge_point if older == head_point else head_point
|
||||
def ruff_version() -> str:
|
||||
return _run(["ruff", "--version"]).strip()
|
||||
|
||||
|
||||
def checker_identity(
|
||||
strict_config: Path = STRICT_CONFIG, base_config: Path = BASE_CONFIG, version: Callable[[], str] = ruff_version
|
||||
) -> Checker:
|
||||
return Checker("ruff-strict", (sha256_of(strict_config), sha256_of(base_config), version()))
|
||||
|
||||
|
||||
def _ruff_json(cwd: Path, config: Path) -> list:
|
||||
|
|
@ -76,7 +76,7 @@ def _ruff_json(cwd: Path, config: Path) -> list:
|
|||
return json.loads(raw or "[]")
|
||||
|
||||
|
||||
def head_violations() -> list:
|
||||
def head_violations() -> list[Violation]:
|
||||
out = []
|
||||
for item in _ruff_json(REPO_ROOT, STRICT_CONFIG):
|
||||
name = Path(item["filename"])
|
||||
|
|
@ -90,47 +90,26 @@ def head_violations() -> list:
|
|||
return out
|
||||
|
||||
|
||||
def count_by_rule(violations: list) -> dict:
|
||||
def count_by_rule(violations: Sequence[Violation]) -> dict[str, int]:
|
||||
return dict(Counter(v.code for v in violations))
|
||||
|
||||
|
||||
def base_counts(ref: str) -> dict:
|
||||
parent = Path(tempfile.mkdtemp(prefix="ruff_base_"))
|
||||
worktree = parent / "wt"
|
||||
def base_counts(ref: str) -> dict[str, int]:
|
||||
parent: Final = Path(tempfile.mkdtemp(prefix="ruff_base_"))
|
||||
worktree: Final = parent / "wt"
|
||||
try:
|
||||
_run(["git", "worktree", "add", "--detach", str(worktree), ref])
|
||||
shutil.copy(STRICT_CONFIG, worktree / "ruff-strict.toml")
|
||||
items = _ruff_json(worktree, worktree / "ruff-strict.toml")
|
||||
shutil.copy(BASE_CONFIG, worktree / BASE_CONFIG.name)
|
||||
shutil.copy(STRICT_CONFIG, worktree / STRICT_CONFIG.name)
|
||||
items: Final = _ruff_json(worktree, worktree / STRICT_CONFIG.name)
|
||||
return dict(Counter(item["code"] for item in items))
|
||||
finally:
|
||||
_run(["git", "worktree", "remove", "--force", str(worktree)])
|
||||
shutil.rmtree(parent, ignore_errors=True)
|
||||
|
||||
|
||||
def over_ceiling(head: dict, budget: dict) -> frozenset:
|
||||
"""Rules whose head count already exceeds their limit.
|
||||
|
||||
A rule can only breach when it is over its limit, so when none are the base
|
||||
comparison cannot change the verdict and the base worktree scan can be skipped.
|
||||
"""
|
||||
return frozenset(
|
||||
rule for rule, spec in budget.items()
|
||||
if head.get(rule, 0) > spec["limit"]
|
||||
)
|
||||
|
||||
|
||||
def evaluate(head: dict, base: dict, budget: dict) -> list:
|
||||
breaches = []
|
||||
for rule, spec in budget.items():
|
||||
cap = spec["limit"]
|
||||
total = head.get(rule, 0)
|
||||
if total > cap and total > base.get(rule, 0):
|
||||
breaches.append(Breach(rule, total, cap, total - base.get(rule, 0)))
|
||||
return sorted(breaches)
|
||||
|
||||
|
||||
def parse_changed_lines(diff_text: str) -> dict:
|
||||
changed: dict = {}
|
||||
def parse_changed_lines(diff_text: str) -> dict[str, set[int]]:
|
||||
changed: dict[str, set[int]] = {}
|
||||
path = None
|
||||
for line in diff_text.splitlines():
|
||||
if line.startswith("+++ b/"):
|
||||
|
|
@ -142,84 +121,49 @@ def parse_changed_lines(diff_text: str) -> dict:
|
|||
return changed
|
||||
|
||||
|
||||
def introduced(violations: list, changed: dict) -> list:
|
||||
def introduced(violations: Sequence[Violation], changed: Mapping[str, set[int]]) -> list[Violation]:
|
||||
return [v for v in violations if v.line in changed.get(v.file, set())]
|
||||
|
||||
|
||||
def cmd_check(base: str) -> None:
|
||||
budget = json.loads(BUDGET_PATH.read_text())
|
||||
head = head_violations()
|
||||
head_counts = count_by_rule(head)
|
||||
if not over_ceiling(head_counts, budget):
|
||||
print(f"OK: every strict rule is within its codebase ceiling (base {base})")
|
||||
return
|
||||
base_point = resolve_base_point(base)
|
||||
breaches = evaluate(head_counts, base_counts(base_point), budget)
|
||||
head: Final = head_violations()
|
||||
base_point: Final = resolve_base_point(base)
|
||||
breaches: Final = evaluate(count_by_rule(head), base_counts_cached(checker_identity(), base_point, base_counts))
|
||||
if not breaches:
|
||||
print(f"OK: every strict rule is within its codebase ceiling (base {base})")
|
||||
print(f"OK: no strict rule grew past its merge-base count (base {base})")
|
||||
return
|
||||
new = introduced(
|
||||
head,
|
||||
parse_changed_lines(
|
||||
_run(["git", "diff", base_point, "--unified=0", "--no-color", "--", TARGET])
|
||||
),
|
||||
)
|
||||
print(f"FAIL: strict-rule totals exceed their limit (base {base}):")
|
||||
diff: Final = _run(["git", "diff", base_point, "--unified=0", "--no-color", "--", TARGET])
|
||||
new: Final = introduced(head, parse_changed_lines(diff))
|
||||
print(f"FAIL: strict-rule totals grew past their merge-base count (base {base}):")
|
||||
for breach in breaches:
|
||||
print(
|
||||
f" {breach.rule}: total {breach.total} over limit {breach.cap} (this change added {breach.added})"
|
||||
)
|
||||
print(f" {breach.rule}: total {breach.total} over ceiling {breach.ceiling} (this change added {breach.added})")
|
||||
for violation in sorted(v for v in new if v.code == breach.rule):
|
||||
print(f" {violation.file}:{violation.line}")
|
||||
print(
|
||||
"Reduce the new violations or remove an equal number elsewhere; the ceiling is the limit in ruff-strict-budget.json."
|
||||
"Reduce the new violations or remove an equal number elsewhere; the ceiling is the merge-base count."
|
||||
)
|
||||
raise SystemExit(1)
|
||||
|
||||
|
||||
def ratcheted_budget(budget: dict, current: dict, base: dict) -> dict:
|
||||
"""Each rule's limit lowered by the violations `current` fixed vs `base`.
|
||||
|
||||
`base` is the count at the branch point (the commit this branch diverged
|
||||
from). The drop is clamped to what was actually cleared (a rule that grew
|
||||
stays put), so the limit only ever falls.
|
||||
"""
|
||||
return {
|
||||
rule: {
|
||||
"limit": max(0, spec["limit"] - max(0, base.get(rule, 0) - current.get(rule, 0)))
|
||||
}
|
||||
for rule, spec in sorted(budget.items())
|
||||
}
|
||||
|
||||
|
||||
def cmd_update(base_ref: str) -> None:
|
||||
"""Ratchet each rule's limit down by the violations this branch fixed.
|
||||
|
||||
The working-tree count is compared against a ruff pass over a detached
|
||||
worktree at the branch point (the merge-base with `base_ref`), so a branch's
|
||||
fixes tighten its own ceilings by exactly what they cleared since it diverged.
|
||||
"""
|
||||
budget = json.loads(BUDGET_PATH.read_text())
|
||||
base_point = resolve_base_point(base_ref)
|
||||
updated = ratcheted_budget(
|
||||
budget, count_by_rule(head_violations()), base_counts(base_point)
|
||||
)
|
||||
BUDGET_PATH.write_text(json.dumps(updated, indent=2, sort_keys=True) + "\n")
|
||||
cleared = sum(budget[rule]["limit"] - updated[rule]["limit"] for rule in updated)
|
||||
print(f"Ratcheted strict-rule limits down by {cleared} violations this branch fixed")
|
||||
|
||||
|
||||
def main() -> None:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser: Final = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--base", help="Comparison ref (default: origin's current default branch)")
|
||||
parser.add_argument("--update", action="store_true")
|
||||
args = parser.parse_args()
|
||||
parser.add_argument(
|
||||
"--emit-counts-dir",
|
||||
type=Path,
|
||||
help="Write HEAD's per-rule counts to this directory as a base-counts artifact instead of gating",
|
||||
)
|
||||
args: Final = parser.parse_args()
|
||||
from default_branch import resolve_base_ref
|
||||
from gate_slot_lock import held_slot
|
||||
|
||||
if args.emit_counts_dir is not None:
|
||||
with held_slot():
|
||||
emit_counts(checker_identity(), count_by_rule(head_violations()), args.emit_counts_dir, head_sha())
|
||||
return
|
||||
base_ref: Final = resolve_base_ref(args.base, REPO_ROOT)
|
||||
with held_slot():
|
||||
cmd_update(base_ref) if args.update else cmd_check(base_ref)
|
||||
cmd_check(base_ref)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
|
|
|||
|
|
@ -1,32 +1,24 @@
|
|||
#!/usr/bin/env python3
|
||||
"""Total-count gate for the TQ* rules in scripts/check_test_quality.py.
|
||||
"""Delta-vs-base gate for the TQ* rules in scripts/check_test_quality.py.
|
||||
|
||||
Sibling of scripts/type_discipline_gate.py, pointed at the test tree instead of
|
||||
the package. Each rule listed in test-quality-budget.json has a hard ``limit``.
|
||||
The gate counts each rule across the whole `tests` tree and fails when a rule is
|
||||
both over its limit and higher than the base it merges into, so a change is
|
||||
blamed for the violations it adds, never for drift that already exists in the
|
||||
base.
|
||||
the package. Each rule is counted across the whole `tests` tree at HEAD and at
|
||||
the merge-base with the branch this change merges into, and the gate fails only
|
||||
when a rule grew past the merge-base count, so a change is blamed for the
|
||||
violations it adds, never for drift that already exists in the base. There is
|
||||
no committed budget: the merge-base count is the ceiling, so it moves only when
|
||||
the base branch does.
|
||||
|
||||
Every rule is seeded at exactly its count on the day the gate landed, so the
|
||||
suite's existing debt is grandfathered and any net-new violation trips the gate
|
||||
immediately. ``--update`` ratchets a limit down by the violations fixed relative
|
||||
to ``--base``, so the ceilings only ever fall. Base counts are measured with the
|
||||
*current* checker, so a rule introduced on this branch is counted at the base too
|
||||
and ratchets like every other one. The ratchet runs as a scheduled automation
|
||||
against the repository's default branch, not on PR branches, so concurrent PRs never
|
||||
race to edit the same limit.
|
||||
|
||||
The deliberate difference from its sibling: this gate has no headroom anywhere.
|
||||
Type discipline seeded LIT010/LIT011 at 1.5x to leave room for an in-flight
|
||||
sweep; a test-quality violation has no such transition to absorb, so the line is
|
||||
today's count and the only legal direction is down.
|
||||
The merge-base counts come from scripts/lint_base_counts.py: the disk cache,
|
||||
then the CI artifact published for that commit, then a pass of the current
|
||||
checker over a detached worktree at the merge-base, so a rule introduced on
|
||||
this branch is counted at the base too. ``--emit-counts-dir`` writes HEAD's
|
||||
counts as the file that artifact is built from.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import json
|
||||
import re
|
||||
import shutil
|
||||
import signal
|
||||
|
|
@ -39,9 +31,18 @@ from pathlib import Path
|
|||
from types import FrameType, MappingProxyType
|
||||
from typing import Final, NamedTuple
|
||||
|
||||
from lint_base_counts import (
|
||||
Checker,
|
||||
base_counts_cached,
|
||||
emit_counts,
|
||||
evaluate,
|
||||
head_sha,
|
||||
resolve_base_point,
|
||||
sha256_of,
|
||||
)
|
||||
|
||||
REPO_ROOT: Final = Path(__file__).resolve().parent.parent
|
||||
CHECKER: Final = REPO_ROOT / "scripts" / "check_test_quality.py"
|
||||
BUDGET_PATH: Final = REPO_ROOT / "test-quality-budget.json"
|
||||
TARGET: Final = "tests"
|
||||
TERMINATION_SIGNALS: Final = (signal.SIGTERM, signal.SIGHUP)
|
||||
|
||||
|
|
@ -56,13 +57,6 @@ class Violation(NamedTuple):
|
|||
code: str
|
||||
|
||||
|
||||
class Breach(NamedTuple):
|
||||
rule: str
|
||||
total: int
|
||||
cap: int
|
||||
added: int
|
||||
|
||||
|
||||
def _run(cmd: Sequence[str], cwd: Path = REPO_ROOT) -> str:
|
||||
proc: Final = subprocess.run(cmd, cwd=cwd, capture_output=True, text=True)
|
||||
if proc.returncode not in (0, 1):
|
||||
|
|
@ -71,22 +65,8 @@ def _run(cmd: Sequence[str], cwd: Path = REPO_ROOT) -> str:
|
|||
return proc.stdout
|
||||
|
||||
|
||||
def resolve_base_point(base_ref: str, cwd: Path = REPO_ROOT) -> str:
|
||||
"""The snapshot commit base counts are measured at: merge-base(base_ref, HEAD),
|
||||
made aware of an in-progress merge. Mid-merge, HEAD is still the pre-merge tip,
|
||||
so its merge-base is the old branch point and every violation the base gained
|
||||
since then would be blamed on this change."""
|
||||
head_point: Final = _run(["git", "merge-base", base_ref, "HEAD"], cwd=cwd).strip()
|
||||
if not head_point:
|
||||
return base_ref
|
||||
merge_head: Final = _run(["git", "rev-parse", "--verify", "--quiet", "MERGE_HEAD"], cwd=cwd).strip()
|
||||
if not merge_head:
|
||||
return head_point
|
||||
merge_point: Final = _run(["git", "merge-base", base_ref, merge_head], cwd=cwd).strip()
|
||||
if not merge_point:
|
||||
return head_point
|
||||
older: Final = _run(["git", "merge-base", head_point, merge_point], cwd=cwd).strip()
|
||||
return merge_point if older == head_point else head_point
|
||||
def checker_identity(checker: Path = CHECKER) -> Checker:
|
||||
return Checker("test-quality", (sha256_of(checker),))
|
||||
|
||||
|
||||
def _check(root: Path, checker: Path) -> tuple[Violation, ...]:
|
||||
|
|
@ -143,26 +123,6 @@ def base_counts(ref: str, repo_root: Path = REPO_ROOT, checker: Path = CHECKER)
|
|||
shutil.rmtree(parent, ignore_errors=True)
|
||||
|
||||
|
||||
def over_ceiling(head: Mapping[str, int], budget: Mapping[str, Mapping[str, int]]) -> frozenset[str]:
|
||||
"""Rules whose head count already exceeds their limit. When none are, the base
|
||||
comparison cannot change the verdict and the base worktree scan is skipped."""
|
||||
return frozenset(
|
||||
rule for rule, spec in budget.items() if head.get(rule, 0) > spec["limit"]
|
||||
)
|
||||
|
||||
|
||||
def evaluate(
|
||||
head: Mapping[str, int],
|
||||
base: Mapping[str, int],
|
||||
budget: Mapping[str, Mapping[str, int]],
|
||||
) -> tuple[Breach, ...]:
|
||||
return tuple(sorted(
|
||||
Breach(rule, head.get(rule, 0), spec["limit"], head.get(rule, 0) - base.get(rule, 0))
|
||||
for rule, spec in budget.items()
|
||||
if head.get(rule, 0) > spec["limit"] and head.get(rule, 0) > base.get(rule, 0)
|
||||
))
|
||||
|
||||
|
||||
def _hunk_lines(body: str) -> frozenset[int]:
|
||||
return frozenset(
|
||||
line
|
||||
|
|
@ -191,92 +151,46 @@ def introduced(
|
|||
|
||||
|
||||
def cmd_check(base: str) -> None:
|
||||
budget: Final = json.loads(BUDGET_PATH.read_text())
|
||||
head: Final = head_violations()
|
||||
head_counts: Final = count_by_rule(head)
|
||||
if not over_ceiling(head_counts, budget):
|
||||
print(f"OK: every TQ rule is within its test-suite ceiling (base {base})")
|
||||
return
|
||||
base_point: Final = resolve_base_point(base)
|
||||
base_at_point: Final = base_counts(base_point)
|
||||
breaches: Final = evaluate(head_counts, base_at_point, budget)
|
||||
breaches: Final = evaluate(count_by_rule(head), base_counts_cached(checker_identity(), base_point, base_counts))
|
||||
if not breaches:
|
||||
print(f"OK: every TQ rule is within its test-suite ceiling (base {base})")
|
||||
print(f"OK: no TQ rule grew past its merge-base count (base {base})")
|
||||
return
|
||||
new: Final = introduced(
|
||||
head,
|
||||
parse_changed_lines(
|
||||
_run(["git", "diff", base_point, "--unified=0", "--no-color", "--", TARGET])
|
||||
),
|
||||
)
|
||||
print(f"FAIL: TQ-rule totals exceed their limit (base {base}):")
|
||||
diff: Final = _run(["git", "diff", base_point, "--unified=0", "--no-color", "--", TARGET])
|
||||
new: Final = introduced(head, parse_changed_lines(diff))
|
||||
print(f"FAIL: TQ-rule totals grew past their merge-base count (base {base}):")
|
||||
for breach in breaches:
|
||||
print(
|
||||
f" {breach.rule}: total {breach.total} over limit {breach.cap} "
|
||||
f"(this change added {breach.added})"
|
||||
)
|
||||
print(f" {breach.rule}: total {breach.total} over ceiling {breach.ceiling} (this change added {breach.added})")
|
||||
for violation in sorted(v for v in new if v.code == breach.rule):
|
||||
print(f" {violation.file}:{violation.line}")
|
||||
print(
|
||||
"Fix the new violations, or give each one a reason "
|
||||
"(`# test-quality-ok: <reason>`), or remove an equal number elsewhere; "
|
||||
"the ceiling is the limit in test-quality-budget.json. "
|
||||
"Run `python scripts/check_test_quality.py tests/` to see every finding."
|
||||
"Fix the new violations, or give each one a reason (`# test-quality-ok: <reason>`), or remove an "
|
||||
"equal number elsewhere; the ceiling is the merge-base count. Run "
|
||||
"`python scripts/check_test_quality.py tests/` to see every finding."
|
||||
)
|
||||
raise SystemExit(1)
|
||||
|
||||
|
||||
def ratcheted_budget(
|
||||
budget: Mapping[str, Mapping[str, int]],
|
||||
current: Mapping[str, int],
|
||||
base: Mapping[str, int],
|
||||
) -> Mapping[str, Mapping[str, int]]:
|
||||
"""Each rule's limit lowered by the violations `current` fixed vs `base`. The drop
|
||||
is clamped to what was actually cleared, so a limit only ever falls."""
|
||||
return MappingProxyType({
|
||||
rule: {"limit": max(0, spec["limit"] - max(0, base.get(rule, 0) - current.get(rule, 0)))}
|
||||
for rule, spec in sorted(budget.items())
|
||||
})
|
||||
|
||||
|
||||
def cmd_update(base_ref: str) -> None:
|
||||
"""Ratchet each rule's limit down by the violations this branch fixed."""
|
||||
budget: Final = json.loads(BUDGET_PATH.read_text())
|
||||
base_point: Final = resolve_base_point(base_ref)
|
||||
updated: Final = ratcheted_budget(
|
||||
budget, count_by_rule(head_violations()), base_counts(base_point)
|
||||
)
|
||||
BUDGET_PATH.write_text(json.dumps(dict(updated), indent=2, sort_keys=True) + "\n")
|
||||
cleared: Final = sum(budget[rule]["limit"] - updated[rule]["limit"] for rule in updated)
|
||||
print(f"Ratcheted TQ-rule limits down by {cleared} violations this branch fixed")
|
||||
|
||||
|
||||
def cmd_seed() -> None:
|
||||
"""Write the budget from the working tree's current counts. Used once, to land
|
||||
the gate; afterwards `--update` is the only thing that may move a limit."""
|
||||
counts: Final = count_by_rule(head_violations())
|
||||
BUDGET_PATH.write_text(
|
||||
json.dumps({rule: {"limit": counts[rule]} for rule in sorted(counts)}, indent=2) + "\n"
|
||||
)
|
||||
print(f"Seeded {BUDGET_PATH.name} at " + ", ".join(f"{r}={counts[r]}" for r in sorted(counts)))
|
||||
|
||||
|
||||
def main() -> None:
|
||||
parser: Final = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--base", help="Comparison ref (default: origin's current default branch)")
|
||||
parser.add_argument("--update", action="store_true")
|
||||
parser.add_argument("--seed", action="store_true")
|
||||
parser.add_argument(
|
||||
"--emit-counts-dir",
|
||||
type=Path,
|
||||
help="Write HEAD's per-rule counts to this directory as a base-counts artifact instead of gating",
|
||||
)
|
||||
args: Final = parser.parse_args()
|
||||
from default_branch import resolve_base_ref
|
||||
from gate_slot_lock import held_slot
|
||||
|
||||
if args.emit_counts_dir is not None:
|
||||
with held_slot():
|
||||
emit_counts(checker_identity(), count_by_rule(head_violations()), args.emit_counts_dir, head_sha())
|
||||
return
|
||||
base_ref: Final = resolve_base_ref(args.base, REPO_ROOT)
|
||||
with held_slot():
|
||||
if args.seed:
|
||||
cmd_seed()
|
||||
elif args.update:
|
||||
cmd_update(resolve_base_ref(args.base, REPO_ROOT))
|
||||
else:
|
||||
cmd_check(resolve_base_ref(args.base, REPO_ROOT))
|
||||
cmd_check(base_ref)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
|
|
|||
|
|
@ -2,15 +2,17 @@
|
|||
"""Delta-vs-base per-rule gate for basedpyright.
|
||||
|
||||
basedpyright's ``--outputjson`` is reduced to a count of errors per *rule*
|
||||
(``reportAny``, ``reportArgumentType``, ...) and checked against a committed
|
||||
budget of the form ``{rule: {limit}}``, the same shape as
|
||||
``ruff-strict-budget.json``. A rule fails only when its codebase-wide total is
|
||||
both over its ``limit`` *and* higher than the count on the base it merges into,
|
||||
so a change is blamed for the errors it adds, never for drift that already sits
|
||||
in the base. That ``> base`` guard is what stops an unrelated PR from inheriting
|
||||
a red once two PRs each land near the limit and their sum crosses it: the
|
||||
bystander's count equals its base, so it is spared, while any PR that actually
|
||||
grows the rule past its limit still fails.
|
||||
(``reportAny``, ``reportArgumentType``, ...) at HEAD and at the merge-base with
|
||||
the branch this change merges into. A rule fails only when its codebase-wide
|
||||
total grew past the merge-base count, so a change is blamed for the errors it
|
||||
adds, never for drift that already sits in the base, and an unrelated PR never
|
||||
inherits a red from what landed next to it: its count equals its base.
|
||||
|
||||
``reportAny`` and ``reportExplicitAny`` are the exception, because Any spreads:
|
||||
a correct change can surface new ones far from the lines it touched. Their
|
||||
ceiling is the larger of the merge-base count and a fixed codebase-wide cap in
|
||||
ANY_CAPS, so a change may add some while the total stays under the cap, and the
|
||||
total can never pass it. The cap moves only when someone lowers it on main.
|
||||
|
||||
Installed packages are part of the measurement: a typed dependency that is
|
||||
present changes what basedpyright can prove (and therefore which diagnostics
|
||||
|
|
@ -28,21 +30,16 @@ The gate runs basedpyright itself, for both the head and the base pass, with
|
|||
``NODE_OPTIONS`` raised to the heap this repo needs: basedpyright's node
|
||||
process OOMs at the ~4 GB default, and when callers had to remember the flag,
|
||||
every hand-copied pipeline (Makefile, CI, a dev running the recipe by hand)
|
||||
was one forgotten env line away from an 80-second crash. The base count only
|
||||
matters once some rule is over its limit, so when none is the base pass is
|
||||
skipped outright. When it is needed, it is a second basedpyright pass over a
|
||||
detached worktree at the merge-base, run under the same environment so import
|
||||
resolution matches, and its per-rule counts are cached under the repo's git
|
||||
was one forgotten env line away from an 80-second crash. The base pass is a
|
||||
second basedpyright run over a detached worktree at the merge-base, under the
|
||||
same environment so import resolution matches, and scripts/lint_base_counts.py
|
||||
spares it whenever it can: the per-rule counts are cached under the repo's git
|
||||
common dir keyed by merge-base commit, ``pyrightconfig.json``, ``uv.lock``,
|
||||
the Prisma schema, and the dependency-group set, so re-runs against the same
|
||||
branch point pay for it once. A CI workflow publishes every main commit's counts as
|
||||
an artifact (``--emit-counts-dir`` is its entry point), and on a disk-cache miss
|
||||
the gate first tries to download the merge-base's artifact through the ``gh``
|
||||
CLI; any fetch failure falls back silently to the local base pass, so the gate
|
||||
never gets worse than it was without CI. ``--update`` ratchets each rule's ``limit`` down by the
|
||||
number of errors this branch fixed relative to its branch point (the merge-base),
|
||||
so the headroom you were granted shrinks by exactly what you cleared and never
|
||||
grows.
|
||||
the Prisma schema, and the dependency-group set, and on a disk-cache miss the
|
||||
artifact publish-lint-base-counts.yml uploaded for the merge-base is
|
||||
downloaded through the ``gh`` CLI (``--emit-counts-dir`` is the publisher's
|
||||
entry point); any fetch failure falls back silently to the local base pass, so
|
||||
the gate never gets worse than it was without CI.
|
||||
|
||||
``--outputjson`` is used rather than text diagnostics because the latter wrap
|
||||
across lines, leaving the ``(reportRule)`` on a continuation line away from the
|
||||
|
|
@ -53,33 +50,36 @@ carries an unambiguous ``rule`` field.
|
|||
import argparse
|
||||
import contextlib
|
||||
import hashlib
|
||||
import io
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
import zipfile
|
||||
from collections import Counter
|
||||
from collections.abc import Callable, Iterator, Mapping, Sequence
|
||||
from collections.abc import Callable, Iterator, Mapping
|
||||
from pathlib import Path
|
||||
from typing import Final, NamedTuple
|
||||
from types import MappingProxyType
|
||||
from typing import Final
|
||||
|
||||
from lint_base_counts import (
|
||||
Checker,
|
||||
base_counts_cached,
|
||||
emit_counts,
|
||||
evaluate,
|
||||
head_sha,
|
||||
resolve_base_point,
|
||||
)
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
BUDGET_PATH = REPO_ROOT / "basedpyright-code-budget.json"
|
||||
PYRIGHT_CONFIG = REPO_ROOT / "pyrightconfig.json"
|
||||
UV_LOCK = REPO_ROOT / "uv.lock"
|
||||
CACHE_FILE_PREFIX = "basedpyright-base-"
|
||||
CACHE_KEEP_ENTRIES = 8
|
||||
ARTIFACT_NAME_PREFIX = "basedpyright-counts-"
|
||||
GH_TIMEOUT_SECONDS = 10
|
||||
|
||||
# The one environment every basedpyright pass measures in. The group set is
|
||||
# the slim one the CI publisher has always installed (not bootstrap's fatter
|
||||
# --extra proxy env), so the committed budgets stay valid; changing it re-keys
|
||||
# every cache and artifact fingerprint, so stale counts can never be matched.
|
||||
# --extra proxy env), so published and cached counts stay comparable; changing
|
||||
# it re-keys every cache and artifact fingerprint, so stale counts can never be
|
||||
# matched.
|
||||
TYPECHECK_ENV_DIR = REPO_ROOT / ".venv-typecheck"
|
||||
TYPECHECK_DEP_GROUPS = ("proxy-dev", "e2e-dev")
|
||||
PRISMA_GENERATE_SCRIPT = REPO_ROOT / "scripts" / "prisma_generate_if_needed.py"
|
||||
|
|
@ -93,17 +93,7 @@ NODE_HEAP_OPTION = "--max-old-space-size=8192"
|
|||
# Bucket for a basedpyright diagnostic with no `rule`. Counted so it's gated.
|
||||
UNCODED = "<uncoded>"
|
||||
|
||||
# Limit for a rule that shows up at HEAD but isn't in the budget at all -- a
|
||||
# brand-new error category (new construct, or a tool/version change). The rule
|
||||
# fails once it clears this many errors.
|
||||
DEFAULT_LIMIT = 10
|
||||
|
||||
|
||||
class Breach(NamedTuple):
|
||||
code: str
|
||||
total: int
|
||||
cap: int
|
||||
added: int
|
||||
ANY_CAPS: Final[Mapping[str, int]] = MappingProxyType({"reportAny": 6150, "reportExplicitAny": 1440})
|
||||
|
||||
|
||||
def _to_relative(raw: str, root: Path) -> str | None:
|
||||
|
|
@ -238,25 +228,6 @@ def run_basedpyright(cwd: Path = REPO_ROOT, env_dir: Path = TYPECHECK_ENV_DIR) -
|
|||
return proc.stdout
|
||||
|
||||
|
||||
def resolve_base_point(base_ref: str, cwd: Path = REPO_ROOT) -> str:
|
||||
"""The snapshot commit base counts are measured at: merge-base(base_ref, HEAD),
|
||||
made aware of an in-progress merge. Mid-merge, HEAD is still the pre-merge tip,
|
||||
so its merge-base is the old branch point and every violation the base gained
|
||||
since then would be blamed on this change. While MERGE_HEAD exists, prefer
|
||||
merge-base(base_ref, MERGE_HEAD) whenever it is the newer of the two."""
|
||||
head_point: Final = _run(["git", "merge-base", base_ref, "HEAD"], cwd=cwd).strip()
|
||||
if not head_point:
|
||||
return base_ref
|
||||
merge_head: Final = _run(["git", "rev-parse", "--verify", "--quiet", "MERGE_HEAD"], cwd=cwd).strip()
|
||||
if not merge_head:
|
||||
return head_point
|
||||
merge_point: Final = _run(["git", "merge-base", base_ref, merge_head], cwd=cwd).strip()
|
||||
if not merge_point:
|
||||
return head_point
|
||||
older: Final = _run(["git", "merge-base", head_point, merge_point], cwd=cwd).strip()
|
||||
return merge_point if older == head_point else head_point
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
def _temp_worktree(ref: str) -> Iterator[Path]:
|
||||
parent = Path(tempfile.mkdtemp(prefix="bpr_base_"))
|
||||
|
|
@ -283,21 +254,6 @@ def base_counts(ref: str) -> dict[str, int]:
|
|||
return count_basedpyright(run_basedpyright(worktree), root=worktree)
|
||||
|
||||
|
||||
def over_ceiling(
|
||||
head: Mapping[str, int], budget: Mapping[str, Mapping[str, int]]
|
||||
) -> frozenset[str]:
|
||||
"""Rules whose head count already exceeds their limit.
|
||||
|
||||
A rule can only breach when it is over its limit, so when none are the base
|
||||
comparison cannot change the verdict and the base worktree pass can be skipped.
|
||||
"""
|
||||
return frozenset(
|
||||
code
|
||||
for code, total in head.items()
|
||||
if total > (budget[code]["limit"] if code in budget else DEFAULT_LIMIT)
|
||||
)
|
||||
|
||||
|
||||
def environment_fingerprints(
|
||||
dep_groups: tuple[str, ...] = TYPECHECK_DEP_GROUPS,
|
||||
) -> tuple[str, ...]:
|
||||
|
|
@ -311,377 +267,70 @@ def environment_fingerprints(
|
|||
)
|
||||
|
||||
|
||||
def cache_key(base_point: str, fingerprints: tuple[str, ...]) -> str:
|
||||
return hashlib.sha256("|".join((base_point, *fingerprints)).encode()).hexdigest()[
|
||||
:16
|
||||
]
|
||||
|
||||
|
||||
def cache_path(
|
||||
directory: Path, base_point: str, fingerprints: tuple[str, ...]
|
||||
) -> Path:
|
||||
return directory / f"{CACHE_FILE_PREFIX}{cache_key(base_point, fingerprints)}.json"
|
||||
|
||||
|
||||
def default_cache_dir() -> Path:
|
||||
common = Path(_run(["git", "rev-parse", "--git-common-dir"]).strip())
|
||||
resolved = common if common.is_absolute() else REPO_ROOT / common
|
||||
return resolved / "litellm-lint-cache"
|
||||
|
||||
|
||||
def validated_counts(data: object) -> dict[str, int] | None:
|
||||
counts: Final = data.get("counts") if isinstance(data, dict) else None
|
||||
if not isinstance(counts, dict):
|
||||
return None
|
||||
if not all(
|
||||
isinstance(code, str) and isinstance(total, int) and not isinstance(total, bool)
|
||||
for code, total in counts.items()
|
||||
):
|
||||
return None
|
||||
return counts
|
||||
|
||||
|
||||
def load_cached_counts(path: Path) -> dict[str, int] | None:
|
||||
try:
|
||||
data = json.loads(path.read_text())
|
||||
except (OSError, json.JSONDecodeError):
|
||||
return None
|
||||
return validated_counts(data)
|
||||
|
||||
|
||||
def scratch_path(path: Path) -> Path:
|
||||
"""In-flight scratch for the tmp+rename write. Dot-prefixed so the prune
|
||||
glob in `store_counts` can never match it (a concurrent run would otherwise
|
||||
unlink it between write and rename), and pid-suffixed so two concurrent
|
||||
writers of the same entry never share a scratch."""
|
||||
return path.with_name(f".{path.name}.{os.getpid()}.tmp")
|
||||
|
||||
|
||||
def counts_payload(base_point: str, counts: Mapping[str, int]) -> str:
|
||||
return (
|
||||
json.dumps(
|
||||
{"base_point": base_point, "counts": dict(sorted(counts.items()))},
|
||||
indent=2,
|
||||
)
|
||||
+ "\n"
|
||||
)
|
||||
|
||||
|
||||
def entry_recency(path: Path) -> float:
|
||||
try:
|
||||
return path.stat().st_mtime
|
||||
except OSError:
|
||||
return 0.0
|
||||
|
||||
|
||||
def evicted_beyond_cap(entries: Sequence[Path], keep: int) -> tuple[Path, ...]:
|
||||
newest_first: Final = sorted(entries, key=entry_recency, reverse=True)
|
||||
return tuple(newest_first[keep:])
|
||||
|
||||
|
||||
def store_counts(
|
||||
directory: Path, path: Path, base_point: str, counts: Mapping[str, int]
|
||||
) -> None:
|
||||
directory.mkdir(parents=True, exist_ok=True)
|
||||
scratch = scratch_path(path)
|
||||
scratch.write_text(counts_payload(base_point, counts))
|
||||
scratch.replace(path)
|
||||
siblings: Final = tuple(
|
||||
entry for entry in directory.glob(f"{CACHE_FILE_PREFIX}*.json") if entry != path
|
||||
)
|
||||
for stale in evicted_beyond_cap(siblings, CACHE_KEEP_ENTRIES - 1):
|
||||
stale.unlink(missing_ok=True)
|
||||
|
||||
|
||||
def parse_origin_slug(url: str) -> str | None:
|
||||
match: Final = re.fullmatch(
|
||||
r"(?:git@github\.com:|https://github\.com/)([^/]+/[^/]+?)(?:\.git)?/?",
|
||||
url.strip(),
|
||||
)
|
||||
return match.group(1) if match else None
|
||||
|
||||
|
||||
def origin_slug() -> str | None:
|
||||
proc: Final = subprocess.run(
|
||||
["git", "remote", "get-url", "origin"],
|
||||
cwd=REPO_ROOT,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
if proc.returncode != 0:
|
||||
return None
|
||||
return parse_origin_slug(proc.stdout)
|
||||
|
||||
|
||||
def artifact_name(base_point: str) -> str:
|
||||
return f"{ARTIFACT_NAME_PREFIX}{cache_key(base_point, environment_fingerprints())}"
|
||||
|
||||
|
||||
def _gh_output(args: list[str]) -> bytes | None:
|
||||
try:
|
||||
proc = subprocess.run(
|
||||
["gh", *args], capture_output=True, timeout=GH_TIMEOUT_SECONDS
|
||||
)
|
||||
except (OSError, subprocess.SubprocessError):
|
||||
return None
|
||||
return proc.stdout if proc.returncode == 0 else None
|
||||
|
||||
|
||||
def _parsed_json(raw: bytes) -> object | None:
|
||||
try:
|
||||
return json.loads(raw)
|
||||
except ValueError:
|
||||
return None
|
||||
|
||||
|
||||
def _artifact_download_url(listing: object) -> str | None:
|
||||
artifacts: Final = listing.get("artifacts") if isinstance(listing, dict) else None
|
||||
if not isinstance(artifacts, list) or not artifacts:
|
||||
return None
|
||||
newest: Final = artifacts[0]
|
||||
if not isinstance(newest, dict) or newest.get("expired"):
|
||||
return None
|
||||
url: Final = newest.get("archive_download_url")
|
||||
return url if isinstance(url, str) else None
|
||||
|
||||
|
||||
def _counts_json_from_zip(zip_bytes: bytes) -> object | None:
|
||||
try:
|
||||
with zipfile.ZipFile(io.BytesIO(zip_bytes)) as archive:
|
||||
members: Final = [
|
||||
name for name in archive.namelist() if name.endswith(".json")
|
||||
]
|
||||
if len(members) != 1:
|
||||
return None
|
||||
return json.loads(archive.read(members[0]))
|
||||
except (zipfile.BadZipFile, ValueError, OSError):
|
||||
return None
|
||||
|
||||
|
||||
def counts_for_base(payload: object, base_point: str) -> dict[str, int] | None:
|
||||
if not isinstance(payload, dict) or payload.get("base_point") != base_point:
|
||||
return None
|
||||
counts: Final = validated_counts(payload)
|
||||
return counts if counts else None
|
||||
|
||||
|
||||
def _fetch_fallback(reason: str) -> None:
|
||||
sys.stderr.write(f"{reason}; computing base counts locally\n")
|
||||
|
||||
|
||||
def fetch_ci_base_counts(
|
||||
base_point: str,
|
||||
gh_output: Callable[[list[str]], bytes | None] = _gh_output,
|
||||
) -> dict[str, int] | None:
|
||||
"""Base counts from the CI artifact published for `base_point`, or None.
|
||||
|
||||
Every failure mode (no gh, no auth, offline, expired or missing artifact,
|
||||
malformed payload, counts for a different commit) returns None so the
|
||||
caller falls back to the local base pass; the fetch is an optimization and
|
||||
must never make the gate less available than local compute alone."""
|
||||
slug: Final = origin_slug()
|
||||
if slug is None:
|
||||
return _fetch_fallback("origin remote is not a github.com URL")
|
||||
name: Final = artifact_name(base_point)
|
||||
listing: Final = gh_output(
|
||||
["api", f"repos/{slug}/actions/artifacts?name={name}&per_page=1"]
|
||||
)
|
||||
if listing is None:
|
||||
return _fetch_fallback(f"could not list CI artifacts named {name}")
|
||||
url: Final = _artifact_download_url(_parsed_json(listing))
|
||||
if url is None:
|
||||
return _fetch_fallback(f"no usable CI artifact named {name}")
|
||||
zip_bytes: Final = gh_output(["api", url])
|
||||
if zip_bytes is None:
|
||||
return _fetch_fallback(f"download failed for CI artifact {name}")
|
||||
counts: Final = counts_for_base(_counts_json_from_zip(zip_bytes), base_point)
|
||||
if counts is None:
|
||||
return _fetch_fallback(
|
||||
f"CI artifact {name} is not valid base counts for {base_point[:12]}"
|
||||
)
|
||||
sys.stderr.write(f"base counts fetched from CI artifact {name}\n")
|
||||
return counts
|
||||
|
||||
|
||||
def base_counts_cached(
|
||||
base_point: str,
|
||||
cache_dir: Path | None = None,
|
||||
compute: Callable[[str], dict[str, int]] = base_counts,
|
||||
fetch: Callable[[str], dict[str, int] | None] = fetch_ci_base_counts,
|
||||
) -> dict[str, int]:
|
||||
"""`base_counts` memoized on disk. The base tree at a given commit is
|
||||
immutable, so its counts are a pure function of the merge-base plus the
|
||||
environment fingerprints in the cache key; an empty result is never stored
|
||||
because it is the signature of a crashed pass, not a clean tree. On a disk
|
||||
miss the counts CI already published for the merge-base are fetched before
|
||||
the expensive local base pass; a fetch miss of any kind computes locally."""
|
||||
directory = default_cache_dir() if cache_dir is None else cache_dir
|
||||
path = cache_path(directory, base_point, environment_fingerprints())
|
||||
cached = load_cached_counts(path)
|
||||
if cached is not None:
|
||||
return cached
|
||||
fetched: Final = fetch(base_point)
|
||||
if fetched:
|
||||
store_counts(directory, path, base_point, fetched)
|
||||
return fetched
|
||||
counts = compute(base_point)
|
||||
if counts:
|
||||
store_counts(directory, path, base_point, counts)
|
||||
return counts
|
||||
|
||||
|
||||
def evaluate(
|
||||
head: Mapping[str, int],
|
||||
base: Mapping[str, int],
|
||||
budget: Mapping[str, Mapping[str, int]],
|
||||
) -> list[Breach]:
|
||||
breaches = []
|
||||
for code, total in head.items():
|
||||
spec = budget.get(code)
|
||||
cap = spec["limit"] if spec else DEFAULT_LIMIT
|
||||
prior = base.get(code, 0)
|
||||
if total > cap and total > prior:
|
||||
breaches.append(Breach(code, total, cap, total - prior))
|
||||
return sorted(breaches)
|
||||
|
||||
|
||||
def is_vacuous_run(
|
||||
counts: Mapping[str, int], budget: Mapping[str, Mapping[str, int]]
|
||||
) -> bool:
|
||||
"""True when nothing was parsed but the budget expects errors -- the
|
||||
signature of a type checker that produced no output. `run_basedpyright`
|
||||
already fails crash exit codes, so this guards the remaining case: a run
|
||||
that exits cleanly while emitting nothing, which would otherwise clear
|
||||
every limit and pass silently."""
|
||||
return not counts and any(spec["limit"] for spec in budget.values())
|
||||
|
||||
|
||||
def ratcheted_budget(
|
||||
budget: Mapping[str, Mapping[str, int]],
|
||||
current: Mapping[str, int],
|
||||
base: Mapping[str, int],
|
||||
) -> dict[str, dict[str, int]]:
|
||||
"""Each rule's limit lowered by the errors `current` fixed vs `base`.
|
||||
|
||||
`base` is the count at the branch point (the commit this branch diverged
|
||||
from). The drop is clamped to what was actually cleared (a rule that grew
|
||||
stays put), so the limit only ever falls. Rules absent from the budget are
|
||||
dropped: a genuinely new error category is added to the JSON deliberately,
|
||||
not on update.
|
||||
"""
|
||||
return {
|
||||
code: {
|
||||
"limit": max(0, spec["limit"] - max(0, base.get(code, 0) - current.get(code, 0)))
|
||||
}
|
||||
for code, spec in sorted(budget.items())
|
||||
}
|
||||
|
||||
|
||||
def cmd_update(current: Mapping[str, int], base_ref: str) -> None:
|
||||
"""Ratchet each rule's limit down by the errors this branch fixed.
|
||||
|
||||
`current` is the working-tree count; the reference count comes
|
||||
from a second basedpyright pass over a detached worktree at the branch point
|
||||
(the merge-base with `base_ref`), so a branch's fixes tighten its own ceilings
|
||||
by exactly what they cleared since it diverged, and limits never rise.
|
||||
"""
|
||||
budget = json.loads(BUDGET_PATH.read_text()) if BUDGET_PATH.exists() else {}
|
||||
base_point = resolve_base_point(base_ref)
|
||||
updated = ratcheted_budget(budget, current, base_counts_cached(base_point))
|
||||
BUDGET_PATH.write_text(json.dumps(updated, indent=2, sort_keys=True) + "\n")
|
||||
cleared = sum(budget[code]["limit"] - updated[code]["limit"] for code in updated)
|
||||
print(
|
||||
f"Ratcheted basedpyright limits down by {cleared} errors this branch fixed "
|
||||
f"across {len(updated)} rules"
|
||||
)
|
||||
|
||||
|
||||
def cmd_emit_counts(head: Mapping[str, int], directory: Path, head_sha: str) -> None:
|
||||
"""Write HEAD's per-rule counts as the file the publisher workflow uploads.
|
||||
|
||||
The filename stem is exactly the artifact name `fetch_ci_base_counts` will
|
||||
later look up for this commit, so emit and fetch cannot drift apart. Empty
|
||||
counts are refused for the same reason `is_vacuous_run` exists: a pass that
|
||||
produced nothing almost certainly crashed, and publishing it would poison
|
||||
every branch that fetches it."""
|
||||
if not head:
|
||||
print(
|
||||
"FAIL: basedpyright produced no errors; refusing to publish empty base "
|
||||
"counts because the pass almost certainly crashed or emitted nothing."
|
||||
)
|
||||
raise SystemExit(1)
|
||||
name: Final = artifact_name(head_sha)
|
||||
directory.mkdir(parents=True, exist_ok=True)
|
||||
(directory / f"{name}.json").write_text(counts_payload(head_sha, head))
|
||||
print(
|
||||
f"Emitted base counts for {head_sha} as {name}.json "
|
||||
f"({sum(head.values())} errors total)"
|
||||
)
|
||||
def checker_identity(dep_groups: tuple[str, ...] = TYPECHECK_DEP_GROUPS) -> Checker:
|
||||
return Checker("basedpyright", environment_fingerprints(dep_groups))
|
||||
|
||||
|
||||
def cmd_check(head: Mapping[str, int], base_ref: str) -> None:
|
||||
budget = json.loads(BUDGET_PATH.read_text())
|
||||
if is_vacuous_run(head, budget):
|
||||
expected = sum(spec["limit"] for spec in budget.values())
|
||||
if not head:
|
||||
print(
|
||||
f"FAIL: basedpyright produced no errors, but {BUDGET_PATH.name} allows "
|
||||
f"up to ~{expected}. The type checker almost certainly crashed or emitted "
|
||||
f"nothing; refusing to certify a vacuous run."
|
||||
"FAIL: basedpyright produced no errors. The type checker almost certainly "
|
||||
"crashed or emitted nothing; refusing to certify a vacuous run."
|
||||
)
|
||||
raise SystemExit(1)
|
||||
if not over_ceiling(head, budget):
|
||||
base_point: Final = resolve_base_point(base_ref)
|
||||
base: Final = base_counts_cached(checker_identity(), base_point, base_counts)
|
||||
if not base:
|
||||
print(
|
||||
f"OK: every rule is within its basedpyright limit ({sum(head.values())} errors total)"
|
||||
)
|
||||
return
|
||||
base_point = resolve_base_point(base_ref)
|
||||
base = base_counts_cached(base_point)
|
||||
if is_vacuous_run(base, budget):
|
||||
print(
|
||||
f"FAIL: basedpyright produced no errors for the base tree at "
|
||||
f"{base_point[:12]}, so every rule would look freshly added. The base "
|
||||
f"pass almost certainly crashed; refusing to blame this change for it."
|
||||
f"FAIL: basedpyright produced no errors for the base tree at {base_point[:12]}, "
|
||||
"so every rule would look freshly added. The base pass almost certainly "
|
||||
"crashed; refusing to blame this change for it."
|
||||
)
|
||||
raise SystemExit(1)
|
||||
breaches = evaluate(head, base, budget)
|
||||
judge(head, base, base_point)
|
||||
|
||||
|
||||
def judge(head: Mapping[str, int], base: Mapping[str, int], base_point: str) -> None:
|
||||
breaches: Final = evaluate(head, base, ANY_CAPS)
|
||||
if not breaches:
|
||||
print(
|
||||
f"OK: every rule is within its basedpyright limit or no higher than base ({sum(head.values())} errors total)"
|
||||
f"OK: every basedpyright rule is within its ceiling "
|
||||
f"({sum(head.values())} errors total, base {base_point[:12]})"
|
||||
)
|
||||
return
|
||||
print("FAIL: basedpyright errors exceed the per-rule limit:")
|
||||
print(f"FAIL: basedpyright errors grew past their ceiling (base {base_point[:12]}):")
|
||||
for breach in breaches:
|
||||
print(
|
||||
f" {breach.code}: total {breach.total} over limit {breach.cap} (this change added {breach.added})"
|
||||
)
|
||||
print(f" {breach.rule}: total {breach.total} over ceiling {breach.ceiling} (this change added {breach.added})")
|
||||
print(
|
||||
"Reduce the new errors or remove an equal number elsewhere; the ceiling is "
|
||||
"the limit in basedpyright-code-budget.json."
|
||||
"Reduce the new errors or remove an equal number elsewhere; the ceiling is the merge-base "
|
||||
"count, or the cap in ANY_CAPS (scripts/type_check_gate.py) when that is higher."
|
||||
)
|
||||
summary = "; ".join(f"{b.code} {b.total}/{b.cap} (+{b.added})" for b in breaches)
|
||||
summary: Final = "; ".join(f"{b.rule} {b.total}/{b.ceiling} (+{b.added})" for b in breaches)
|
||||
print(f"BREACHED RULES: {summary}")
|
||||
raise SystemExit(1)
|
||||
|
||||
|
||||
def main() -> None:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser: Final = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--base", help="Comparison ref (default: origin's current default branch)")
|
||||
parser.add_argument("--update", action="store_true")
|
||||
parser.add_argument("--emit-counts-dir", type=Path)
|
||||
args = parser.parse_args()
|
||||
parser.add_argument(
|
||||
"--emit-counts-dir",
|
||||
type=Path,
|
||||
help="Write HEAD's per-rule counts to this directory as a base-counts artifact instead of gating",
|
||||
)
|
||||
args: Final = parser.parse_args()
|
||||
from default_branch import resolve_base_ref
|
||||
from gate_slot_lock import held_slot
|
||||
|
||||
base_ref: Final = None if args.emit_counts_dir is not None else resolve_base_ref(args.base, REPO_ROOT)
|
||||
if args.emit_counts_dir is not None:
|
||||
with held_slot():
|
||||
ensure_typecheck_env()
|
||||
emit_counts(checker_identity(), count_basedpyright(run_basedpyright()), args.emit_counts_dir, head_sha())
|
||||
return
|
||||
base_ref: Final = resolve_base_ref(args.base, REPO_ROOT)
|
||||
with held_slot():
|
||||
ensure_typecheck_env()
|
||||
head = count_basedpyright(run_basedpyright())
|
||||
if args.emit_counts_dir is not None:
|
||||
cmd_emit_counts(
|
||||
head, args.emit_counts_dir, _run(["git", "rev-parse", "HEAD"]).strip()
|
||||
)
|
||||
elif base_ref is not None:
|
||||
cmd_update(head, base_ref) if args.update else cmd_check(head, base_ref)
|
||||
cmd_check(count_basedpyright(run_basedpyright()), base_ref)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
|
|
|||
|
|
@ -1,57 +1,62 @@
|
|||
#!/usr/bin/env python3
|
||||
"""Total-count gate for the LIT* rules in scripts/check_type_discipline.py.
|
||||
"""Delta-vs-base gate for the LIT* rules in scripts/check_type_discipline.py.
|
||||
|
||||
Sibling of scripts/ruff_strict_gate.py. Each rule listed in
|
||||
type-discipline-budget.json has a hard ``limit``. The gate counts each rule
|
||||
across the whole `litellm` tree and fails when a rule is both over its limit and
|
||||
higher than the base it merges into, so a change is blamed for the violations it
|
||||
adds, never for drift that already exists in the base.
|
||||
Sibling of scripts/ruff_strict_gate.py. Each rule is counted across the whole
|
||||
`litellm` tree at HEAD and at the merge-base with the branch this change merges
|
||||
into, and the gate fails only when a rule grew past the merge-base count, so a
|
||||
change is blamed for the violations it adds, never for drift that already exists
|
||||
in the base. There is no committed budget: the merge-base count is the ceiling,
|
||||
so it moves only when the base branch does.
|
||||
|
||||
Rules not present in the budget are ignored, but today every rule the checker
|
||||
emits is gated: LIT001 (mutable collection in any annotation), LIT003/LIT004
|
||||
(noqa / pyright-mypy ignore without codes or reason), LIT006 (cast), LIT008 (`**kwargs`), LIT009 (inert
|
||||
`# type: ignore`, dead syntax while enableTypeIgnoreComments is false), LIT010
|
||||
(assignment without a Final declaration; suppress deliberate rebinding with
|
||||
`# rebind-ok: <reason>`), LIT011 (parameter rebinding or in-place mutation), and
|
||||
LIT012 (TypedDict field without a `ReadOnly[...]` qualifier; suppress with
|
||||
`# writable-ok: <reason>`), and LIT014 (comprehension with more than one `for`
|
||||
or `if` clause; suppress with `# comprehension-ok: <reason>` on a spanned
|
||||
line, which belongs to the innermost violating comprehension spanning it and
|
||||
to any single-line violating comprehension on that line), and LIT015
|
||||
(pydantic model not frozen; suppress with `# frozen-ok: <reason>`) carry limits at
|
||||
or above their current count to ratchet down; LIT005 (`*-ok` suppression
|
||||
without a reason) is frozen at limit 0
|
||||
so any net-new reasonless suppression trips the gate; LIT013 (`*-ok` suppression
|
||||
that suppresses nothing) is frozen at 0 for the same reason; and LIT007
|
||||
(TypeGuard/TypeIs) is a hard zero.
|
||||
LIT010 and LIT011 were seeded at 1.5x the count left after the sweep that
|
||||
annotated every never-rebound name with Final, so that headroom is the hard
|
||||
line new code cannot cross.
|
||||
``--update`` ratchets a limit down by the violations this branch fixed relative
|
||||
to its branch point (the merge-base). A rule absent from the budget at the
|
||||
merge-base was seeded on this branch; ``--update`` leaves its limit untouched,
|
||||
because the base tree predates the rule and its whole grandfathered count would
|
||||
otherwise be misread as "fixed", collapsing the deliberate headroom to zero.
|
||||
Every rule the checker emits is gated: LIT001 (mutable collection in any
|
||||
annotation), LIT003/LIT004 (noqa / pyright-mypy ignore without codes or
|
||||
reason), LIT005 (`*-ok` suppression without a reason), LIT006 (cast), LIT007
|
||||
(TypeGuard/TypeIs), LIT008 (`**kwargs`), LIT009 (inert `# type: ignore`, dead
|
||||
syntax while enableTypeIgnoreComments is false), LIT010 (assignment without a
|
||||
Final declaration; suppress deliberate rebinding with `# rebind-ok: <reason>`),
|
||||
LIT011 (parameter rebinding or in-place mutation), LIT012 (TypedDict field
|
||||
without a `ReadOnly[...]` qualifier; suppress with `# writable-ok: <reason>`),
|
||||
LIT013 (`*-ok` suppression that suppresses nothing), LIT014 (comprehension
|
||||
with more than one `for` or `if` clause; suppress with
|
||||
`# comprehension-ok: <reason>` on a spanned line, which belongs to the innermost
|
||||
violating comprehension spanning it and to any single-line violating
|
||||
comprehension on that line), and LIT015 (pydantic model not frozen; suppress
|
||||
with `# frozen-ok: <reason>`).
|
||||
|
||||
The merge-base counts come from scripts/lint_base_counts.py: the disk cache,
|
||||
then the CI artifact published for that commit, then a pass of the current
|
||||
checker over a detached worktree at the merge-base, so a rule change on this
|
||||
branch is measured on both sides. ``--emit-counts-dir`` writes HEAD's counts
|
||||
as the file that artifact is built from.
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import json
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
from collections import Counter
|
||||
from collections.abc import Mapping, Sequence
|
||||
from pathlib import Path
|
||||
from typing import Final, NamedTuple
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
CHECKER = REPO_ROOT / "scripts" / "check_type_discipline.py"
|
||||
BUDGET_PATH = REPO_ROOT / "type-discipline-budget.json"
|
||||
TARGET = "litellm"
|
||||
from lint_base_counts import (
|
||||
Checker,
|
||||
base_counts_cached,
|
||||
emit_counts,
|
||||
evaluate,
|
||||
head_sha,
|
||||
resolve_base_point,
|
||||
sha256_of,
|
||||
)
|
||||
|
||||
_HUNK = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@")
|
||||
_LINE = re.compile(r"^(?P<file>.+?):(?P<line>\d+): (?P<code>LIT\d+) ")
|
||||
REPO_ROOT: Final = Path(__file__).resolve().parent.parent
|
||||
CHECKER: Final = REPO_ROOT / "scripts" / "check_type_discipline.py"
|
||||
TARGET: Final = "litellm"
|
||||
|
||||
_HUNK: Final = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@")
|
||||
_LINE: Final = re.compile(r"^(?P<file>.+?):(?P<line>\d+): (?P<code>LIT\d+) ")
|
||||
|
||||
|
||||
class Violation(NamedTuple):
|
||||
|
|
@ -60,41 +65,19 @@ class Violation(NamedTuple):
|
|||
code: str
|
||||
|
||||
|
||||
class Breach(NamedTuple):
|
||||
rule: str
|
||||
total: int
|
||||
cap: int
|
||||
added: int
|
||||
|
||||
|
||||
def _run(cmd: list, cwd: Path = REPO_ROOT) -> str:
|
||||
proc = subprocess.run(cmd, cwd=cwd, capture_output=True, text=True)
|
||||
def _run(cmd: Sequence[str], cwd: Path = REPO_ROOT) -> str:
|
||||
proc: Final = subprocess.run(cmd, cwd=cwd, capture_output=True, text=True)
|
||||
if proc.returncode not in (0, 1):
|
||||
sys.stderr.write(proc.stderr)
|
||||
raise SystemExit(f"{cmd[0]} exited {proc.returncode}")
|
||||
return proc.stdout
|
||||
|
||||
|
||||
def resolve_base_point(base_ref: str, cwd: Path = REPO_ROOT) -> str:
|
||||
"""The snapshot commit base counts are measured at: merge-base(base_ref, HEAD),
|
||||
made aware of an in-progress merge. Mid-merge, HEAD is still the pre-merge tip,
|
||||
so its merge-base is the old branch point and every violation the base gained
|
||||
since then would be blamed on this change. While MERGE_HEAD exists, prefer
|
||||
merge-base(base_ref, MERGE_HEAD) whenever it is the newer of the two."""
|
||||
head_point: Final = _run(["git", "merge-base", base_ref, "HEAD"], cwd=cwd).strip()
|
||||
if not head_point:
|
||||
return base_ref
|
||||
merge_head: Final = _run(["git", "rev-parse", "--verify", "--quiet", "MERGE_HEAD"], cwd=cwd).strip()
|
||||
if not merge_head:
|
||||
return head_point
|
||||
merge_point: Final = _run(["git", "merge-base", base_ref, merge_head], cwd=cwd).strip()
|
||||
if not merge_point:
|
||||
return head_point
|
||||
older: Final = _run(["git", "merge-base", head_point, merge_point], cwd=cwd).strip()
|
||||
return merge_point if older == head_point else head_point
|
||||
def checker_identity(checker: Path = CHECKER) -> Checker:
|
||||
return Checker("type-discipline", (sha256_of(checker),))
|
||||
|
||||
|
||||
def _check(root: Path, checker: Path) -> list:
|
||||
def _check(root: Path, checker: Path) -> list[Violation]:
|
||||
# Resolve root first: on macOS tempfile dirs (/var/...) resolve to /private/var/...,
|
||||
# and the checker prints already-resolved absolute paths, so relative_to would fail.
|
||||
root = root.resolve()
|
||||
|
|
@ -111,22 +94,22 @@ def _check(root: Path, checker: Path) -> list:
|
|||
return found
|
||||
|
||||
|
||||
def head_violations() -> list:
|
||||
def head_violations() -> list[Violation]:
|
||||
return _check(REPO_ROOT, CHECKER)
|
||||
|
||||
|
||||
def count_by_rule(violations: list) -> dict:
|
||||
def count_by_rule(violations: Sequence[Violation]) -> dict[str, int]:
|
||||
return dict(Counter(v.code for v in violations))
|
||||
|
||||
|
||||
def base_counts(ref: str) -> dict:
|
||||
parent = Path(tempfile.mkdtemp(prefix="lit_base_"))
|
||||
worktree = parent / "wt"
|
||||
def base_counts(ref: str) -> dict[str, int]:
|
||||
parent: Final = Path(tempfile.mkdtemp(prefix="lit_base_"))
|
||||
worktree: Final = parent / "wt"
|
||||
try:
|
||||
_run(["git", "worktree", "add", "--detach", str(worktree), ref])
|
||||
# Measure the base with the *current* rule logic, not whatever shipped at base.
|
||||
(worktree / "scripts").mkdir(parents=True, exist_ok=True)
|
||||
checker = worktree / "scripts" / "check_type_discipline.py"
|
||||
checker: Final = worktree / "scripts" / "check_type_discipline.py"
|
||||
shutil.copy(CHECKER, checker)
|
||||
return count_by_rule(_check(worktree, checker))
|
||||
finally:
|
||||
|
|
@ -141,27 +124,8 @@ def base_counts(ref: str) -> dict:
|
|||
shutil.rmtree(parent, ignore_errors=True)
|
||||
|
||||
|
||||
def over_ceiling(head: dict, budget: dict) -> frozenset:
|
||||
"""Rules whose head count already exceeds their limit.
|
||||
|
||||
A rule can only breach when it is over its limit, so when none are the base
|
||||
comparison cannot change the verdict and the base worktree scan can be skipped.
|
||||
"""
|
||||
return frozenset(rule for rule, spec in budget.items() if head.get(rule, 0) > spec["limit"])
|
||||
|
||||
|
||||
def evaluate(head: dict, base: dict, budget: dict) -> list:
|
||||
breaches = []
|
||||
for rule, spec in budget.items():
|
||||
cap = spec["limit"]
|
||||
total = head.get(rule, 0)
|
||||
if total > cap and total > base.get(rule, 0):
|
||||
breaches.append(Breach(rule, total, cap, total - base.get(rule, 0)))
|
||||
return sorted(breaches)
|
||||
|
||||
|
||||
def parse_changed_lines(diff_text: str) -> dict:
|
||||
changed: dict = {}
|
||||
def parse_changed_lines(diff_text: str) -> dict[str, set[int]]:
|
||||
changed: dict[str, set[int]] = {}
|
||||
path = None
|
||||
for line in diff_text.splitlines():
|
||||
if line.startswith("+++ b/"):
|
||||
|
|
@ -173,104 +137,54 @@ def parse_changed_lines(diff_text: str) -> dict:
|
|||
return changed
|
||||
|
||||
|
||||
def introduced(violations: list, changed: dict) -> list:
|
||||
def introduced(violations: Sequence[Violation], changed: Mapping[str, set[int]]) -> list[Violation]:
|
||||
return [v for v in violations if v.line in changed.get(v.file, set())]
|
||||
|
||||
|
||||
def cmd_check(base: str) -> None:
|
||||
budget = json.loads(BUDGET_PATH.read_text())
|
||||
head = head_violations()
|
||||
head_counts = count_by_rule(head)
|
||||
if not over_ceiling(head_counts, budget):
|
||||
print(f"OK: every LIT rule is within its codebase ceiling (base {base})")
|
||||
return
|
||||
base_point = resolve_base_point(base)
|
||||
breaches = evaluate(head_counts, base_counts(base_point), budget)
|
||||
head: Final = head_violations()
|
||||
base_point: Final = resolve_base_point(base)
|
||||
breaches: Final = evaluate(count_by_rule(head), base_counts_cached(checker_identity(), base_point, base_counts))
|
||||
if not breaches:
|
||||
print(f"OK: every LIT rule is within its codebase ceiling (base {base})")
|
||||
print(f"OK: no LIT rule grew past its merge-base count (base {base})")
|
||||
return
|
||||
new = introduced(
|
||||
head,
|
||||
parse_changed_lines(_run(["git", "diff", base_point, "--unified=0", "--no-color", "--", TARGET])),
|
||||
)
|
||||
print(f"FAIL: LIT-rule totals exceed their limit (base {base}):")
|
||||
diff: Final = _run(["git", "diff", base_point, "--unified=0", "--no-color", "--", TARGET])
|
||||
new: Final = introduced(head, parse_changed_lines(diff))
|
||||
print(f"FAIL: LIT-rule totals grew past their merge-base count (base {base}):")
|
||||
for breach in breaches:
|
||||
print(f" {breach.rule}: total {breach.total} over limit {breach.cap} (this change added {breach.added})")
|
||||
print(f" {breach.rule}: total {breach.total} over ceiling {breach.ceiling} (this change added {breach.added})")
|
||||
for violation in sorted(v for v in new if v.code == breach.rule):
|
||||
print(f" {violation.file}:{violation.line}")
|
||||
print(
|
||||
"Remove the new violations, give each a reason (`# noqa: XXX # <reason>`, "
|
||||
"`# pyright: ignore[rule] # <reason>`, `# mutable-ok: <reason>`, "
|
||||
"`# cast-ok: <reason>`, `# guard-ok: <reason>`, `# kwargs-ok: <reason>`, "
|
||||
"`# rebind-ok: <reason>`, `# writable-ok: <reason>`, "
|
||||
"`# comprehension-ok: <reason>`, `# frozen-ok: <reason>`), or remove an equal "
|
||||
"number elsewhere; the ceiling "
|
||||
"is the limit in type-discipline-budget.json."
|
||||
"`# pyright: ignore[rule] # <reason>`, `# mutable-ok: <reason>`, `# cast-ok: <reason>`, "
|
||||
"`# guard-ok: <reason>`, `# kwargs-ok: <reason>`, `# rebind-ok: <reason>`, "
|
||||
"`# writable-ok: <reason>`, `# comprehension-ok: <reason>`, `# frozen-ok: <reason>`), "
|
||||
"or remove an equal number "
|
||||
"elsewhere; the ceiling is the merge-base count."
|
||||
)
|
||||
raise SystemExit(1)
|
||||
|
||||
|
||||
def ratcheted_budget(budget: dict, current: dict, base: dict, seeded: frozenset = frozenset()) -> dict:
|
||||
"""Each rule's limit lowered by the violations `current` fixed vs `base`.
|
||||
|
||||
`base` is the count at the branch point (the commit this branch diverged
|
||||
from). The drop is clamped to what was actually cleared (a rule that grew
|
||||
stays put), so the limit only ever falls. Rules in `seeded` were introduced
|
||||
on this branch with deliberate grandfathered headroom; their limits pass
|
||||
through untouched, since the base predates the rule and comparing against it
|
||||
would misread the entire grandfathered count as fixed.
|
||||
"""
|
||||
return {
|
||||
rule: {
|
||||
"limit": spec["limit"]
|
||||
if rule in seeded
|
||||
else max(0, spec["limit"] - max(0, base.get(rule, 0) - current.get(rule, 0)))
|
||||
}
|
||||
for rule, spec in sorted(budget.items())
|
||||
}
|
||||
|
||||
|
||||
def _base_budget_rules(base_point: str) -> frozenset:
|
||||
proc = subprocess.run(
|
||||
["git", "show", f"{base_point}:{BUDGET_PATH.name}"],
|
||||
cwd=REPO_ROOT,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
if proc.returncode != 0:
|
||||
return frozenset()
|
||||
return frozenset(json.loads(proc.stdout))
|
||||
|
||||
|
||||
def cmd_update(base_ref: str) -> None:
|
||||
"""Ratchet each rule's limit down by the violations this branch fixed.
|
||||
|
||||
The working-tree count is compared against a checker pass over a detached
|
||||
worktree at the branch point (the merge-base with `base_ref`), so a branch's
|
||||
fixes tighten its own ceilings by exactly what they cleared since it diverged.
|
||||
"""
|
||||
budget = json.loads(BUDGET_PATH.read_text())
|
||||
base_point = resolve_base_point(base_ref)
|
||||
seeded = frozenset(budget) - _base_budget_rules(base_point)
|
||||
updated = ratcheted_budget(budget, count_by_rule(head_violations()), base_counts(base_point), seeded)
|
||||
BUDGET_PATH.write_text(json.dumps(updated, indent=2, sort_keys=True) + "\n")
|
||||
cleared = sum(budget[rule]["limit"] - updated[rule]["limit"] for rule in updated)
|
||||
print(f"Ratcheted LIT-rule limits down by {cleared} violations this branch fixed")
|
||||
if seeded:
|
||||
print("Left untouched (seeded on this branch, absent from the base budget): " + ", ".join(sorted(seeded)))
|
||||
|
||||
|
||||
def main() -> None:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser: Final = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--base", help="Comparison ref (default: origin's current default branch)")
|
||||
parser.add_argument("--update", action="store_true")
|
||||
args = parser.parse_args()
|
||||
parser.add_argument(
|
||||
"--emit-counts-dir",
|
||||
type=Path,
|
||||
help="Write HEAD's per-rule counts to this directory as a base-counts artifact instead of gating",
|
||||
)
|
||||
args: Final = parser.parse_args()
|
||||
from default_branch import resolve_base_ref
|
||||
from gate_slot_lock import held_slot
|
||||
|
||||
if args.emit_counts_dir is not None:
|
||||
with held_slot():
|
||||
emit_counts(checker_identity(), count_by_rule(head_violations()), args.emit_counts_dir, head_sha())
|
||||
return
|
||||
base_ref: Final = resolve_base_ref(args.base, REPO_ROOT)
|
||||
with held_slot():
|
||||
cmd_update(base_ref) if args.update else cmd_check(base_ref)
|
||||
cmd_check(base_ref)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
|
|
|||
|
|
@ -1,26 +0,0 @@
|
|||
{
|
||||
"TQ001": {
|
||||
"limit": 733
|
||||
},
|
||||
"TQ002": {
|
||||
"limit": 737
|
||||
},
|
||||
"TQ003": {
|
||||
"limit": 62
|
||||
},
|
||||
"TQ004": {
|
||||
"limit": 469
|
||||
},
|
||||
"TQ005": {
|
||||
"limit": 2399
|
||||
},
|
||||
"TQ006": {
|
||||
"limit": 34
|
||||
},
|
||||
"TQ007": {
|
||||
"limit": 117
|
||||
},
|
||||
"TQ009": {
|
||||
"limit": 59
|
||||
}
|
||||
}
|
||||
|
|
@ -1,189 +0,0 @@
|
|||
"""Tests for scripts/budget_ratchet_check.py.
|
||||
|
||||
The guard's contract is "limits may only fall": a raised limit, a dropped rule, or
|
||||
a deleted file is a regression, while a lowered/equal limit, a brand-new rule, or a
|
||||
brand-new budget file is fine. Each branch is pinned here.
|
||||
"""
|
||||
|
||||
import importlib.util
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from typing import Final
|
||||
|
||||
_MODULE_PATH = (
|
||||
Path(__file__).resolve().parents[2] / "scripts" / "budget_ratchet_check.py"
|
||||
)
|
||||
_spec = importlib.util.spec_from_file_location("budget_ratchet_check", _MODULE_PATH)
|
||||
ratchet = importlib.util.module_from_spec(_spec)
|
||||
_spec.loader.exec_module(ratchet)
|
||||
|
||||
|
||||
def _spec_of(limit):
|
||||
return {"limit": limit}
|
||||
|
||||
|
||||
def test_limits_read_the_limit_and_skip_malformed():
|
||||
limits = ratchet._limits({"LIT006": _spec_of(1023), "junk": 5})
|
||||
assert limits == {"LIT006": 1023} # malformed (non-dict) spec ignored
|
||||
|
||||
|
||||
def test_limits_fall_back_to_legacy_baseline_plus_slack():
|
||||
# The base side of a diff can predate the `limit` migration; its ceiling is
|
||||
# baseline + slack, read on the same footing as a new-schema `limit`.
|
||||
assert ratchet._limits({"LIT006": {"baseline": 1013, "slack": 10}}) == {"LIT006": 1023}
|
||||
|
||||
|
||||
def test_migration_from_legacy_schema_to_equal_limit_is_clean():
|
||||
# baseline+slack (1023) -> limit 1023 is the same ceiling, so no regression.
|
||||
base = {"LIT006": {"baseline": 1013, "slack": 10}}
|
||||
assert ratchet.regressions_for("b.json", base, {"LIT006": _spec_of(1023)}) == []
|
||||
# ...and a genuine raise across the migration is still caught.
|
||||
regs = ratchet.regressions_for("b.json", base, {"LIT006": _spec_of(1024)})
|
||||
assert [r.rule for r in regs] == ["LIT006"] and "1023 -> 1024" in regs[0].detail
|
||||
|
||||
|
||||
def test_raised_limit_is_a_regression():
|
||||
base = {"LIT006": _spec_of(1023)}
|
||||
head = {"LIT006": _spec_of(1024)}
|
||||
regs = ratchet.regressions_for("b.json", base, head)
|
||||
assert [r.rule for r in regs] == ["LIT006"]
|
||||
assert "1023 -> 1024" in regs[0].detail
|
||||
|
||||
|
||||
def test_lowered_or_equal_limit_is_clean():
|
||||
base = {"LIT006": _spec_of(1023)}
|
||||
# limit drops
|
||||
assert ratchet.regressions_for("b.json", base, {"LIT006": _spec_of(1000)}) == []
|
||||
# nothing changes
|
||||
assert ratchet.regressions_for("b.json", base, {"LIT006": _spec_of(1023)}) == []
|
||||
|
||||
|
||||
def test_dropped_rule_is_a_regression():
|
||||
regs = ratchet.regressions_for("b.json", {"LIT007": _spec_of(0)}, {})
|
||||
assert [r.rule for r in regs] == ["LIT007"]
|
||||
assert "dropped" in regs[0].detail
|
||||
|
||||
|
||||
def test_new_rule_in_head_is_clean():
|
||||
assert ratchet.regressions_for("b.json", {}, {"new-rule": _spec_of(5)}) == []
|
||||
|
||||
|
||||
def test_dropped_rule_that_graduated_to_a_hard_failing_config_is_clean():
|
||||
base = {"UP006": _spec_of(0)}
|
||||
assert ratchet.regressions_for("b.json", base, {}, graduated=("UP006",)) == []
|
||||
|
||||
|
||||
def test_graduation_matches_by_prefix_like_ruff_selectors_do():
|
||||
base = {"ANN202": _spec_of(865)}
|
||||
assert ratchet.regressions_for("b.json", base, {}, graduated=("ANN",)) == []
|
||||
|
||||
|
||||
def test_an_unrelated_graduation_does_not_excuse_a_dropped_rule():
|
||||
base = {"C901": _spec_of(3)}
|
||||
regs = ratchet.regressions_for("b.json", base, {}, graduated=("UP006", "SIM118"))
|
||||
assert [r.rule for r in regs] == ["C901"]
|
||||
assert "dropped" in regs[0].detail
|
||||
|
||||
|
||||
def test_graduation_never_excuses_a_raised_limit():
|
||||
base = {"UP006": _spec_of(0)}
|
||||
regs = ratchet.regressions_for("b.json", base, {"UP006": _spec_of(7)}, graduated=("UP006",))
|
||||
assert [r.rule for r in regs] == ["UP006"]
|
||||
assert "0 -> 7" in regs[0].detail
|
||||
|
||||
|
||||
def test_dropped_rule_the_checker_retired_is_clean():
|
||||
base: Final = {"TQ008": _spec_of(10993)}
|
||||
assert ratchet.regressions_for("b.json", base, {}, retired=frozenset({"TQ008"})) == []
|
||||
|
||||
|
||||
def test_dropped_rule_the_checker_still_emits_is_a_regression():
|
||||
base: Final = {"TQ001": _spec_of(5), "TQ008": _spec_of(10993)}
|
||||
regs: Final = ratchet.regressions_for("b.json", base, {}, retired=frozenset({"TQ008"}))
|
||||
assert [r.rule for r in regs] == ["TQ001"]
|
||||
assert "dropped" in regs[0].detail
|
||||
|
||||
|
||||
def test_retirement_never_excuses_a_raised_limit():
|
||||
base: Final = {"TQ008": _spec_of(0)}
|
||||
regs: Final = ratchet.regressions_for("b.json", base, {"TQ008": _spec_of(7)}, retired=frozenset({"TQ008"}))
|
||||
assert [r.rule for r in regs] == ["TQ008"]
|
||||
assert "0 -> 7" in regs[0].detail
|
||||
|
||||
|
||||
def test_retired_rules_come_from_the_paired_checker():
|
||||
base: Final = {"TQ001": _spec_of(5), "TQ008": _spec_of(10993)}
|
||||
assert ratchet.retired_rules("test-quality-budget.json", base) == frozenset({"TQ008"})
|
||||
|
||||
|
||||
def test_budgets_without_a_paired_checker_never_retire():
|
||||
base: Final = {"TQ008": _spec_of(1)}
|
||||
for rel in ("ruff-strict-budget.json", "type-discipline-budget.json", "basedpyright-code-budget.json"):
|
||||
assert ratchet.retired_rules(rel, base) == frozenset()
|
||||
|
||||
|
||||
def test_graduated_selectors_come_from_the_paired_ruff_config():
|
||||
selectors = ratchet.graduated_selectors("ruff-strict-budget.json")
|
||||
assert "UP006" in selectors
|
||||
assert "ANN" not in selectors
|
||||
|
||||
|
||||
def test_budgets_without_a_paired_config_can_never_graduate():
|
||||
assert ratchet.graduated_selectors("type-discipline-budget.json") == ()
|
||||
assert ratchet.graduated_selectors("basedpyright-code-budget.json") == ()
|
||||
|
||||
|
||||
def test_a_selector_the_config_also_ignores_does_not_count_as_graduated():
|
||||
lint = {"ignore": ["UP006"], "extend-select": ["UP006", "SIM118"]}
|
||||
assert ratchet.selectors_hard_failed_by(lint) == ("SIM118",)
|
||||
|
||||
|
||||
def test_selectors_hard_failed_by_reads_a_config_with_no_ignore_list():
|
||||
assert ratchet.selectors_hard_failed_by({"extend-select": ["UP006"]}) == ("UP006",)
|
||||
|
||||
|
||||
def test_deleted_budget_file_is_a_regression():
|
||||
regs = ratchet.regressions_for("b.json", {"LIT006": _spec_of(1)}, None)
|
||||
assert [r.rule for r in regs] == ["*"]
|
||||
assert "deleted" in regs[0].detail
|
||||
|
||||
|
||||
def test_new_budget_file_has_nothing_to_ratchet():
|
||||
assert ratchet.regressions_for("b.json", None, {"LIT006": _spec_of(1)}) == []
|
||||
|
||||
|
||||
def test_default_budgets_watch_every_budget_file_in_the_repo():
|
||||
# This job is the repo's only ceiling-raise alarm, so every *-budget.json on disk must be
|
||||
# watched; a budget left out of DEFAULT_BUDGETS (e.g. basedpyright-code-budget.json) can be
|
||||
# loosened with no signal. Equality also catches a phantom entry that no longer exists.
|
||||
repo_root = _MODULE_PATH.parents[1]
|
||||
on_disk = frozenset(p.name for p in repo_root.glob("*budget*.json"))
|
||||
assert on_disk == frozenset(ratchet.DEFAULT_BUDGETS)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Base-ref resolution: a bad ref must fail loudly, never pass vacuously
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
||||
|
||||
def test_ref_is_commit_distinguishes_real_from_bogus():
|
||||
assert ratchet._ref_is_commit("HEAD") is True
|
||||
assert ratchet._ref_is_commit("definitely-not-a-real-ref-zzz") is False
|
||||
|
||||
|
||||
def test_load_base_reads_a_present_file_and_none_for_an_absent_one():
|
||||
# A real budget file exists at HEAD; a made-up path is absent at the same (valid) ref.
|
||||
assert ratchet._load_base("type-discipline-budget.json", "HEAD") is not None
|
||||
assert ratchet._load_base("scripts/no-such-budget-xyz.json", "HEAD") is None
|
||||
|
||||
|
||||
def test_unresolvable_base_ref_exits_nonzero_instead_of_skipping():
|
||||
proc = subprocess.run(
|
||||
[sys.executable, str(_MODULE_PATH), "--base", "definitely-not-a-real-ref-zzz"],
|
||||
cwd=_MODULE_PATH.parents[1],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
assert proc.returncode == 1
|
||||
assert "does not resolve to a commit" in proc.stderr
|
||||
|
|
@ -7,9 +7,7 @@ a test fail. The comment-scanner cases are the regression for the readline path:
|
|||
"""
|
||||
|
||||
import importlib.util
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
|
@ -1003,20 +1001,6 @@ def test_violation_message_names_the_clause_counts(tmp_path: Path):
|
|||
assert "2 `for` clauses and 1 `if` clause" in messages[0]
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Budget integrity: every emittable LIT rule (bar the LIT000 read/parse error) is gated
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
||||
|
||||
def test_budget_covers_exactly_the_checker_rules():
|
||||
budget = json.loads((_REPO_ROOT / "type-discipline-budget.json").read_text())
|
||||
emitted = set(re.findall(r"LIT\d{3}", _MODULE_PATH.read_text(encoding="utf-8"))) - {"LIT000"}
|
||||
assert set(budget) == emitted
|
||||
for spec in budget.values():
|
||||
assert isinstance(spec["limit"], int)
|
||||
assert spec["limit"] >= 0
|
||||
|
||||
|
||||
_FANS_OUT = checker._worker_count(checker.PARALLEL_MIN_PATHS) > 1
|
||||
_SERIAL_ONLY = "one usable core, so scan_paths stays serial and there is no fan-out to compare"
|
||||
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ def remote_and_clone(tmp_path: Path) -> tuple[Path, Path]:
|
|||
(seed / "scripts").mkdir()
|
||||
for name in (
|
||||
"default_branch.py",
|
||||
"budget_ratchet_check.py",
|
||||
"lint_base_counts.py",
|
||||
"ruff_strict_gate.py",
|
||||
"type_discipline_gate.py",
|
||||
"test_quality_gate.py",
|
||||
|
|
@ -39,11 +39,9 @@ def remote_and_clone(tmp_path: Path) -> tuple[Path, Path]:
|
|||
shutil.copyfile(ROOT / "Makefile", seed / "Makefile")
|
||||
(seed / "litellm").mkdir()
|
||||
(seed / "litellm" / "example.py").write_text("value = 0\n")
|
||||
(seed / "ruff-strict-budget.json").write_text('{"C901": {"limit": 1}}\n')
|
||||
_commit(seed, "staging base")
|
||||
_git(seed, "checkout", "-qb", "main")
|
||||
(seed / "litellm" / "example.py").write_text("value = 1\n")
|
||||
(seed / "ruff-strict-budget.json").write_text('{"C901": {"limit": 0}}\n')
|
||||
_commit(seed, "main base")
|
||||
remote: Final = tmp_path / "remote.git"
|
||||
_git(tmp_path, "clone", "-q", "--bare", str(seed), str(remote))
|
||||
|
|
@ -121,28 +119,6 @@ def test_explicit_base_works_without_remote_access(
|
|||
assert "No changed litellm Python files" in checked.stdout
|
||||
|
||||
|
||||
def test_budget_ratchet_compares_against_new_default(remote_and_clone: tuple[Path, Path]) -> None:
|
||||
remote, repo = remote_and_clone
|
||||
_git(remote, "symbolic-ref", "HEAD", "refs/heads/main")
|
||||
resolved: Final = _resolve(repo)
|
||||
assert resolved.returncode == 0, resolved.stderr
|
||||
_git(repo, "checkout", "-qb", "litellm_feature", "origin/main")
|
||||
(repo / "ruff-strict-budget.json").write_text('{"C901": {"limit": 1}}\n')
|
||||
command: Final = [sys.executable, "scripts/budget_ratchet_check.py"]
|
||||
checked: Final = subprocess.run(command, cwd=repo, capture_output=True, text=True, check=False)
|
||||
assert checked.returncode == 1
|
||||
assert "limit raised 0 -> 1" in checked.stdout
|
||||
assert "base origin/main" in checked.stdout
|
||||
overridden: Final = subprocess.run(
|
||||
[*command, "--base", "origin/release_branch"],
|
||||
cwd=repo,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
assert overridden.returncode == 0, overridden.stdout + overridden.stderr
|
||||
|
||||
|
||||
def _freshness(repo: Path, *args: str) -> subprocess.CompletedProcess[str]:
|
||||
return subprocess.run(
|
||||
[
|
||||
|
|
@ -192,7 +168,6 @@ def test_migration_freshness_refuses_unavailable_remote(remote_and_clone: tuple[
|
|||
@pytest.mark.parametrize(
|
||||
"gate",
|
||||
[
|
||||
"budget_ratchet_check",
|
||||
"ruff_strict_gate",
|
||||
"type_discipline_gate",
|
||||
"test_quality_gate",
|
||||
|
|
@ -213,14 +188,11 @@ def test_each_gate_refuses_an_unverifiable_default(remote_and_clone: tuple[Path,
|
|||
assert "Cannot verify the base branch against origin" in result.stderr
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"target", ["lint-format-check-changed", "lint-test-quality", "lint-test-quality-budget-update"]
|
||||
)
|
||||
@pytest.mark.parametrize("target", ["lint-format-check-changed", "lint-test-quality"])
|
||||
def test_direct_make_target_fetches_default_once(remote_and_clone: tuple[Path, Path], target: str) -> None:
|
||||
_, repo = remote_and_clone
|
||||
trace: Final = repo.parent / "git-trace.jsonl"
|
||||
shutil.copyfile(ROOT / "scripts" / "check_test_quality.py", repo / "scripts" / "check_test_quality.py")
|
||||
shutil.copyfile(ROOT / "test-quality-budget.json", repo / "test-quality-budget.json")
|
||||
(repo / "tests").mkdir()
|
||||
result: Final = subprocess.run(
|
||||
["make", "-o", "install-dev", target, "LINT_DEP_INSTALL=", "UV_RUN=env"],
|
||||
|
|
|
|||
454
tests/unit/test_lint_base_counts.py
Normal file
454
tests/unit/test_lint_base_counts.py
Normal file
|
|
@ -0,0 +1,454 @@
|
|||
"""Tests for scripts/lint_base_counts.py, the merge-base counting shared by the
|
||||
four lint gates: the ceiling rule with its optional per-rule caps, the on-disk cache and
|
||||
its eviction, the CI artifact fetch, the artifact emit, and the merge-base
|
||||
resolution."""
|
||||
|
||||
import fnmatch
|
||||
import io
|
||||
import json
|
||||
import os
|
||||
import zipfile
|
||||
from collections.abc import Callable, Mapping, Sequence
|
||||
from pathlib import Path
|
||||
from typing import Final, NamedTuple, NoReturn
|
||||
|
||||
import pytest
|
||||
|
||||
import lint_base_counts as counts
|
||||
|
||||
_CHECKER: Final = counts.Checker("basedpyright", ("f1", "f2"))
|
||||
_OTHER_CHECKER: Final = counts.Checker("ruff-strict", ("f1", "f2"))
|
||||
|
||||
|
||||
def test_evaluate_passes_a_rule_that_did_not_grow() -> None:
|
||||
assert counts.evaluate({"LIT006": 12}, {"LIT006": 12}) == ()
|
||||
|
||||
|
||||
def test_evaluate_blames_one_new_violation_of_an_uncapped_rule() -> None:
|
||||
assert counts.evaluate({"LIT006": 13}, {"LIT006": 12}) == (counts.Breach("LIT006", 13, 12, 1),)
|
||||
|
||||
|
||||
def test_evaluate_lets_a_capped_rule_grow_up_to_its_cap_and_no_further() -> None:
|
||||
caps: Final = {"reportAny": 110}
|
||||
assert counts.evaluate({"reportAny": 110}, {"reportAny": 100}, caps) == ()
|
||||
assert counts.evaluate({"reportAny": 111}, {"reportAny": 100}, caps) == (counts.Breach("reportAny", 111, 110, 11),)
|
||||
|
||||
|
||||
def test_evaluate_never_blames_a_bystander_for_a_base_already_over_the_cap() -> None:
|
||||
caps: Final = {"reportAny": 110}
|
||||
assert counts.evaluate({"reportAny": 120}, {"reportAny": 120}, caps) == ()
|
||||
assert counts.evaluate({"reportAny": 119}, {"reportAny": 120}, caps) == ()
|
||||
|
||||
|
||||
def test_evaluate_holds_a_base_over_its_cap_to_no_growth() -> None:
|
||||
caps: Final = {"reportAny": 110}
|
||||
assert counts.evaluate({"reportAny": 121}, {"reportAny": 120}, caps) == (counts.Breach("reportAny", 121, 120, 1),)
|
||||
|
||||
|
||||
def test_evaluate_cap_applies_only_to_the_rule_it_names() -> None:
|
||||
caps: Final = {"reportAny": 110}
|
||||
assert counts.evaluate({"LIT006": 13}, {"LIT006": 12}, caps) == (counts.Breach("LIT006", 13, 12, 1),)
|
||||
|
||||
|
||||
def test_evaluate_counts_a_rule_absent_from_the_base_as_zero() -> None:
|
||||
assert counts.evaluate({"NEW99": 1}, {}) == (counts.Breach("NEW99", 1, 0, 1),)
|
||||
|
||||
|
||||
def test_evaluate_never_blames_a_change_that_reduced_a_rule() -> None:
|
||||
assert counts.evaluate({"LIT006": 11}, {"LIT006": 12}) == ()
|
||||
|
||||
|
||||
def test_evaluate_reports_every_grown_rule_sorted_by_name() -> None:
|
||||
head: Final = {"TQ008": 3, "TQ001": 2, "TQ003": 5}
|
||||
base: Final = {"TQ008": 2, "TQ001": 1, "TQ003": 5}
|
||||
assert [b.rule for b in counts.evaluate(head, base)] == ["TQ001", "TQ008"]
|
||||
|
||||
|
||||
def test_evaluate_ignores_a_base_rule_the_head_fixed_entirely() -> None:
|
||||
assert counts.evaluate({}, {"LIT006": 12}) == ()
|
||||
|
||||
|
||||
def test_cache_key_changes_with_base_point_and_each_fingerprint() -> None:
|
||||
key: Final = counts.cache_key("abc", ("cfg", "lock"))
|
||||
assert counts.cache_key("abc", ("cfg", "lock")) == key
|
||||
assert counts.cache_key("def", ("cfg", "lock")) != key
|
||||
assert counts.cache_key("abc", ("cfg2", "lock")) != key
|
||||
assert counts.cache_key("abc", ("cfg", "lock2")) != key
|
||||
|
||||
|
||||
def test_checker_names_its_artifact_and_cache_file_by_the_same_key() -> None:
|
||||
key: Final = counts.cache_key("abc123", ("f1", "f2"))
|
||||
assert _CHECKER.artifact_name("abc123") == f"basedpyright-counts-{key}"
|
||||
assert _CHECKER.cache_file_name("abc123") == f"basedpyright-base-{key}.json"
|
||||
assert fnmatch.fnmatch(_CHECKER.cache_file_name("abc123"), _CHECKER.cache_glob())
|
||||
|
||||
|
||||
def test_checkers_with_the_same_fingerprints_never_share_a_name() -> None:
|
||||
assert _CHECKER.artifact_name("abc123") != _OTHER_CHECKER.artifact_name("abc123")
|
||||
assert not fnmatch.fnmatch(_OTHER_CHECKER.cache_file_name("abc123"), _CHECKER.cache_glob())
|
||||
|
||||
|
||||
def test_cached_counts_round_trip(tmp_path: Path) -> None:
|
||||
path: Final = counts.store_counts(tmp_path, _CHECKER, "abc123", {"reportAny": 3, "reportCall": 1})
|
||||
assert path == tmp_path / _CHECKER.cache_file_name("abc123")
|
||||
assert counts.load_cached_counts(path) == {"reportAny": 3, "reportCall": 1}
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"content",
|
||||
[
|
||||
None,
|
||||
"{not json",
|
||||
json.dumps(["counts"]),
|
||||
json.dumps({"base_point": "abc"}),
|
||||
json.dumps({"counts": {"reportAny": "three"}}),
|
||||
json.dumps({"counts": {"reportAny": True}}),
|
||||
],
|
||||
)
|
||||
def test_missing_corrupt_or_misshapen_cache_reads_as_none(tmp_path: Path, content: str | None) -> None:
|
||||
path: Final = tmp_path / "cache.json"
|
||||
if content is not None:
|
||||
path.write_text(content)
|
||||
assert counts.load_cached_counts(path) is None
|
||||
|
||||
|
||||
def test_scratch_is_invisible_to_the_prune_glob() -> None:
|
||||
scratch: Final = counts.scratch_path(Path("/c") / _CHECKER.cache_file_name("abc"))
|
||||
assert not fnmatch.fnmatch(scratch.name, _CHECKER.cache_glob())
|
||||
|
||||
|
||||
def test_store_prune_spares_a_concurrent_runs_in_flight_scratch(tmp_path: Path) -> None:
|
||||
foreign: Final = counts.scratch_path(tmp_path / _CHECKER.cache_file_name("other"))
|
||||
foreign.write_text("{}")
|
||||
mine: Final = counts.store_counts(tmp_path, _CHECKER, "mine", {"reportAny": 1})
|
||||
assert foreign.exists()
|
||||
assert counts.load_cached_counts(mine) == {"reportAny": 1}
|
||||
|
||||
|
||||
def test_store_keeps_a_concurrent_worktrees_entry_for_another_branch_point(tmp_path: Path) -> None:
|
||||
old: Final = counts.store_counts(tmp_path, _CHECKER, "old", {"reportAny": 1})
|
||||
new: Final = counts.store_counts(tmp_path, _CHECKER, "new", {"reportAny": 2})
|
||||
assert counts.load_cached_counts(old) == {"reportAny": 1}
|
||||
assert counts.load_cached_counts(new) == {"reportAny": 2}
|
||||
|
||||
|
||||
def test_store_evicts_only_the_oldest_entries_beyond_the_cap(tmp_path: Path) -> None:
|
||||
aged: Final = tuple(
|
||||
counts.store_counts(tmp_path, _CHECKER, f"base{age}", {"reportAny": age})
|
||||
for age in range(counts.CACHE_KEEP_ENTRIES)
|
||||
)
|
||||
for age, path in enumerate(aged):
|
||||
os.utime(path, (age, age))
|
||||
newest: Final = counts.store_counts(tmp_path, _CHECKER, "newest", {"reportAny": 99})
|
||||
assert not aged[0].exists()
|
||||
assert all(path.exists() for path in aged[1:])
|
||||
assert counts.load_cached_counts(newest) == {"reportAny": 99}
|
||||
|
||||
|
||||
def test_store_never_evicts_the_entry_it_just_wrote_even_on_mtime_ties(tmp_path: Path) -> None:
|
||||
for index in range(counts.CACHE_KEEP_ENTRIES + 2):
|
||||
os.utime(counts.store_counts(tmp_path, _CHECKER, f"base{index}", {"reportAny": 1}), (9_999_999_999,) * 2)
|
||||
mine: Final = counts.store_counts(tmp_path, _CHECKER, "mine", {"reportAny": 2})
|
||||
assert counts.load_cached_counts(mine) == {"reportAny": 2}
|
||||
assert len(list(tmp_path.glob(_CHECKER.cache_glob()))) == counts.CACHE_KEEP_ENTRIES
|
||||
|
||||
|
||||
def test_store_eviction_never_touches_another_checkers_entries(tmp_path: Path) -> None:
|
||||
other: Final = counts.store_counts(tmp_path, _OTHER_CHECKER, "base", {"E501": 1})
|
||||
os.utime(other, (1, 1))
|
||||
for index in range(counts.CACHE_KEEP_ENTRIES + 1):
|
||||
counts.store_counts(tmp_path, _CHECKER, f"base{index}", {"reportAny": 1})
|
||||
assert counts.load_cached_counts(other) == {"E501": 1}
|
||||
|
||||
|
||||
def _no_fetch(checker: counts.Checker, base_point: str) -> None:
|
||||
return None
|
||||
|
||||
|
||||
def _never(reason: str) -> Callable[..., NoReturn]:
|
||||
def callback(*args: object) -> NoReturn:
|
||||
raise AssertionError(reason)
|
||||
|
||||
return callback
|
||||
|
||||
|
||||
def test_base_counts_cached_returns_the_hit_without_recomputing(tmp_path: Path) -> None:
|
||||
counts.store_counts(tmp_path, _CHECKER, "abc123", {"reportAny": 7})
|
||||
assert counts.base_counts_cached(
|
||||
_CHECKER,
|
||||
"abc123",
|
||||
_never("a cache hit must not re-run the base pass"),
|
||||
cache_dir=tmp_path,
|
||||
fetch=_never("a cache hit must not reach for CI"),
|
||||
) == {"reportAny": 7}
|
||||
|
||||
|
||||
def test_base_counts_cached_computes_once_then_hits(tmp_path: Path) -> None:
|
||||
calls: Final[list[str]] = []
|
||||
|
||||
def fake(ref: str) -> counts.Counts:
|
||||
calls.append(ref)
|
||||
return {"reportAny": 4}
|
||||
|
||||
first: Final = counts.base_counts_cached(_CHECKER, "abc123", fake, cache_dir=tmp_path, fetch=_no_fetch)
|
||||
second: Final = counts.base_counts_cached(_CHECKER, "abc123", fake, cache_dir=tmp_path, fetch=_no_fetch)
|
||||
assert first == second == {"reportAny": 4}
|
||||
assert calls == ["abc123"]
|
||||
|
||||
|
||||
def test_base_counts_cached_keeps_each_checker_apart(tmp_path: Path) -> None:
|
||||
counts.store_counts(tmp_path, _OTHER_CHECKER, "abc123", {"E501": 7})
|
||||
assert counts.base_counts_cached(
|
||||
_CHECKER, "abc123", lambda ref: {"reportAny": 4}, cache_dir=tmp_path, fetch=_no_fetch
|
||||
) == {"reportAny": 4}
|
||||
|
||||
|
||||
def test_an_empty_base_pass_is_never_cached(tmp_path: Path) -> None:
|
||||
calls: Final[list[str]] = []
|
||||
|
||||
def crashed(ref: str) -> counts.Counts:
|
||||
calls.append(ref)
|
||||
return {}
|
||||
|
||||
assert counts.base_counts_cached(_CHECKER, "abc123", crashed, cache_dir=tmp_path, fetch=_no_fetch) == {}
|
||||
assert counts.base_counts_cached(_CHECKER, "abc123", crashed, cache_dir=tmp_path, fetch=_no_fetch) == {}
|
||||
assert calls == ["abc123", "abc123"]
|
||||
assert list(tmp_path.iterdir()) == []
|
||||
|
||||
|
||||
def test_base_counts_cached_uses_fetched_counts_and_persists_them(tmp_path: Path) -> None:
|
||||
fetched: Final = counts.base_counts_cached(
|
||||
_CHECKER,
|
||||
"abc123",
|
||||
_never("fetched counts must skip the local base pass"),
|
||||
cache_dir=tmp_path,
|
||||
fetch=lambda checker, base_point: {"reportAny": 9},
|
||||
)
|
||||
assert fetched == {"reportAny": 9}
|
||||
assert counts.load_cached_counts(tmp_path / _CHECKER.cache_file_name("abc123")) == {"reportAny": 9}
|
||||
assert counts.base_counts_cached(
|
||||
_CHECKER,
|
||||
"abc123",
|
||||
_never("the persisted fetch must satisfy later runs"),
|
||||
cache_dir=tmp_path,
|
||||
fetch=_never("the persisted fetch must satisfy later runs"),
|
||||
) == {"reportAny": 9}
|
||||
|
||||
|
||||
def test_base_counts_cached_hands_the_fetcher_the_checker_and_base_point(tmp_path: Path) -> None:
|
||||
seen: Final[list[tuple[counts.Checker, str]]] = []
|
||||
|
||||
def fetch(checker: counts.Checker, base_point: str) -> None:
|
||||
seen.append((checker, base_point))
|
||||
|
||||
counts.base_counts_cached(_CHECKER, "abc123", lambda ref: {"reportAny": 4}, cache_dir=tmp_path, fetch=fetch)
|
||||
assert seen == [(_CHECKER, "abc123")]
|
||||
|
||||
|
||||
def test_base_counts_cached_falls_back_to_compute_on_a_fetch_miss(tmp_path: Path) -> None:
|
||||
calls: Final[list[str]] = []
|
||||
|
||||
def local(ref: str) -> counts.Counts:
|
||||
calls.append(ref)
|
||||
return {"reportAny": 4}
|
||||
|
||||
assert counts.base_counts_cached(_CHECKER, "abc123", local, cache_dir=tmp_path, fetch=_no_fetch) == {
|
||||
"reportAny": 4
|
||||
}
|
||||
assert calls == ["abc123"]
|
||||
|
||||
|
||||
def test_base_counts_cached_treats_empty_fetched_counts_as_a_miss(tmp_path: Path) -> None:
|
||||
assert counts.base_counts_cached(
|
||||
_CHECKER,
|
||||
"abc123",
|
||||
lambda ref: {"reportAny": 2},
|
||||
cache_dir=tmp_path,
|
||||
fetch=lambda checker, base_point: {},
|
||||
) == {"reportAny": 2}
|
||||
assert counts.load_cached_counts(tmp_path / _CHECKER.cache_file_name("abc123")) == {"reportAny": 2}
|
||||
|
||||
|
||||
def test_origin_slug_parsing_supports_ssh_and_https_github_forms() -> None:
|
||||
assert counts.parse_origin_slug("git@github.com:BerriAI/litellm.git") == "BerriAI/litellm"
|
||||
assert counts.parse_origin_slug("git@github.com:BerriAI/litellm") == "BerriAI/litellm"
|
||||
assert counts.parse_origin_slug("https://github.com/BerriAI/litellm.git") == "BerriAI/litellm"
|
||||
assert counts.parse_origin_slug("https://github.com/BerriAI/litellm") == "BerriAI/litellm"
|
||||
assert counts.parse_origin_slug("https://github.com/BerriAI/litellm/") == "BerriAI/litellm"
|
||||
|
||||
|
||||
def test_origin_slug_parsing_rejects_non_github_urls() -> None:
|
||||
assert counts.parse_origin_slug("https://gitlab.com/BerriAI/litellm.git") is None
|
||||
assert counts.parse_origin_slug("git@bitbucket.org:BerriAI/litellm.git") is None
|
||||
assert counts.parse_origin_slug("not a url") is None
|
||||
assert counts.parse_origin_slug("") is None
|
||||
|
||||
|
||||
def _artifact_zip(payload: Mapping[str, object]) -> bytes:
|
||||
buffer: Final = io.BytesIO()
|
||||
with zipfile.ZipFile(buffer, "w") as archive:
|
||||
archive.writestr("counts.json", json.dumps(payload))
|
||||
return buffer.getvalue()
|
||||
|
||||
|
||||
def _gh_stub(
|
||||
listing: Mapping[str, object], zip_bytes: bytes, seen: list[tuple[str, ...]] | None = None
|
||||
) -> counts.GhOutput:
|
||||
def gh_output(args: Sequence[str]) -> bytes:
|
||||
if seen is not None:
|
||||
seen.append(tuple(args))
|
||||
if args[-1].startswith("repos/"):
|
||||
return json.dumps(listing).encode()
|
||||
return zip_bytes
|
||||
|
||||
return gh_output
|
||||
|
||||
|
||||
def _live_listing() -> Mapping[str, object]:
|
||||
return {"artifacts": [{"expired": False, "archive_download_url": "https://api.github.com/x/zip"}]}
|
||||
|
||||
|
||||
def test_fetcher_returns_counts_from_a_matching_artifact(capsys: pytest.CaptureFixture[str]) -> None:
|
||||
payload: Final = {"base_point": "abc123", "counts": {"reportAny": 3}}
|
||||
fetched: Final = counts.fetch_ci_base_counts(
|
||||
_CHECKER, "abc123", gh=_gh_stub(_live_listing(), _artifact_zip(payload))
|
||||
)
|
||||
assert fetched == {"reportAny": 3}
|
||||
assert "fetched from CI artifact" in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_fetcher_asks_for_the_artifact_named_by_the_checker_and_base_point() -> None:
|
||||
seen: Final[list[tuple[str, ...]]] = []
|
||||
payload: Final = {"base_point": "abc123", "counts": {"reportAny": 3}}
|
||||
counts.fetch_ci_base_counts(_CHECKER, "abc123", gh=_gh_stub(_live_listing(), _artifact_zip(payload), seen))
|
||||
listing_request: Final = seen[0][-1]
|
||||
assert f"name={_CHECKER.artifact_name('abc123')}" in listing_request
|
||||
assert seen[1][-1] == "https://api.github.com/x/zip"
|
||||
|
||||
|
||||
def test_fetcher_rejects_an_artifact_for_a_different_base_point() -> None:
|
||||
payload: Final = {"base_point": "someothersha", "counts": {"reportAny": 3}}
|
||||
assert (
|
||||
counts.fetch_ci_base_counts(_CHECKER, "abc123", gh=_gh_stub(_live_listing(), _artifact_zip(payload))) is None
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("bad_counts", [{}, {"reportAny": "three"}, {"reportAny": True}])
|
||||
def test_fetcher_rejects_empty_or_misshapen_artifact_counts(bad_counts: Mapping[str, object]) -> None:
|
||||
payload: Final = {"base_point": "abc123", "counts": bad_counts}
|
||||
assert (
|
||||
counts.fetch_ci_base_counts(_CHECKER, "abc123", gh=_gh_stub(_live_listing(), _artifact_zip(payload))) is None
|
||||
)
|
||||
|
||||
|
||||
def test_fetcher_rejects_an_expired_artifact() -> None:
|
||||
listing: Final = {"artifacts": [{"expired": True, "archive_download_url": "https://api.github.com/x/zip"}]}
|
||||
payload: Final = {"base_point": "abc123", "counts": {"reportAny": 3}}
|
||||
assert counts.fetch_ci_base_counts(_CHECKER, "abc123", gh=_gh_stub(listing, _artifact_zip(payload))) is None
|
||||
|
||||
|
||||
def test_fetcher_misses_when_no_artifact_is_published() -> None:
|
||||
assert counts.fetch_ci_base_counts(_CHECKER, "abc123", gh=_gh_stub({"artifacts": []}, b"")) is None
|
||||
|
||||
|
||||
def test_fetcher_misses_when_gh_is_unusable(capsys: pytest.CaptureFixture[str]) -> None:
|
||||
assert counts.fetch_ci_base_counts(_CHECKER, "abc123", gh=lambda args: None) is None
|
||||
assert "computing base counts locally" in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_fetcher_misses_on_a_corrupt_artifact_archive() -> None:
|
||||
assert counts.fetch_ci_base_counts(_CHECKER, "abc123", gh=_gh_stub(_live_listing(), b"not a zip")) is None
|
||||
|
||||
|
||||
def test_emit_writes_the_artifact_json_named_by_the_head_key(
|
||||
tmp_path: Path, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
counts.emit_counts(_CHECKER, {"reportAny": 3, "aRule": 1}, tmp_path, "deadbeef")
|
||||
name: Final = _CHECKER.artifact_name("deadbeef")
|
||||
assert json.loads((tmp_path / f"{name}.json").read_text()) == {
|
||||
"base_point": "deadbeef",
|
||||
"counts": {"aRule": 1, "reportAny": 3},
|
||||
}
|
||||
summary: Final = capsys.readouterr().out
|
||||
assert "deadbeef" in summary
|
||||
assert name in summary
|
||||
assert "4" in summary
|
||||
|
||||
|
||||
def test_emit_refuses_to_publish_empty_counts(tmp_path: Path) -> None:
|
||||
with pytest.raises(SystemExit):
|
||||
counts.emit_counts(_CHECKER, {}, tmp_path, "deadbeef")
|
||||
assert list(tmp_path.iterdir()) == []
|
||||
|
||||
|
||||
def test_emitted_file_is_the_one_the_fetcher_looks_up(tmp_path: Path) -> None:
|
||||
written: Final = counts.emit_counts(_CHECKER, {"reportAny": 3}, tmp_path, "deadbeef")
|
||||
payload: Final = json.loads(written.read_text())
|
||||
assert counts.counts_for_base(payload, "deadbeef") == {"reportAny": 3}
|
||||
assert counts.counts_for_base(payload, "someothersha") is None
|
||||
listing_zip: Final = _artifact_zip(payload)
|
||||
assert counts.fetch_ci_base_counts(_CHECKER, "deadbeef", gh=_gh_stub(_live_listing(), listing_zip)) == {
|
||||
"reportAny": 3
|
||||
}
|
||||
|
||||
|
||||
class _History(NamedTuple):
|
||||
parents: Mapping[str, tuple[str, ...]]
|
||||
refs: Mapping[str, str]
|
||||
|
||||
def ancestry(self, commit: str) -> frozenset[str]:
|
||||
return frozenset((commit,)).union(*(self.ancestry(parent) for parent in self.parents[commit]))
|
||||
|
||||
def merge_base(self, left: str, right: str) -> str:
|
||||
common: Final = self.ancestry(self.refs.get(left, left)) & self.ancestry(self.refs.get(right, right))
|
||||
return next(c for c in common if not any(c != other and c in self.ancestry(other) for other in common))
|
||||
|
||||
|
||||
def _git_over(history: _History) -> counts.Git:
|
||||
def git(args: Sequence[str]) -> str:
|
||||
match tuple(args):
|
||||
case ("merge-base", left, right):
|
||||
return f"{history.merge_base(left, right)}\n"
|
||||
case ("rev-parse", "HEAD"):
|
||||
return f"{history.refs['HEAD']}\n"
|
||||
case ("rev-parse", "--verify", "--quiet", ref):
|
||||
return f"{history.refs[ref]}\n" if ref in history.refs else ""
|
||||
case ("rev-parse", "--path-format=absolute", "--git-common-dir"):
|
||||
return "/repo/.git\n"
|
||||
case ("rev-parse", "--path-format=absolute", "--git-dir"):
|
||||
return "/repo/.git/worktrees/feature\n"
|
||||
case _:
|
||||
raise AssertionError(f"unexpected git call: {args}")
|
||||
|
||||
return git
|
||||
|
||||
|
||||
_FEATURE_OFF_MAIN: Final = _History(
|
||||
parents={"shared": (), "feature": ("shared",), "drift": ("shared",)},
|
||||
refs={"main": "drift", "HEAD": "feature"},
|
||||
)
|
||||
|
||||
|
||||
def test_base_point_is_the_branch_point_when_no_merge_is_in_progress() -> None:
|
||||
assert counts.resolve_base_point("main", _git_over(_FEATURE_OFF_MAIN)) == "shared"
|
||||
|
||||
|
||||
def test_base_point_mid_merge_advances_to_the_merged_in_base_tip() -> None:
|
||||
merging_main: Final = _FEATURE_OFF_MAIN._replace(refs={**_FEATURE_OFF_MAIN.refs, "MERGE_HEAD": "drift"})
|
||||
assert counts.resolve_base_point("main", _git_over(merging_main)) == "drift"
|
||||
|
||||
|
||||
def test_base_point_mid_merge_of_an_older_side_branch_keeps_the_newer_branch_point() -> None:
|
||||
merging_old_side: Final = _History(
|
||||
parents={"shared": (), "old": ("shared",), "drift": ("shared",), "feature": ("drift",)},
|
||||
refs={"main": "drift", "HEAD": "feature", "MERGE_HEAD": "old"},
|
||||
)
|
||||
assert counts.resolve_base_point("main", _git_over(merging_old_side)) == "drift"
|
||||
|
||||
|
||||
def test_head_sha_is_the_checked_out_commit() -> None:
|
||||
assert counts.head_sha(_git_over(_FEATURE_OFF_MAIN)) == "feature"
|
||||
|
||||
|
||||
def test_default_cache_dir_is_shared_by_every_worktree() -> None:
|
||||
assert counts.default_cache_dir(_git_over(_FEATURE_OFF_MAIN)) == Path("/repo/.git") / counts.CACHE_DIR_NAME
|
||||
|
|
@ -14,9 +14,9 @@ from tests._process_helpers import process_is_gone
|
|||
ROOT = Path(__file__).resolve().parents[2]
|
||||
SCRIPT = ROOT / "scripts" / "pre_commit_lint.sh"
|
||||
WHOLE_TREE_RUFF = "run --no-sync ruff check --config ruff-tests.toml tests"
|
||||
TEST_TREE_RAN = "ran: test-tree lint (ruff-tests.toml + test-quality budget)"
|
||||
TEST_TREE_RAN = "ran: test-tree lint (ruff-tests.toml + test-quality gate)"
|
||||
TEST_TREE_SKIPPED = (
|
||||
"skipped: test-tree lint (ruff-tests.toml + test-quality budget) "
|
||||
"skipped: test-tree lint (ruff-tests.toml + test-quality gate) "
|
||||
"(no tests/ Python files or test-tree lint inputs in scope)"
|
||||
)
|
||||
|
||||
|
|
@ -470,7 +470,6 @@ def test_tests_only_change_runs_the_whole_test_tree_ruff_and_the_quality_gate(tm
|
|||
"changed",
|
||||
[
|
||||
"ruff-tests.toml",
|
||||
"test-quality-budget.json",
|
||||
"scripts/check_test_quality.py",
|
||||
"scripts/test_quality_gate.py",
|
||||
"tests/e2e/test_x.py",
|
||||
|
|
@ -523,7 +522,7 @@ def test_a_failing_quality_gate_fails_a_tests_only_run(tmp_path: Path) -> None:
|
|||
_stage_file(repo, "tests/test_a.py", "def test_a() -> None: ...\n")
|
||||
proc = _run(repo, bin_dir, {"STUB_FAIL": "test-quality"})
|
||||
assert proc.returncode == 1
|
||||
assert "Test-quality budget failed" in proc.stdout + proc.stderr
|
||||
assert "Test-quality gate failed" in proc.stdout + proc.stderr
|
||||
assert "check: FAIL" in proc.stdout
|
||||
|
||||
|
||||
|
|
@ -566,7 +565,7 @@ def test_partial_staging_warns_when_test_files_are_left_unstaged(tmp_path: Path)
|
|||
(repo / "tests" / "test_a.py").write_text("def test_a() -> None:\n assert True\n")
|
||||
proc = _run(repo, bin_dir, {"STUB_ARGS_DIR": str(args_dir)})
|
||||
assert proc.returncode == 0, proc.stdout + proc.stderr
|
||||
assert "SKIPPED test-tree lint (ruff-tests.toml + test-quality budget)" in proc.stdout
|
||||
assert "SKIPPED test-tree lint (ruff-tests.toml + test-quality gate)" in proc.stdout
|
||||
assert "tests/test_a.py" in proc.stdout
|
||||
assert _recorded(args_dir, "ruff_tests.args") == []
|
||||
assert _recorded(args_dir, "make.args") == []
|
||||
|
|
|
|||
|
|
@ -1,87 +1,27 @@
|
|||
import importlib.util
|
||||
import json
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from typing import Final
|
||||
|
||||
import pytest
|
||||
|
||||
import ruff_strict_gate as gate
|
||||
|
||||
if sys.version_info >= (3, 11):
|
||||
import tomllib
|
||||
else:
|
||||
import tomli as tomllib
|
||||
|
||||
_REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
_MODULE_PATH = _REPO_ROOT / "scripts" / "ruff_strict_gate.py"
|
||||
_spec = importlib.util.spec_from_file_location("ruff_strict_gate", _MODULE_PATH)
|
||||
gate = importlib.util.module_from_spec(_spec)
|
||||
_spec.loader.exec_module(gate)
|
||||
|
||||
Violation = gate.Violation
|
||||
|
||||
_ENABLED_BY_RUFF_DEFAULTS = frozenset({"F401"})
|
||||
|
||||
|
||||
def rule(name, limit):
|
||||
return {name: {"limit": limit}}
|
||||
|
||||
|
||||
def test_under_ceiling_passes():
|
||||
assert gate.evaluate({"ANN001": 100}, {"ANN001": 100}, rule("ANN001", 110)) == []
|
||||
|
||||
|
||||
def test_ceiling_is_the_limit_boundary():
|
||||
budget = rule("ANN001", 110)
|
||||
at = gate.evaluate({"ANN001": 110}, {"ANN001": 90}, budget)
|
||||
over = gate.evaluate({"ANN001": 111}, {"ANN001": 90}, budget)
|
||||
assert at == []
|
||||
assert [b.rule for b in over] == ["ANN001"]
|
||||
assert over[0].cap == 110
|
||||
assert over[0].added == 21
|
||||
|
||||
|
||||
def test_over_ceiling_and_change_added_fails():
|
||||
breaches = gate.evaluate({"C901": 11}, {"C901": 9}, rule("C901", 10))
|
||||
assert [b.rule for b in breaches] == ["C901"]
|
||||
assert breaches[0].added == 2
|
||||
|
||||
|
||||
def test_base_already_over_ceiling_change_added_nothing_is_not_blamed():
|
||||
# drift safety: base is over limit, this change leaves the count where it is
|
||||
assert gate.evaluate({"C901": 15}, {"C901": 15}, rule("C901", 10)) == []
|
||||
|
||||
|
||||
def test_change_that_reduces_an_over_ceiling_rule_is_not_blamed():
|
||||
# still over limit, but moving the right direction
|
||||
assert gate.evaluate({"C901": 14}, {"C901": 16}, rule("C901", 10)) == []
|
||||
|
||||
|
||||
def test_rules_are_independent():
|
||||
budget = {**rule("ANN001", 150), **rule("C901", 10)}
|
||||
breaches = gate.evaluate(
|
||||
{"ANN001": 130, "C901": 11}, {"ANN001": 100, "C901": 10}, budget
|
||||
)
|
||||
assert [b.rule for b in breaches] == ["C901"] # ANN001 130 <= 150, C901 11 > 10
|
||||
|
||||
|
||||
def test_missing_rule_counts_as_zero():
|
||||
assert gate.evaluate({}, {}, rule("C901", 0)) == []
|
||||
|
||||
|
||||
def test_update_ratchets_limit_down_by_what_the_branch_fixed_never_up():
|
||||
budget = {**rule("ANN001", 150), **rule("C901", 10)}
|
||||
# ANN001 fixed 20 (100 -> 80) so its limit falls 150 -> 130; C901 grew, so its
|
||||
# limit holds flat at 10 (a fix must never loosen a ceiling).
|
||||
current = {"ANN001": 80, "C901": 12}
|
||||
base = {"ANN001": 100, "C901": 9}
|
||||
assert gate.ratcheted_budget(budget, current, base) == {
|
||||
"ANN001": {"limit": 130},
|
||||
"C901": {"limit": 10},
|
||||
}
|
||||
|
||||
|
||||
def test_parse_changed_lines_maps_added_lines_per_file():
|
||||
diff = (
|
||||
"+++ b/litellm/a.py\n"
|
||||
|
|
@ -109,62 +49,6 @@ def test_parse_changed_lines_handles_single_and_ranged_hunks(hunk):
|
|||
assert gate.parse_changed_lines(f"+++ b/litellm/a.py\n{hunk}\n")["litellm/a.py"]
|
||||
|
||||
|
||||
def test_over_ceiling_flags_only_counts_above_the_limit():
|
||||
budget = rule("C901", 10)
|
||||
assert gate.over_ceiling({"C901": 10}, budget) == frozenset()
|
||||
assert gate.over_ceiling({"C901": 11}, budget) == frozenset({"C901"})
|
||||
assert gate.over_ceiling({}, budget) == frozenset()
|
||||
|
||||
|
||||
def test_over_ceiling_ignores_rules_missing_from_the_budget():
|
||||
assert gate.over_ceiling({"NEW99": 100}, rule("C901", 10)) == frozenset()
|
||||
|
||||
|
||||
def test_over_ceiling_is_independent_across_rules():
|
||||
budget = {**rule("ANN001", 150), **rule("C901", 10)}
|
||||
assert gate.over_ceiling({"ANN001": 130, "C901": 11}, budget) == frozenset({"C901"})
|
||||
|
||||
|
||||
def _git(cwd, *args):
|
||||
proc = subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True)
|
||||
assert proc.returncode == 0, proc.stderr
|
||||
return proc.stdout.strip()
|
||||
|
||||
|
||||
def _commit(cwd, name):
|
||||
(cwd / name).write_text(name)
|
||||
_git(cwd, "add", "-A")
|
||||
_git(cwd, "commit", "-q", "-m", name)
|
||||
return _git(cwd, "rev-parse", "HEAD")
|
||||
|
||||
|
||||
def _branched_repo(tmp_path):
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
_git(repo, "init", "-q", "-b", "main")
|
||||
_git(repo, "config", "user.email", "gate@example.com")
|
||||
_git(repo, "config", "user.name", "gate")
|
||||
_git(repo, "config", "commit.gpgsign", "false")
|
||||
branch_point = _commit(repo, "shared.txt")
|
||||
_git(repo, "checkout", "-q", "-b", "feature")
|
||||
_commit(repo, "feature.txt")
|
||||
_git(repo, "checkout", "-q", "main")
|
||||
base_tip = _commit(repo, "drift.txt")
|
||||
_git(repo, "checkout", "-q", "feature")
|
||||
return repo, branch_point, base_tip
|
||||
|
||||
|
||||
def test_base_point_is_the_branch_point_when_no_merge_is_in_progress(tmp_path):
|
||||
repo, branch_point, _ = _branched_repo(tmp_path)
|
||||
assert gate.resolve_base_point("main", cwd=repo) == branch_point
|
||||
|
||||
|
||||
def test_base_point_mid_merge_advances_to_the_merged_in_base_tip(tmp_path):
|
||||
repo, _, base_tip = _branched_repo(tmp_path)
|
||||
_git(repo, "merge", "--no-commit", "--no-ff", "main")
|
||||
assert gate.resolve_base_point("main", cwd=repo) == base_tip
|
||||
|
||||
|
||||
def _lint_section(config_name: str) -> dict:
|
||||
return tomllib.loads((_REPO_ROOT / config_name).read_text())["lint"]
|
||||
|
||||
|
|
@ -189,10 +73,6 @@ def _selected_by_the_normal_config() -> frozenset:
|
|||
return frozenset(_lint_section("ruff.toml")["extend-select"]) | _ENABLED_BY_RUFF_DEFAULTS
|
||||
|
||||
|
||||
def _budgeted_rules() -> frozenset:
|
||||
return frozenset(json.loads((_REPO_ROOT / "ruff-strict-budget.json").read_text()))
|
||||
|
||||
|
||||
def _ruff_binary() -> str | None:
|
||||
beside_interpreter = Path(sys.executable).with_name("ruff")
|
||||
return str(beside_interpreter) if beside_interpreter.exists() else shutil.which("ruff")
|
||||
|
|
@ -302,37 +182,6 @@ def test_every_base_owned_rule_is_external_or_selected_in_the_strict_config(all_
|
|||
)
|
||||
|
||||
|
||||
def test_every_budgeted_rule_is_one_the_gate_actually_measures():
|
||||
selectors = tuple(_lint_section("ruff-strict.toml")["select"])
|
||||
unmeasured = frozenset(code for code in _budgeted_rules() if not code.startswith(selectors))
|
||||
assert unmeasured == frozenset(), (
|
||||
f"the gate never counts {sorted(unmeasured)}, so their ceilings are dead config that reads "
|
||||
"as coverage. Either select them in ruff-strict.toml or drop them from the budget."
|
||||
)
|
||||
|
||||
|
||||
@_needs_ruff
|
||||
def test_every_strict_selected_rule_is_budgeted_or_hard_failed_by_the_base_config(all_ruff_rule_codes):
|
||||
strict_enabled = frozenset(
|
||||
code
|
||||
for code in all_ruff_rule_codes
|
||||
if code.startswith(tuple(_lint_section("ruff-strict.toml")["select"]))
|
||||
)
|
||||
base_hard_failed = tuple(_lint_section("ruff.toml")["extend-select"])
|
||||
unpoliced = frozenset(
|
||||
code
|
||||
for code in strict_enabled
|
||||
if code not in _budgeted_rules()
|
||||
and not code.startswith(base_hard_failed)
|
||||
and code not in _ENABLED_BY_RUFF_DEFAULTS
|
||||
)
|
||||
assert unpoliced == frozenset(), (
|
||||
f"nothing enforces {sorted(unpoliced)}: the gate skips rules missing from the budget, and "
|
||||
"the base config does not hard-fail them. Re-add a budget ceiling or graduate them into "
|
||||
"ruff.toml's lint.extend-select."
|
||||
)
|
||||
|
||||
|
||||
@_needs_ruff
|
||||
def test_a_noqa_for_a_strict_gate_rule_survives_the_normal_ruff_run():
|
||||
assert "RUF100" not in _ruff_output_for_noqa("ANN202")
|
||||
|
|
@ -373,3 +222,21 @@ def test_a_graduated_rule_can_still_be_suppressed_without_tripping_unused_noqa()
|
|||
output = _ruff_output_for_source(suppressed)
|
||||
assert "UP006" not in output
|
||||
assert "RUF100" not in output
|
||||
|
||||
|
||||
def test_editing_either_ruff_config_or_upgrading_ruff_rekeys_the_base_counts(tmp_path: Path) -> None:
|
||||
strict: Final = tmp_path / "ruff-strict.toml"
|
||||
base: Final = tmp_path / "ruff.toml"
|
||||
strict.write_text("strict v1\n")
|
||||
base.write_text("base v1\n")
|
||||
|
||||
def artifact(version: str) -> str:
|
||||
return gate.checker_identity(strict, base, lambda: version).artifact_name("abc123")
|
||||
|
||||
before: Final = artifact("ruff 0.1.0")
|
||||
assert artifact("ruff 0.1.0") == before
|
||||
assert artifact("ruff 0.2.0") != before
|
||||
strict.write_text("strict v2\n")
|
||||
after_strict_edit: Final = artifact("ruff 0.1.0")
|
||||
base.write_text("base v2\n")
|
||||
assert len({before, after_strict_edit, artifact("ruff 0.1.0")}) == 3
|
||||
|
|
|
|||
|
|
@ -1,12 +1,11 @@
|
|||
"""Tests for scripts/test_quality_gate.py.
|
||||
|
||||
The gate's whole value is that it blames a change only for what it adds and that a
|
||||
limit can never rise. Both live in pure functions, so they are tested directly:
|
||||
`evaluate` for the blame rule, `ratcheted_budget` for the one-way ratchet, and
|
||||
`parse_changed_lines` for the diff scan that turns a breach into file:line.
|
||||
The blame rule itself lives in scripts/lint_base_counts.py and is tested there;
|
||||
what is tested here is `parse_changed_lines`, the diff scan that turns a breach into
|
||||
file:line. The base scan spawns a worktree and a checker subprocess, so its cleanup
|
||||
on termination is driven end to end.
|
||||
"""
|
||||
|
||||
import importlib.util
|
||||
import os
|
||||
import signal
|
||||
import subprocess
|
||||
|
|
@ -15,77 +14,28 @@ import time
|
|||
from collections.abc import Callable
|
||||
from contextlib import suppress
|
||||
from pathlib import Path
|
||||
from typing import NamedTuple
|
||||
from typing import Final, NamedTuple
|
||||
|
||||
import test_quality_gate as gate
|
||||
|
||||
_REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
_MODULE_PATH = _REPO_ROOT / "scripts" / "test_quality_gate.py"
|
||||
_spec = importlib.util.spec_from_file_location("test_quality_gate", _MODULE_PATH)
|
||||
gate = importlib.util.module_from_spec(_spec)
|
||||
# @dataclass(slots=True) rebuilds its class through sys.modules[__module__], so the
|
||||
# module has to be registered before exec_module runs or Scope fails to construct.
|
||||
sys.modules[_spec.name] = gate
|
||||
_spec.loader.exec_module(gate)
|
||||
|
||||
_BUDGET = {"TQ001": {"limit": 10}, "TQ003": {"limit": 5}}
|
||||
|
||||
_SCAN_BASE = (
|
||||
"import importlib.util, pathlib, sys\n"
|
||||
"spec = importlib.util.spec_from_file_location('test_quality_gate', sys.argv[1])\n"
|
||||
"gate = importlib.util.module_from_spec(spec)\n"
|
||||
"sys.modules[spec.name] = gate\n"
|
||||
"spec.loader.exec_module(gate)\n"
|
||||
"gate.base_counts('HEAD', repo_root=pathlib.Path(sys.argv[2]), checker=pathlib.Path(sys.argv[3]))\n"
|
||||
"import pathlib, sys\n"
|
||||
"import test_quality_gate as gate\n"
|
||||
"gate.base_counts('HEAD', repo_root=pathlib.Path(sys.argv[1]), checker=pathlib.Path(sys.argv[2]))\n"
|
||||
)
|
||||
_SCAN_BASE_WITH_SIGHUP_IGNORED = "import signal\nsignal.signal(signal.SIGHUP, signal.SIG_IGN)\n" + _SCAN_BASE
|
||||
|
||||
|
||||
def test_a_rule_within_its_limit_is_not_a_breach():
|
||||
assert gate.evaluate({"TQ001": 10}, {"TQ001": 10}, _BUDGET) == ()
|
||||
|
||||
|
||||
def test_a_rule_over_its_limit_that_the_change_added_is_a_breach():
|
||||
breaches = gate.evaluate({"TQ001": 12}, {"TQ001": 10}, _BUDGET)
|
||||
assert [(b.rule, b.total, b.cap, b.added) for b in breaches] == [("TQ001", 12, 10, 2)]
|
||||
|
||||
|
||||
def test_drift_already_in_the_base_is_not_blamed_on_the_change():
|
||||
assert gate.evaluate({"TQ001": 14}, {"TQ001": 14}, _BUDGET) == ()
|
||||
|
||||
|
||||
def test_a_change_that_reduces_an_over_limit_rule_is_not_blamed():
|
||||
assert gate.evaluate({"TQ001": 13}, {"TQ001": 14}, _BUDGET) == ()
|
||||
|
||||
|
||||
def test_a_rule_absent_from_head_counts_as_zero():
|
||||
assert gate.evaluate({}, {}, _BUDGET) == ()
|
||||
|
||||
|
||||
def test_over_ceiling_names_only_the_rules_above_their_limit():
|
||||
assert gate.over_ceiling({"TQ001": 11, "TQ003": 5}, _BUDGET) == frozenset({"TQ001"})
|
||||
|
||||
|
||||
def test_over_ceiling_is_empty_when_everything_fits():
|
||||
assert gate.over_ceiling({"TQ001": 10, "TQ003": 4}, _BUDGET) == frozenset()
|
||||
|
||||
|
||||
def test_ratchet_lowers_a_limit_by_what_the_branch_fixed():
|
||||
updated = gate.ratcheted_budget(_BUDGET, {"TQ001": 6}, {"TQ001": 10})
|
||||
assert updated["TQ001"]["limit"] == 6
|
||||
|
||||
|
||||
def test_ratchet_never_raises_a_limit_when_violations_grew():
|
||||
updated = gate.ratcheted_budget(_BUDGET, {"TQ001": 20}, {"TQ001": 10})
|
||||
assert updated["TQ001"]["limit"] == 10
|
||||
|
||||
|
||||
def test_ratchet_never_goes_below_zero():
|
||||
updated = gate.ratcheted_budget({"TQ001": {"limit": 2}}, {"TQ001": 0}, {"TQ001": 100})
|
||||
assert updated["TQ001"]["limit"] == 0
|
||||
|
||||
|
||||
def test_ratchet_lowers_a_rule_introduced_on_this_branch_like_any_other():
|
||||
updated = gate.ratcheted_budget(_BUDGET, {"TQ001": 4}, {"TQ001": 10})
|
||||
assert updated["TQ001"]["limit"] == 4
|
||||
def test_editing_the_checker_rekeys_the_base_counts(tmp_path: Path) -> None:
|
||||
checker: Final = tmp_path / "check.py"
|
||||
checker.write_text("print('v1')\n")
|
||||
before: Final = gate.checker_identity(checker).artifact_name("abc123")
|
||||
assert gate.checker_identity(checker).artifact_name("abc123") == before
|
||||
checker.write_text("print('v2')\n")
|
||||
assert gate.checker_identity(checker).artifact_name("abc123") != before
|
||||
|
||||
|
||||
def test_parse_changed_lines_groups_hunks_under_their_own_file():
|
||||
|
|
@ -132,16 +82,6 @@ def test_introduced_keeps_only_violations_on_changed_lines():
|
|||
assert kept == (gate.Violation("tests/a.py", 3, "TQ001"),)
|
||||
|
||||
|
||||
def test_the_shipped_budget_covers_every_rule_the_checker_can_emit():
|
||||
import json
|
||||
|
||||
budget = json.loads((_REPO_ROOT / "test-quality-budget.json").read_text())
|
||||
assert set(budget) == {
|
||||
"TQ001", "TQ002", "TQ003", "TQ004", "TQ005", "TQ006", "TQ007", "TQ009"
|
||||
}
|
||||
assert all(spec["limit"] >= 0 for spec in budget.values())
|
||||
|
||||
|
||||
def _git(cwd: Path, *args: str) -> str:
|
||||
proc = subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True)
|
||||
assert proc.returncode == 0, proc.stderr
|
||||
|
|
@ -204,7 +144,8 @@ def _base_scan_stalled_in_its_checker(tmp_path: Path, driver: str) -> _StalledSc
|
|||
temp_dir = tmp_path / "tmp"
|
||||
temp_dir.mkdir()
|
||||
scan = subprocess.Popen(
|
||||
[sys.executable, "-c", driver, str(_MODULE_PATH), str(repo), str(slow_checker)],
|
||||
[sys.executable, "-c", driver, str(repo), str(slow_checker)],
|
||||
cwd=_MODULE_PATH.parent,
|
||||
env={**os.environ, "TMPDIR": str(temp_dir)},
|
||||
)
|
||||
if not _wait_until(scanning.exists, 30):
|
||||
|
|
|
|||
|
|
@ -1,14 +1,11 @@
|
|||
import hashlib
|
||||
import importlib.util
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from typing import Final
|
||||
|
||||
_MODULE_PATH = Path(__file__).resolve().parents[2] / "scripts" / "type_check_gate.py"
|
||||
_spec = importlib.util.spec_from_file_location("type_check_gate", _MODULE_PATH)
|
||||
gate = importlib.util.module_from_spec(_spec)
|
||||
_spec.loader.exec_module(gate)
|
||||
import pytest
|
||||
|
||||
import type_check_gate as gate
|
||||
|
||||
ROOT = gate.REPO_ROOT
|
||||
|
||||
|
|
@ -132,99 +129,6 @@ def test_run_basedpyright_fails_loudly_on_a_crash_exit_code(tmp_path):
|
|||
gate.run_basedpyright(cwd=tmp_path, env_dir=env_dir)
|
||||
|
||||
|
||||
def test_at_or_under_ceiling_passes():
|
||||
budget = {"no-any-return": {"limit": 5}}
|
||||
assert gate.evaluate({"no-any-return": 5}, {}, budget) == []
|
||||
|
||||
|
||||
def test_one_more_error_than_ceiling_fails():
|
||||
budget = {"no-any-return": {"limit": 5}}
|
||||
assert gate.evaluate({"no-any-return": 6}, {}, budget) == [
|
||||
gate.Breach("no-any-return", 6, 5, 6)
|
||||
]
|
||||
|
||||
|
||||
def test_limit_absorbs_increase_up_to_it_then_fails_past_it():
|
||||
budget = {"arg-type": {"limit": 10}}
|
||||
assert gate.evaluate({"arg-type": 10}, {}, budget) == []
|
||||
assert gate.evaluate({"arg-type": 11}, {}, budget) == [
|
||||
gate.Breach("arg-type", 11, 10, 11)
|
||||
]
|
||||
|
||||
|
||||
def test_unbudgeted_new_code_uses_default_limit():
|
||||
assert gate.evaluate({"brand-new": gate.DEFAULT_LIMIT}, {}, {}) == []
|
||||
assert gate.evaluate({"brand-new": gate.DEFAULT_LIMIT + 1}, {}, {}) == [
|
||||
gate.Breach(
|
||||
"brand-new",
|
||||
gate.DEFAULT_LIMIT + 1,
|
||||
gate.DEFAULT_LIMIT,
|
||||
gate.DEFAULT_LIMIT + 1,
|
||||
)
|
||||
]
|
||||
|
||||
|
||||
def test_drift_already_over_cap_in_base_is_not_blamed_on_a_flat_change():
|
||||
# The bystander case: a rule sits over its limit because two earlier PRs
|
||||
# summed past it. A PR that branches off that base and adds nothing must pass
|
||||
# -- total > limit but total == base, so the `> base` guard spares it.
|
||||
budget = {"arg-type": {"limit": 10}}
|
||||
assert gate.evaluate({"arg-type": 12}, {"arg-type": 12}, budget) == []
|
||||
|
||||
|
||||
def test_change_that_grows_an_over_cap_rule_is_blamed_for_only_what_it_added():
|
||||
# Over limit AND above base: blamed, and `added` is the delta vs base, not the
|
||||
# whole overage, so the message points at this change's contribution.
|
||||
budget = {"arg-type": {"limit": 10}}
|
||||
assert gate.evaluate({"arg-type": 14}, {"arg-type": 12}, budget) == [
|
||||
gate.Breach("arg-type", 14, 10, 2)
|
||||
]
|
||||
|
||||
|
||||
def test_reducing_an_over_cap_rule_below_base_passes():
|
||||
budget = {"arg-type": {"limit": 10}}
|
||||
assert gate.evaluate({"arg-type": 11}, {"arg-type": 12}, budget) == []
|
||||
|
||||
|
||||
def test_no_output_against_a_nonempty_budget_is_a_vacuous_run():
|
||||
# A crashed type checker emits nothing; the gate must not certify it as clean.
|
||||
budget = {"no-untyped-def": {"limit": 4898}}
|
||||
assert gate.is_vacuous_run({}, budget) is True
|
||||
|
||||
|
||||
def test_genuine_zero_and_empty_budget_are_not_vacuous():
|
||||
assert gate.is_vacuous_run({}, {}) is False
|
||||
assert gate.is_vacuous_run({}, {"no-untyped-def": {"limit": 0}}) is False
|
||||
assert (
|
||||
gate.is_vacuous_run({"arg-type": 1}, {"arg-type": {"limit": 10}}) is False
|
||||
)
|
||||
|
||||
|
||||
def test_update_ratchets_a_limit_down_by_what_the_branch_fixed():
|
||||
# A rule that dropped from 40 (branch point) to 30 (current) fixed 10, so its
|
||||
# limit of 100 falls to 90 -- the granted headroom (60) is preserved, not the
|
||||
# raw count.
|
||||
budget = {"reportAny": {"limit": 100}}
|
||||
assert gate.ratcheted_budget(budget, {"reportAny": 30}, {"reportAny": 40}) == {
|
||||
"reportAny": {"limit": 90}
|
||||
}
|
||||
|
||||
|
||||
def test_update_never_raises_a_limit_when_a_rule_grows():
|
||||
# Adding violations must not loosen the ceiling; the limit holds flat.
|
||||
budget = {"reportAny": {"limit": 100}}
|
||||
assert gate.ratcheted_budget(budget, {"reportAny": 55}, {"reportAny": 40}) == {
|
||||
"reportAny": {"limit": 100}
|
||||
}
|
||||
|
||||
|
||||
def test_update_clamps_a_limit_at_zero_never_negative():
|
||||
budget = {"reportAny": {"limit": 5}}
|
||||
assert gate.ratcheted_budget(budget, {"reportAny": 0}, {"reportAny": 40}) == {
|
||||
"reportAny": {"limit": 0}
|
||||
}
|
||||
|
||||
|
||||
def test_malformed_basedpyright_json_exits_loudly_not_as_zero_errors():
|
||||
import pytest
|
||||
|
||||
|
|
@ -238,35 +142,6 @@ def test_empty_basedpyright_payload_counts_zero():
|
|||
assert gate.count_basedpyright("") == {}
|
||||
|
||||
|
||||
def test_over_ceiling_flags_only_rules_above_their_limit():
|
||||
budget = {"reportAny": {"limit": 10}}
|
||||
assert gate.over_ceiling({"reportAny": 10}, budget) == frozenset()
|
||||
assert gate.over_ceiling({"reportAny": 11}, budget) == frozenset({"reportAny"})
|
||||
assert gate.over_ceiling({}, budget) == frozenset()
|
||||
|
||||
|
||||
def test_over_ceiling_holds_unbudgeted_rules_to_the_default_limit():
|
||||
assert gate.over_ceiling({"brand-new": gate.DEFAULT_LIMIT}, {}) == frozenset()
|
||||
assert gate.over_ceiling({"brand-new": gate.DEFAULT_LIMIT + 1}, {}) == frozenset(
|
||||
{"brand-new"}
|
||||
)
|
||||
|
||||
|
||||
def test_over_ceiling_is_independent_across_rules():
|
||||
budget = {"reportAny": {"limit": 10}, "reportArgumentType": {"limit": 5}}
|
||||
assert gate.over_ceiling(
|
||||
{"reportAny": 9, "reportArgumentType": 6}, budget
|
||||
) == frozenset({"reportArgumentType"})
|
||||
|
||||
|
||||
def test_cache_key_changes_with_base_point_and_each_fingerprint():
|
||||
key = gate.cache_key("abc", ("cfg", "lock"))
|
||||
assert gate.cache_key("abc", ("cfg", "lock")) == key
|
||||
assert gate.cache_key("def", ("cfg", "lock")) != key
|
||||
assert gate.cache_key("abc", ("cfg2", "lock")) != key
|
||||
assert gate.cache_key("abc", ("cfg", "lock2")) != key
|
||||
|
||||
|
||||
def test_fingerprints_carry_the_dependency_group_set():
|
||||
# Counts measured under one group set must never be compared against
|
||||
# another's: the fingerprint difference re-keys every cache entry and
|
||||
|
|
@ -350,381 +225,42 @@ def test_ensure_env_is_silent_when_the_env_already_exists(tmp_path, capsys):
|
|||
assert capsys.readouterr().err == ""
|
||||
|
||||
|
||||
def test_cached_counts_round_trip(tmp_path):
|
||||
path = gate.cache_path(tmp_path, "abc123", ("f1", "f2"))
|
||||
gate.store_counts(tmp_path, path, "abc123", {"reportAny": 3, "reportCall": 1})
|
||||
assert gate.load_cached_counts(path) == {"reportAny": 3, "reportCall": 1}
|
||||
|
||||
|
||||
def test_missing_corrupt_or_misshapen_cache_reads_as_none(tmp_path):
|
||||
path = tmp_path / "cache.json"
|
||||
assert gate.load_cached_counts(path) is None
|
||||
path.write_text("{not json")
|
||||
assert gate.load_cached_counts(path) is None
|
||||
path.write_text(json.dumps(["counts"]))
|
||||
assert gate.load_cached_counts(path) is None
|
||||
path.write_text(json.dumps({"base_point": "abc"}))
|
||||
assert gate.load_cached_counts(path) is None
|
||||
path.write_text(json.dumps({"counts": {"reportAny": "three"}}))
|
||||
assert gate.load_cached_counts(path) is None
|
||||
path.write_text(json.dumps({"counts": {"reportAny": True}}))
|
||||
assert gate.load_cached_counts(path) is None
|
||||
|
||||
|
||||
def test_scratch_is_invisible_to_the_prune_glob():
|
||||
import fnmatch
|
||||
|
||||
scratch = gate.scratch_path(gate.cache_path(Path("/c"), "abc", ("f",)))
|
||||
assert not fnmatch.fnmatch(scratch.name, f"{gate.CACHE_FILE_PREFIX}*")
|
||||
|
||||
|
||||
def test_store_prune_spares_a_concurrent_runs_in_flight_scratch(tmp_path):
|
||||
foreign = gate.scratch_path(gate.cache_path(tmp_path, "other", ("f",)))
|
||||
foreign.parent.mkdir(parents=True, exist_ok=True)
|
||||
foreign.write_text("{}")
|
||||
mine = gate.cache_path(tmp_path, "mine", ("f",))
|
||||
gate.store_counts(tmp_path, mine, "mine", {"reportAny": 1})
|
||||
assert foreign.exists()
|
||||
assert gate.load_cached_counts(mine) == {"reportAny": 1}
|
||||
|
||||
|
||||
def test_store_keeps_a_concurrent_worktrees_entry_for_another_branch_point(tmp_path):
|
||||
old = gate.cache_path(tmp_path, "old", ("f",))
|
||||
gate.store_counts(tmp_path, old, "old", {"reportAny": 1})
|
||||
new = gate.cache_path(tmp_path, "new", ("f",))
|
||||
gate.store_counts(tmp_path, new, "new", {"reportAny": 2})
|
||||
assert gate.load_cached_counts(old) == {"reportAny": 1}
|
||||
assert gate.load_cached_counts(new) == {"reportAny": 2}
|
||||
|
||||
|
||||
def test_store_evicts_only_the_oldest_entries_beyond_the_cap(tmp_path):
|
||||
aged = [
|
||||
gate.cache_path(tmp_path, f"base{i}", ("f",))
|
||||
for i in range(gate.CACHE_KEEP_ENTRIES)
|
||||
]
|
||||
for age, path in enumerate(aged):
|
||||
gate.store_counts(tmp_path, path, f"base{age}", {"reportAny": age})
|
||||
os.utime(path, (age, age))
|
||||
newest = gate.cache_path(tmp_path, "newest", ("f",))
|
||||
gate.store_counts(tmp_path, newest, "newest", {"reportAny": 99})
|
||||
assert not aged[0].exists()
|
||||
assert all(path.exists() for path in aged[1:])
|
||||
assert gate.load_cached_counts(newest) == {"reportAny": 99}
|
||||
|
||||
|
||||
def test_store_never_evicts_the_entry_it_just_wrote_even_on_mtime_ties(tmp_path):
|
||||
others = [
|
||||
gate.cache_path(tmp_path, f"base{i}", ("f",))
|
||||
for i in range(gate.CACHE_KEEP_ENTRIES + 2)
|
||||
]
|
||||
for path in others:
|
||||
gate.store_counts(tmp_path, path, path.name, {"reportAny": 1})
|
||||
os.utime(path, (9_999_999_999, 9_999_999_999))
|
||||
mine = gate.cache_path(tmp_path, "mine", ("f",))
|
||||
gate.store_counts(tmp_path, mine, "mine", {"reportAny": 2})
|
||||
assert gate.load_cached_counts(mine) == {"reportAny": 2}
|
||||
survivors = list(tmp_path.glob(f"{gate.CACHE_FILE_PREFIX}*.json"))
|
||||
assert len(survivors) == gate.CACHE_KEEP_ENTRIES
|
||||
|
||||
|
||||
def _no_fetch(ref):
|
||||
return None
|
||||
|
||||
|
||||
def _never(reason):
|
||||
def callback(ref):
|
||||
raise AssertionError(reason)
|
||||
|
||||
return callback
|
||||
|
||||
|
||||
def test_base_counts_cached_returns_the_hit_without_recomputing(tmp_path):
|
||||
path = gate.cache_path(tmp_path, "abc123", gate.environment_fingerprints())
|
||||
gate.store_counts(tmp_path, path, "abc123", {"reportAny": 7})
|
||||
|
||||
assert gate.base_counts_cached(
|
||||
"abc123",
|
||||
cache_dir=tmp_path,
|
||||
compute=_never("a cache hit must not re-run the base pass"),
|
||||
fetch=_never("a cache hit must not reach for CI"),
|
||||
) == {"reportAny": 7}
|
||||
|
||||
|
||||
def test_base_counts_cached_computes_once_then_hits(tmp_path):
|
||||
calls = []
|
||||
|
||||
def fake(ref):
|
||||
calls.append(ref)
|
||||
return {"reportAny": 4}
|
||||
|
||||
first = gate.base_counts_cached(
|
||||
"abc123", cache_dir=tmp_path, compute=fake, fetch=_no_fetch
|
||||
)
|
||||
second = gate.base_counts_cached(
|
||||
"abc123", cache_dir=tmp_path, compute=fake, fetch=_no_fetch
|
||||
)
|
||||
assert first == second == {"reportAny": 4}
|
||||
assert calls == ["abc123"]
|
||||
|
||||
|
||||
def test_an_empty_base_pass_is_never_cached(tmp_path):
|
||||
calls = []
|
||||
|
||||
def crashed(ref):
|
||||
calls.append(ref)
|
||||
return {}
|
||||
|
||||
assert (
|
||||
gate.base_counts_cached(
|
||||
"abc123", cache_dir=tmp_path, compute=crashed, fetch=_no_fetch
|
||||
)
|
||||
== {}
|
||||
)
|
||||
assert (
|
||||
gate.base_counts_cached(
|
||||
"abc123", cache_dir=tmp_path, compute=crashed, fetch=_no_fetch
|
||||
)
|
||||
== {}
|
||||
)
|
||||
assert calls == ["abc123", "abc123"]
|
||||
assert list(tmp_path.iterdir()) == []
|
||||
|
||||
|
||||
def test_base_counts_cached_uses_fetched_counts_and_persists_them(tmp_path):
|
||||
counts = gate.base_counts_cached(
|
||||
"abc123",
|
||||
cache_dir=tmp_path,
|
||||
compute=_never("fetched counts must skip the local base pass"),
|
||||
fetch=lambda ref: {"reportAny": 9},
|
||||
)
|
||||
assert counts == {"reportAny": 9}
|
||||
path = gate.cache_path(tmp_path, "abc123", gate.environment_fingerprints())
|
||||
assert gate.load_cached_counts(path) == {"reportAny": 9}
|
||||
assert gate.base_counts_cached(
|
||||
"abc123",
|
||||
cache_dir=tmp_path,
|
||||
compute=_never("the persisted fetch must satisfy later runs"),
|
||||
fetch=_never("the persisted fetch must satisfy later runs"),
|
||||
) == {"reportAny": 9}
|
||||
|
||||
|
||||
def test_base_counts_cached_falls_back_to_compute_on_a_fetch_miss(tmp_path):
|
||||
calls = []
|
||||
|
||||
def local(ref):
|
||||
calls.append(ref)
|
||||
return {"reportAny": 4}
|
||||
|
||||
assert gate.base_counts_cached(
|
||||
"abc123", cache_dir=tmp_path, compute=local, fetch=_no_fetch
|
||||
) == {"reportAny": 4}
|
||||
assert calls == ["abc123"]
|
||||
|
||||
|
||||
def test_base_counts_cached_treats_empty_fetched_counts_as_a_miss(tmp_path):
|
||||
assert gate.base_counts_cached(
|
||||
"abc123",
|
||||
cache_dir=tmp_path,
|
||||
compute=lambda ref: {"reportAny": 2},
|
||||
fetch=lambda ref: {},
|
||||
) == {"reportAny": 2}
|
||||
path = gate.cache_path(tmp_path, "abc123", gate.environment_fingerprints())
|
||||
assert gate.load_cached_counts(path) == {"reportAny": 2}
|
||||
|
||||
|
||||
def test_origin_slug_parsing_supports_ssh_and_https_github_forms():
|
||||
assert gate.parse_origin_slug("git@github.com:BerriAI/litellm.git") == "BerriAI/litellm"
|
||||
assert gate.parse_origin_slug("git@github.com:BerriAI/litellm") == "BerriAI/litellm"
|
||||
assert gate.parse_origin_slug("https://github.com/BerriAI/litellm.git") == "BerriAI/litellm"
|
||||
assert gate.parse_origin_slug("https://github.com/BerriAI/litellm") == "BerriAI/litellm"
|
||||
assert gate.parse_origin_slug("https://github.com/BerriAI/litellm/") == "BerriAI/litellm"
|
||||
|
||||
|
||||
def test_origin_slug_parsing_rejects_non_github_urls():
|
||||
assert gate.parse_origin_slug("https://gitlab.com/BerriAI/litellm.git") is None
|
||||
assert gate.parse_origin_slug("git@bitbucket.org:BerriAI/litellm.git") is None
|
||||
assert gate.parse_origin_slug("not a url") is None
|
||||
assert gate.parse_origin_slug("") is None
|
||||
|
||||
|
||||
def _artifact_zip(payload):
|
||||
import io
|
||||
import zipfile
|
||||
|
||||
buffer = io.BytesIO()
|
||||
with zipfile.ZipFile(buffer, "w") as archive:
|
||||
archive.writestr("basedpyright-counts.json", json.dumps(payload))
|
||||
return buffer.getvalue()
|
||||
|
||||
|
||||
def _gh_stub(listing, zip_bytes):
|
||||
def gh_output(args):
|
||||
if args[-1].startswith("repos/"):
|
||||
return json.dumps(listing).encode()
|
||||
return zip_bytes
|
||||
|
||||
return gh_output
|
||||
|
||||
|
||||
def _live_listing():
|
||||
return {
|
||||
"artifacts": [
|
||||
{"expired": False, "archive_download_url": "https://api.github.com/x/zip"}
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
def test_fetcher_returns_counts_from_a_matching_artifact(capsys):
|
||||
payload = {"base_point": "abc123", "counts": {"reportAny": 3}}
|
||||
fetched = gate.fetch_ci_base_counts(
|
||||
"abc123", gh_output=_gh_stub(_live_listing(), _artifact_zip(payload))
|
||||
)
|
||||
assert fetched == {"reportAny": 3}
|
||||
assert "fetched from CI artifact" in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_fetcher_rejects_an_artifact_for_a_different_base_point():
|
||||
payload = {"base_point": "someothersha", "counts": {"reportAny": 3}}
|
||||
assert (
|
||||
gate.fetch_ci_base_counts(
|
||||
"abc123", gh_output=_gh_stub(_live_listing(), _artifact_zip(payload))
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
def test_fetcher_rejects_empty_or_misshapen_artifact_counts():
|
||||
for counts in ({}, {"reportAny": "three"}, {"reportAny": True}):
|
||||
payload = {"base_point": "abc123", "counts": counts}
|
||||
assert (
|
||||
gate.fetch_ci_base_counts(
|
||||
"abc123", gh_output=_gh_stub(_live_listing(), _artifact_zip(payload))
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
def test_fetcher_rejects_an_expired_artifact():
|
||||
listing = {
|
||||
"artifacts": [
|
||||
{"expired": True, "archive_download_url": "https://api.github.com/x/zip"}
|
||||
]
|
||||
}
|
||||
payload = {"base_point": "abc123", "counts": {"reportAny": 3}}
|
||||
assert (
|
||||
gate.fetch_ci_base_counts(
|
||||
"abc123", gh_output=_gh_stub(listing, _artifact_zip(payload))
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
def test_fetcher_misses_when_no_artifact_is_published():
|
||||
assert (
|
||||
gate.fetch_ci_base_counts(
|
||||
"abc123", gh_output=_gh_stub({"artifacts": []}, b"")
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
def test_fetcher_misses_when_gh_is_unusable(capsys):
|
||||
assert gate.fetch_ci_base_counts("abc123", gh_output=lambda args: None) is None
|
||||
assert "computing base counts locally" in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_fetcher_misses_on_a_corrupt_artifact_archive():
|
||||
assert (
|
||||
gate.fetch_ci_base_counts(
|
||||
"abc123", gh_output=_gh_stub(_live_listing(), b"not a zip")
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
def test_emit_writes_the_artifact_json_named_by_the_head_key(tmp_path, capsys):
|
||||
gate.cmd_emit_counts({"reportAny": 3, "aRule": 1}, tmp_path, "deadbeef")
|
||||
key = gate.cache_key("deadbeef", gate.environment_fingerprints())
|
||||
path = tmp_path / f"basedpyright-counts-{key}.json"
|
||||
assert json.loads(path.read_text()) == {
|
||||
"base_point": "deadbeef",
|
||||
"counts": {"aRule": 1, "reportAny": 3},
|
||||
}
|
||||
summary = capsys.readouterr().out
|
||||
assert "deadbeef" in summary
|
||||
assert key in summary
|
||||
assert "4" in summary
|
||||
|
||||
|
||||
def test_emit_refuses_to_publish_empty_counts(tmp_path):
|
||||
import pytest
|
||||
|
||||
def test_changing_the_dependency_groups_rekeys_the_base_counts() -> None:
|
||||
default: Final = gate.checker_identity().artifact_name("abc123")
|
||||
assert gate.checker_identity().artifact_name("abc123") == default
|
||||
assert gate.checker_identity(("proxy-dev",)).artifact_name("abc123") != default
|
||||
|
||||
|
||||
@pytest.mark.parametrize("rule", ["reportAny", "reportExplicitAny"])
|
||||
def test_any_rules_may_grow_past_their_base_up_to_the_cap_and_no_further(
|
||||
rule: str, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
cap: Final = gate.ANY_CAPS[rule]
|
||||
base: Final = {rule: cap - 5, "reportArgumentType": 3}
|
||||
gate.judge({rule: cap, "reportArgumentType": 3}, base, "a" * 40)
|
||||
assert "OK" in capsys.readouterr().out
|
||||
with pytest.raises(SystemExit) as exit_info:
|
||||
gate.judge({rule: cap + 1, "reportArgumentType": 3}, base, "a" * 40)
|
||||
assert exit_info.value.code == 1
|
||||
assert f"BREACHED RULES: {rule} {cap + 1}/{cap} (+6)" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_an_any_rule_already_over_its_cap_at_base_does_not_fail_a_bystander(
|
||||
capsys: pytest.CaptureFixture[str],
|
||||
) -> None:
|
||||
over: Final = gate.ANY_CAPS["reportAny"] + 50
|
||||
gate.judge({"reportAny": over}, {"reportAny": over}, "a" * 40)
|
||||
assert "OK" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_rules_without_a_cap_may_not_grow_past_their_base(capsys: pytest.CaptureFixture[str]) -> None:
|
||||
with pytest.raises(SystemExit):
|
||||
gate.cmd_emit_counts({}, tmp_path, "deadbeef")
|
||||
assert list(tmp_path.iterdir()) == []
|
||||
gate.judge({"reportArgumentType": 4}, {"reportArgumentType": 3}, "a" * 40)
|
||||
assert "BREACHED RULES: reportArgumentType 4/3 (+1)" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_emitted_file_round_trips_through_the_fetch_validation(tmp_path):
|
||||
gate.cmd_emit_counts({"reportAny": 3}, tmp_path, "deadbeef")
|
||||
key = gate.cache_key("deadbeef", gate.environment_fingerprints())
|
||||
payload = json.loads((tmp_path / f"basedpyright-counts-{key}.json").read_text())
|
||||
assert gate.counts_for_base(payload, "deadbeef") == {"reportAny": 3}
|
||||
assert gate.counts_for_base(payload, "someothersha") is None
|
||||
|
||||
|
||||
def _git(cwd, *args):
|
||||
proc = subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True)
|
||||
assert proc.returncode == 0, proc.stderr
|
||||
return proc.stdout.strip()
|
||||
|
||||
|
||||
def _commit(cwd, name):
|
||||
(cwd / name).write_text(name)
|
||||
_git(cwd, "add", "-A")
|
||||
_git(cwd, "commit", "-q", "-m", name)
|
||||
return _git(cwd, "rev-parse", "HEAD")
|
||||
|
||||
|
||||
def _init_repo(tmp_path):
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
_git(repo, "init", "-q", "-b", "main")
|
||||
_git(repo, "config", "user.email", "gate@example.com")
|
||||
_git(repo, "config", "user.name", "gate")
|
||||
_git(repo, "config", "commit.gpgsign", "false")
|
||||
return repo
|
||||
|
||||
|
||||
def _branched_repo(tmp_path):
|
||||
repo = _init_repo(tmp_path)
|
||||
branch_point = _commit(repo, "shared.txt")
|
||||
_git(repo, "checkout", "-q", "-b", "feature")
|
||||
_commit(repo, "feature.txt")
|
||||
_git(repo, "checkout", "-q", "main")
|
||||
base_tip = _commit(repo, "drift.txt")
|
||||
_git(repo, "checkout", "-q", "feature")
|
||||
return repo, branch_point, base_tip
|
||||
|
||||
|
||||
def test_base_point_is_the_branch_point_when_no_merge_is_in_progress(tmp_path):
|
||||
repo, branch_point, _ = _branched_repo(tmp_path)
|
||||
assert gate.resolve_base_point("main", cwd=repo) == branch_point
|
||||
|
||||
|
||||
def test_base_point_mid_merge_advances_to_the_merged_in_base_tip(tmp_path):
|
||||
repo, _, base_tip = _branched_repo(tmp_path)
|
||||
_git(repo, "merge", "--no-commit", "--no-ff", "main")
|
||||
assert gate.resolve_base_point("main", cwd=repo) == base_tip
|
||||
|
||||
|
||||
def test_base_point_mid_merge_of_an_older_side_branch_keeps_the_newer_branch_point(tmp_path):
|
||||
repo = _init_repo(tmp_path)
|
||||
_commit(repo, "shared.txt")
|
||||
_git(repo, "checkout", "-q", "-b", "old-side")
|
||||
_commit(repo, "old.txt")
|
||||
_git(repo, "checkout", "-q", "main")
|
||||
newer_point = _commit(repo, "drift.txt")
|
||||
_git(repo, "checkout", "-q", "-b", "feature")
|
||||
_commit(repo, "feature.txt")
|
||||
_git(repo, "merge", "--no-commit", "--no-ff", "old-side")
|
||||
assert gate.resolve_base_point("main", cwd=repo) == newer_point
|
||||
def test_no_head_output_is_refused_as_vacuous_before_any_base_lookup(capsys: pytest.CaptureFixture[str]) -> None:
|
||||
with pytest.raises(SystemExit) as exit_info:
|
||||
gate.cmd_check({}, "irrelevant-base-ref")
|
||||
assert exit_info.value.code == 1
|
||||
assert "vacuous" in capsys.readouterr().out
|
||||
|
|
|
|||
|
|
@ -1,106 +1,64 @@
|
|||
"""Tests for scripts/type_discipline_gate.py.
|
||||
|
||||
The gate's correctness lives in two pure functions: `over_ceiling` (which decides
|
||||
whether the expensive base worktree scan is even needed) and `evaluate` (the
|
||||
drift-safe breach check). Both are pinned here.
|
||||
The gate compares each LIT rule's codebase count against the merge-base count, so
|
||||
what is pinned here is the identity that keys those base counts and the diff scan
|
||||
that turns a breach into file:line.
|
||||
"""
|
||||
|
||||
import importlib.util
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from typing import Final
|
||||
|
||||
_MODULE_PATH = Path(__file__).resolve().parents[2] / "scripts" / "type_discipline_gate.py"
|
||||
_spec = importlib.util.spec_from_file_location("type_discipline_gate", _MODULE_PATH)
|
||||
gate = importlib.util.module_from_spec(_spec)
|
||||
_spec.loader.exec_module(gate)
|
||||
import type_discipline_gate as gate
|
||||
|
||||
|
||||
def _budget(limit):
|
||||
return {"LIT006": {"limit": limit}}
|
||||
def test_editing_the_checker_rekeys_the_base_counts(tmp_path: Path) -> None:
|
||||
checker: Final = tmp_path / "check.py"
|
||||
checker.write_text("print('v1')\n")
|
||||
before: Final = gate.checker_identity(checker).artifact_name("abc123")
|
||||
assert gate.checker_identity(checker).artifact_name("abc123") == before
|
||||
checker.write_text("print('v2')\n")
|
||||
assert gate.checker_identity(checker).artifact_name("abc123") != before
|
||||
|
||||
|
||||
def test_over_ceiling_flags_only_counts_above_the_limit():
|
||||
budget = _budget(12)
|
||||
assert gate.over_ceiling({"LIT006": 12}, budget) == frozenset() # at limit
|
||||
assert gate.over_ceiling({"LIT006": 13}, budget) == frozenset({"LIT006"}) # over limit
|
||||
assert gate.over_ceiling({}, budget) == frozenset() # missing rule counts as zero
|
||||
def test_parse_changed_lines_groups_hunks_under_their_own_file() -> None:
|
||||
diff: Final = (
|
||||
"diff --git a/litellm/a.py b/litellm/a.py\n"
|
||||
"--- a/litellm/a.py\n"
|
||||
"+++ b/litellm/a.py\n"
|
||||
"@@ -0,0 +3,2 @@\n"
|
||||
"+one\n"
|
||||
"+two\n"
|
||||
"diff --git a/litellm/b.py b/litellm/b.py\n"
|
||||
"--- a/litellm/b.py\n"
|
||||
"+++ b/litellm/b.py\n"
|
||||
"@@ -0,0 +10 @@\n"
|
||||
"+only\n"
|
||||
)
|
||||
changed: Final = gate.parse_changed_lines(diff)
|
||||
assert changed["litellm/a.py"] == {3, 4}
|
||||
assert changed["litellm/b.py"] == {10}
|
||||
|
||||
|
||||
def test_over_ceiling_is_independent_across_rules():
|
||||
budget = {"LIT001": {"limit": 5}, "LIT006": {"limit": 10}}
|
||||
assert gate.over_ceiling({"LIT001": 6, "LIT006": 10}, budget) == frozenset({"LIT001"})
|
||||
def test_parse_changed_lines_handles_several_hunks_in_one_file() -> None:
|
||||
diff: Final = (
|
||||
"+++ b/litellm/a.py\n"
|
||||
"@@ -0,0 +1,2 @@\n"
|
||||
"+a\n"
|
||||
"@@ -9,0 +20,1 @@\n"
|
||||
"+b\n"
|
||||
)
|
||||
assert gate.parse_changed_lines(diff)["litellm/a.py"] == {1, 2, 20}
|
||||
|
||||
|
||||
def test_evaluate_blames_only_a_rule_over_limit_and_over_base():
|
||||
budget = _budget(10)
|
||||
# over limit and grown vs base -> breach
|
||||
assert [b.rule for b in gate.evaluate({"LIT006": 12}, {"LIT006": 9}, budget)] == ["LIT006"]
|
||||
# over limit but flat vs base (pre-existing drift) -> not blamed
|
||||
assert gate.evaluate({"LIT006": 12}, {"LIT006": 12}, budget) == []
|
||||
# within limit -> not blamed regardless of base
|
||||
assert gate.evaluate({"LIT006": 10}, {"LIT006": 0}, budget) == []
|
||||
def test_parse_changed_lines_on_an_empty_diff_is_empty() -> None:
|
||||
assert gate.parse_changed_lines("") == {}
|
||||
|
||||
|
||||
def test_update_ratchets_limit_down_by_what_the_branch_fixed_never_up():
|
||||
budget = {"LIT001": {"limit": 100}, "LIT006": {"limit": 10}}
|
||||
# LIT001 fixed 15 (60 -> 45) so its limit falls 100 -> 85; LIT006 grew, so its
|
||||
# limit holds flat at 10.
|
||||
current = {"LIT001": 45, "LIT006": 12}
|
||||
base = {"LIT001": 60, "LIT006": 9}
|
||||
assert gate.ratcheted_budget(budget, current, base) == {
|
||||
"LIT001": {"limit": 85},
|
||||
"LIT006": {"limit": 10},
|
||||
}
|
||||
|
||||
|
||||
def test_update_leaves_rules_seeded_on_this_branch_untouched():
|
||||
# A rule absent from the base budget was seeded with grandfathered headroom on
|
||||
# this branch; the base tree predates the rule (e.g. no Final annotations yet),
|
||||
# so ratcheting against it would collapse the deliberate headroom.
|
||||
budget = {"LIT001": {"limit": 100}, "LIT010": {"limit": 24600}}
|
||||
current = {"LIT001": 45, "LIT010": 16400}
|
||||
base = {"LIT001": 60, "LIT010": 40000}
|
||||
assert gate.ratcheted_budget(budget, current, base, frozenset({"LIT010"})) == {
|
||||
"LIT001": {"limit": 85},
|
||||
"LIT010": {"limit": 24600},
|
||||
}
|
||||
|
||||
|
||||
def _git(cwd, *args):
|
||||
proc = subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True)
|
||||
assert proc.returncode == 0, proc.stderr
|
||||
return proc.stdout.strip()
|
||||
|
||||
|
||||
def _commit(cwd, name):
|
||||
(cwd / name).write_text(name)
|
||||
_git(cwd, "add", "-A")
|
||||
_git(cwd, "commit", "-q", "-m", name)
|
||||
return _git(cwd, "rev-parse", "HEAD")
|
||||
|
||||
|
||||
def _branched_repo(tmp_path):
|
||||
repo = tmp_path / "repo"
|
||||
repo.mkdir()
|
||||
_git(repo, "init", "-q", "-b", "main")
|
||||
_git(repo, "config", "user.email", "gate@example.com")
|
||||
_git(repo, "config", "user.name", "gate")
|
||||
_git(repo, "config", "commit.gpgsign", "false")
|
||||
branch_point = _commit(repo, "shared.txt")
|
||||
_git(repo, "checkout", "-q", "-b", "feature")
|
||||
_commit(repo, "feature.txt")
|
||||
_git(repo, "checkout", "-q", "main")
|
||||
base_tip = _commit(repo, "drift.txt")
|
||||
_git(repo, "checkout", "-q", "feature")
|
||||
return repo, branch_point, base_tip
|
||||
|
||||
|
||||
def test_base_point_is_the_branch_point_when_no_merge_is_in_progress(tmp_path):
|
||||
repo, branch_point, _ = _branched_repo(tmp_path)
|
||||
assert gate.resolve_base_point("main", cwd=repo) == branch_point
|
||||
|
||||
|
||||
def test_base_point_mid_merge_advances_to_the_merged_in_base_tip(tmp_path):
|
||||
repo, _, base_tip = _branched_repo(tmp_path)
|
||||
_git(repo, "merge", "--no-commit", "--no-ff", "main")
|
||||
assert gate.resolve_base_point("main", cwd=repo) == base_tip
|
||||
def test_introduced_keeps_only_violations_on_changed_lines() -> None:
|
||||
violations: Final = (
|
||||
gate.Violation("litellm/a.py", 3, "LIT006"),
|
||||
gate.Violation("litellm/a.py", 99, "LIT006"),
|
||||
gate.Violation("litellm/b.py", 3, "LIT001"),
|
||||
)
|
||||
kept: Final = gate.introduced(violations, {"litellm/a.py": {3}})
|
||||
assert kept == [gate.Violation("litellm/a.py", 3, "LIT006")]
|
||||
|
|
|
|||
|
|
@ -1,44 +0,0 @@
|
|||
{
|
||||
"LIT001": {
|
||||
"limit": 22174
|
||||
},
|
||||
"LIT003": {
|
||||
"limit": 261
|
||||
},
|
||||
"LIT004": {
|
||||
"limit": 38
|
||||
},
|
||||
"LIT005": {
|
||||
"limit": 0
|
||||
},
|
||||
"LIT006": {
|
||||
"limit": 1035
|
||||
},
|
||||
"LIT007": {
|
||||
"limit": 0
|
||||
},
|
||||
"LIT008": {
|
||||
"limit": 945
|
||||
},
|
||||
"LIT009": {
|
||||
"limit": 0
|
||||
},
|
||||
"LIT010": {
|
||||
"limit": 16398
|
||||
},
|
||||
"LIT011": {
|
||||
"limit": 5504
|
||||
},
|
||||
"LIT012": {
|
||||
"limit": 4486
|
||||
},
|
||||
"LIT013": {
|
||||
"limit": 0
|
||||
},
|
||||
"LIT014": {
|
||||
"limit": 369
|
||||
},
|
||||
"LIT015": {
|
||||
"limit": 1448
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue