mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-09 22:31:41 +00:00
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.
This commit is contained in:
parent
1009976c49
commit
37f2b2e9bf
29 changed files with 1291 additions and 2715 deletions
|
|
@ -1,66 +0,0 @@
|
|||
name: Publish basedpyright base counts
|
||||
|
||||
# Every commit on main or litellm_internal_staging 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
|
||||
- litellm_internal_staging
|
||||
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
|
||||
97
.github/workflows/publish-lint-base-counts.yml
vendored
Normal file
97
.github/workflows/publish-lint-base-counts.yml
vendored
Normal file
|
|
@ -0,0 +1,97 @@
|
|||
name: Publish lint base counts
|
||||
|
||||
# Every commit on main or litellm_internal_staging 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
|
||||
- litellm_internal_staging
|
||||
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
|
|
@ -19,9 +19,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
|
||||
|
|
@ -142,18 +142,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"
|
||||
|
||||
|
|
@ -162,7 +168,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 }}
|
||||
|
|
@ -190,40 +196,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
|
||||
|
|
|
|||
|
|
@ -52,11 +52,11 @@ 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 the merge-base with `litellm_internal_staging`, plus the small per-rule headroom listed in each gate script's `HEADROOM`. 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
|
||||
|
||||
`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
|
||||
|
||||
|
|
|
|||
39
Makefile
39
Makefile
|
|
@ -5,9 +5,8 @@
|
|||
test-unit-integrations test-unit-core-utils test-unit-other test-unit-root \
|
||||
test-proxy-unit-a test-proxy-unit-b test-integration test-unit-helm \
|
||||
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
|
||||
|
|
@ -31,13 +30,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 (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"
|
||||
|
|
@ -138,7 +134,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:
|
||||
|
|
@ -207,25 +203,20 @@ lint-basedpyright: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE)
|
|||
lint-e2e-basedpyright: $(LINT_E2E_DEP_INSTALL)
|
||||
$(UV_RUN) basedpyright tests/e2e
|
||||
|
||||
# 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
|
||||
|
|
@ -233,18 +224,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 ..
|
||||
|
||||
|
|
@ -254,7 +233,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": 44358
|
||||
},
|
||||
"reportUnknownLambdaType": {
|
||||
"limit": 109
|
||||
},
|
||||
"reportUnknownMemberType": {
|
||||
"limit": 38271
|
||||
},
|
||||
"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
|
||||
}
|
||||
}
|
||||
|
|
@ -334,6 +334,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": 2956
|
||||
},
|
||||
"ANN002": {
|
||||
"limit": 71
|
||||
},
|
||||
"ANN003": {
|
||||
"limit": 806
|
||||
},
|
||||
"ANN201": {
|
||||
"limit": 1979
|
||||
},
|
||||
"ANN202": {
|
||||
"limit": 831
|
||||
},
|
||||
"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": 2916
|
||||
},
|
||||
"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": 217
|
||||
},
|
||||
"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,233 +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), 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.
|
||||
|
||||
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 json
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from types import MappingProxyType
|
||||
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"})
|
||||
|
||||
|
||||
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 _regression_detail(
|
||||
rule: str,
|
||||
base_limits: dict[str, int],
|
||||
head_limits: dict[str, int],
|
||||
graduated: tuple[str, ...],
|
||||
) -> str | None:
|
||||
"""Why `rule` regressed vs base, or None when it held flat, fell, or graduated.
|
||||
|
||||
A dropped rule is terminal unless it graduated; otherwise the only loosening
|
||||
left is a raised limit.
|
||||
"""
|
||||
base_limit = base_limits[rule]
|
||||
if rule not in head_limits:
|
||||
if 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, ...] = (),
|
||||
) -> 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)) 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)))
|
||||
|
||||
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
|
||||
|
|
|
|||
313
scripts/lint_base_counts.py
Normal file
313
scripts/lint_base_counts.py
Normal file
|
|
@ -0,0 +1,313 @@
|
|||
#!/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 the merge-base count plus
|
||||
that rule's fixed headroom. 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, 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 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]
|
||||
|
||||
|
||||
class Breach(NamedTuple):
|
||||
rule: str
|
||||
total: int
|
||||
cap: 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]
|
||||
|
||||
|
||||
def _git(args: Sequence[str], cwd: Path) -> 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
|
||||
|
||||
|
||||
def head_sha(cwd: Path = REPO_ROOT) -> str:
|
||||
return _git(["rev-parse", "HEAD"], cwd).strip()
|
||||
|
||||
|
||||
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 = _git(["merge-base", base_ref, "HEAD"], cwd).strip()
|
||||
if not head_point:
|
||||
return base_ref
|
||||
merge_head: Final = _git(["rev-parse", "--verify", "--quiet", "MERGE_HEAD"], cwd).strip()
|
||||
if not merge_head:
|
||||
return head_point
|
||||
merge_point: Final = _git(["merge-base", base_ref, merge_head], cwd).strip()
|
||||
if not merge_point:
|
||||
return head_point
|
||||
older: Final = _git(["merge-base", head_point, merge_point], cwd).strip()
|
||||
return merge_point if older == head_point else head_point
|
||||
|
||||
|
||||
def default_cache_dir(cwd: Path = REPO_ROOT) -> Path:
|
||||
common: Final = Path(_git(["rev-parse", "--git-common-dir"], cwd).strip())
|
||||
resolved: Final = common if common.is_absolute() else cwd / common
|
||||
return resolved / 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 evaluate(head: Counts, base: Counts, headroom: Counts) -> tuple[Breach, ...]:
|
||||
return tuple(
|
||||
Breach(rule, total, base.get(rule, 0) + headroom.get(rule, 0), total - base.get(rule, 0))
|
||||
for rule, total in sorted(head.items())
|
||||
if total > base.get(rule, 0) + headroom.get(rule, 0)
|
||||
)
|
||||
|
|
@ -12,10 +12,10 @@
|
|||
# - litellm/ Python -> `make lint` (test-linting.yml's lint job)
|
||||
# - tests/e2e 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)
|
||||
#
|
||||
|
|
@ -32,7 +32,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" "$@"
|
||||
|
|
@ -96,7 +96,7 @@ existing_files() {
|
|||
|
||||
litellm_py_pattern='^litellm/.*\.py$'
|
||||
e2e_py_pattern='^tests/e2e/.*\.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)$'
|
||||
|
|
@ -143,7 +143,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
|
||||
|
|
@ -300,9 +300,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
|
||||
|
|
@ -330,7 +330,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,18 @@
|
|||
#!/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 plus its headroom in HEADROOM (none today), 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 +23,28 @@ import subprocess
|
|||
import sys
|
||||
import tempfile
|
||||
from collections import Counter
|
||||
from collections.abc import Mapping, Sequence
|
||||
from pathlib import Path
|
||||
from types import MappingProxyType
|
||||
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"
|
||||
HEADROOM: Final[Mapping[str, int]] = MappingProxyType({})
|
||||
|
||||
_HUNK: Final = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@")
|
||||
|
||||
|
||||
class Violation(NamedTuple):
|
||||
|
|
@ -34,38 +53,20 @@ 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() -> Checker:
|
||||
return Checker("ruff-strict", (sha256_of(STRICT_CONFIG), sha256_of(BASE_CONFIG), ruff_version()))
|
||||
|
||||
|
||||
def _ruff_json(cwd: Path, config: Path) -> list:
|
||||
|
|
@ -76,7 +77,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 +91,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 +122,52 @@ 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)
|
||||
if not breaches:
|
||||
print(f"OK: every strict rule is within its codebase ceiling (base {base})")
|
||||
return
|
||||
new = introduced(
|
||||
head,
|
||||
parse_changed_lines(
|
||||
_run(["git", "diff", base_point, "--unified=0", "--no-color", "--", TARGET])
|
||||
),
|
||||
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), HEADROOM
|
||||
)
|
||||
print(f"FAIL: strict-rule totals exceed their limit (base {base}):")
|
||||
if not breaches:
|
||||
print(f"OK: no strict rule grew past its merge-base count (base {base})")
|
||||
return
|
||||
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.cap} (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 plus the rule's headroom in scripts/ruff_strict_gate.py."
|
||||
)
|
||||
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,29 @@
|
|||
#!/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.
|
||||
|
||||
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 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.
|
||||
|
||||
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.
|
||||
Type discipline keeps the LIT010/LIT011 pool its seeding left; a test-quality
|
||||
violation has no such transition to absorb, so the line is the merge-base 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,10 +36,20 @@ 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"
|
||||
HEADROOM: Final[Mapping[str, int]] = MappingProxyType({})
|
||||
TERMINATION_SIGNALS: Final = (signal.SIGTERM, signal.SIGHUP)
|
||||
|
||||
_HUNK: Final = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@", re.MULTILINE)
|
||||
|
|
@ -56,13 +63,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 +71,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:
|
||||
return Checker("test-quality", (sha256_of(CHECKER),))
|
||||
|
||||
|
||||
def _check(root: Path, checker: Path) -> tuple[Violation, ...]:
|
||||
|
|
@ -143,26 +129,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 +157,48 @@ 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)
|
||||
if not breaches:
|
||||
print(f"OK: every TQ rule is within its test-suite ceiling (base {base})")
|
||||
return
|
||||
new: Final = introduced(
|
||||
head,
|
||||
parse_changed_lines(
|
||||
_run(["git", "diff", base_point, "--unified=0", "--no-color", "--", TARGET])
|
||||
),
|
||||
breaches: Final = evaluate(
|
||||
count_by_rule(head), base_counts_cached(checker_identity(), base_point, base_counts), HEADROOM
|
||||
)
|
||||
print(f"FAIL: TQ-rule totals exceed their limit (base {base}):")
|
||||
if not breaches:
|
||||
print(f"OK: no TQ rule grew past its merge-base count (base {base})")
|
||||
return
|
||||
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.cap} (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,14 @@
|
|||
"""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 plus its headroom in HEADROOM (10 for the
|
||||
two Any-discipline rules, 0 for everything else), 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.
|
||||
There is no committed budget: the merge-base count is the ceiling, so it moves
|
||||
only when the base branch does.
|
||||
|
||||
Installed packages are part of the measurement: a typed dependency that is
|
||||
present changes what basedpyright can prove (and therefore which diagnostics
|
||||
|
|
@ -28,21 +27,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 staging 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 +47,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 +90,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
|
||||
HEADROOM: Final[Mapping[str, int]] = MappingProxyType({"reportAny": 10, "reportExplicitAny": 10})
|
||||
|
||||
|
||||
def _to_relative(raw: str, root: Path) -> str | None:
|
||||
|
|
@ -238,25 +225,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 +251,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 +264,66 @@ 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() -> Checker:
|
||||
return Checker("basedpyright", environment_fingerprints())
|
||||
|
||||
|
||||
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)
|
||||
breaches: Final = evaluate(head, base, HEADROOM)
|
||||
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: no basedpyright rule grew past its merge-base count "
|
||||
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 merge-base count (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.cap} (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 plus the rule's headroom in scripts/type_check_gate.py."
|
||||
)
|
||||
summary = "; ".join(f"{b.code} {b.total}/{b.cap} (+{b.added})" for b in breaches)
|
||||
summary: Final = "; ".join(f"{b.rule} {b.total}/{b.cap} (+{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,52 +1,61 @@
|
|||
#!/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 plus
|
||||
its headroom in HEADROOM, 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), LIT002
|
||||
(mutable-collection construction), 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>`) 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; 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), LIT002 (mutable-collection construction), 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), and LIT012 (TypedDict field
|
||||
without a `ReadOnly[...]` qualifier; suppress with `# writable-ok: <reason>`).
|
||||
Every rule has zero headroom except LIT010 and LIT011, which keep the pool
|
||||
their seeding left them: a change may add that many before the gate trips.
|
||||
|
||||
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 types import MappingProxyType
|
||||
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"
|
||||
HEADROOM: Final[Mapping[str, int]] = MappingProxyType({"LIT010": 44, "LIT011": 5})
|
||||
|
||||
_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):
|
||||
|
|
@ -55,41 +64,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:
|
||||
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()
|
||||
|
|
@ -106,22 +93,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:
|
||||
|
|
@ -134,30 +121,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/"):
|
||||
|
|
@ -169,109 +134,55 @@ 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)
|
||||
if not breaches:
|
||||
print(f"OK: every LIT rule is within its codebase ceiling (base {base})")
|
||||
return
|
||||
new = introduced(
|
||||
head,
|
||||
parse_changed_lines(
|
||||
_run(["git", "diff", base_point, "--unified=0", "--no-color", "--", TARGET])
|
||||
),
|
||||
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), HEADROOM
|
||||
)
|
||||
print(f"FAIL: LIT-rule totals exceed their limit (base {base}):")
|
||||
if not breaches:
|
||||
print(f"OK: no LIT rule grew past its merge-base count (base {base})")
|
||||
return
|
||||
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.cap} (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>`), 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>`), or remove an equal number elsewhere; the ceiling is the "
|
||||
"merge-base count plus the rule's headroom in scripts/type_discipline_gate.py."
|
||||
)
|
||||
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
|
||||
},
|
||||
"TQ008": {
|
||||
"limit": 10993
|
||||
}
|
||||
}
|
||||
|
|
@ -1,158 +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
|
||||
|
||||
_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_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,7 +7,6 @@ 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
|
||||
|
|
@ -16,6 +15,8 @@ from pathlib import Path
|
|||
|
||||
import pytest
|
||||
|
||||
import type_discipline_gate as gate
|
||||
|
||||
_REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
_MODULE_PATH = _REPO_ROOT / "scripts" / "check_type_discipline.py"
|
||||
_spec = importlib.util.spec_from_file_location("check_type_discipline", _MODULE_PATH)
|
||||
|
|
@ -688,17 +689,16 @@ def test_writable_ok_without_reason_is_lit005_and_does_not_suppress(tmp_path):
|
|||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Budget integrity: every emittable LIT rule (bar the LIT000 read/parse error) is gated
|
||||
# Gate integrity: headroom only ever names rules the checker can emit
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
||||
|
||||
def test_budget_covers_exactly_the_checker_rules():
|
||||
budget = json.loads((_REPO_ROOT / "type-discipline-budget.json").read_text())
|
||||
def test_headroom_names_only_rules_the_checker_emits():
|
||||
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
|
||||
assert set(gate.HEADROOM) <= emitted
|
||||
for value in gate.HEADROOM.values():
|
||||
assert isinstance(value, int)
|
||||
assert value > 0
|
||||
|
||||
|
||||
_FANS_OUT = checker._worker_count(checker.PARALLEL_MIN_PATHS) > 1
|
||||
|
|
|
|||
|
|
@ -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/litellm_internal_staging"],
|
||||
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"],
|
||||
|
|
|
|||
443
tests/test_litellm/test_lint_base_counts.py
Normal file
443
tests/test_litellm/test_lint_base_counts.py
Normal file
|
|
@ -0,0 +1,443 @@
|
|||
"""Tests for scripts/lint_base_counts.py, the merge-base counting shared by the
|
||||
four lint gates: the ceiling rule with per-rule headroom, 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 subprocess
|
||||
import zipfile
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
import lint_base_counts as counts
|
||||
|
||||
_CHECKER = counts.Checker("basedpyright", ("f1", "f2"))
|
||||
_OTHER_CHECKER = counts.Checker("ruff-strict", ("f1", "f2"))
|
||||
|
||||
|
||||
def test_evaluate_passes_a_rule_that_did_not_grow():
|
||||
assert counts.evaluate({"LIT006": 12}, {"LIT006": 12}, {}) == ()
|
||||
|
||||
|
||||
def test_evaluate_blames_one_new_violation_of_a_zero_headroom_rule():
|
||||
assert counts.evaluate({"LIT006": 13}, {"LIT006": 12}, {}) == (counts.Breach("LIT006", 13, 12, 1),)
|
||||
|
||||
|
||||
def test_evaluate_lets_a_rule_grow_up_to_its_headroom_and_no_further():
|
||||
headroom = {"LIT010": 44}
|
||||
assert counts.evaluate({"LIT010": 144}, {"LIT010": 100}, headroom) == ()
|
||||
assert counts.evaluate({"LIT010": 145}, {"LIT010": 100}, headroom) == (counts.Breach("LIT010", 145, 144, 45),)
|
||||
|
||||
|
||||
def test_evaluate_headroom_applies_only_to_the_rule_it_names():
|
||||
assert counts.evaluate({"LIT006": 13}, {"LIT006": 12}, {"LIT010": 44}) == (counts.Breach("LIT006", 13, 12, 1),)
|
||||
|
||||
|
||||
def test_evaluate_counts_a_rule_absent_from_the_base_as_zero():
|
||||
assert counts.evaluate({"NEW99": 1}, {}, {}) == (counts.Breach("NEW99", 1, 0, 1),)
|
||||
|
||||
|
||||
def test_evaluate_never_blames_a_change_that_reduced_a_rule():
|
||||
assert counts.evaluate({"LIT006": 11}, {"LIT006": 12}, {}) == ()
|
||||
|
||||
|
||||
def test_evaluate_reports_every_grown_rule_sorted_by_name():
|
||||
head = {"TQ008": 3, "TQ001": 2, "TQ003": 5}
|
||||
base = {"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():
|
||||
assert counts.evaluate({}, {"LIT006": 12}, {}) == ()
|
||||
|
||||
|
||||
def test_cache_key_changes_with_base_point_and_each_fingerprint():
|
||||
key = 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():
|
||||
key = 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():
|
||||
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 = 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, content):
|
||||
path = 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():
|
||||
scratch = 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):
|
||||
foreign = counts.scratch_path(tmp_path / _CHECKER.cache_file_name("other"))
|
||||
foreign.write_text("{}")
|
||||
mine = 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):
|
||||
old = counts.store_counts(tmp_path, _CHECKER, "old", {"reportAny": 1})
|
||||
new = 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):
|
||||
aged = 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 = 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):
|
||||
for index in range(counts.CACHE_KEEP_ENTRIES + 2):
|
||||
path = counts.store_counts(tmp_path, _CHECKER, f"base{index}", {"reportAny": 1})
|
||||
os.utime(path, (9_999_999_999, 9_999_999_999))
|
||||
mine = 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):
|
||||
other = 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, base_point):
|
||||
return None
|
||||
|
||||
|
||||
def _never(reason):
|
||||
def callback(*args):
|
||||
raise AssertionError(reason)
|
||||
|
||||
return callback
|
||||
|
||||
|
||||
def test_base_counts_cached_returns_the_hit_without_recomputing(tmp_path):
|
||||
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):
|
||||
calls = []
|
||||
|
||||
def fake(ref):
|
||||
calls.append(ref)
|
||||
return {"reportAny": 4}
|
||||
|
||||
first = counts.base_counts_cached(_CHECKER, "abc123", fake, cache_dir=tmp_path, fetch=_no_fetch)
|
||||
second = 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):
|
||||
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):
|
||||
calls = []
|
||||
|
||||
def crashed(ref):
|
||||
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):
|
||||
fetched = 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):
|
||||
seen = []
|
||||
|
||||
def fetch(checker, base_point):
|
||||
seen.append((checker, base_point))
|
||||
return None
|
||||
|
||||
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):
|
||||
calls = []
|
||||
|
||||
def local(ref):
|
||||
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):
|
||||
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():
|
||||
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():
|
||||
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):
|
||||
buffer = io.BytesIO()
|
||||
with zipfile.ZipFile(buffer, "w") as archive:
|
||||
archive.writestr("counts.json", json.dumps(payload))
|
||||
return buffer.getvalue()
|
||||
|
||||
|
||||
def _gh_stub(listing, zip_bytes, seen=None):
|
||||
def gh_output(args):
|
||||
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():
|
||||
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 = 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():
|
||||
seen = []
|
||||
payload = {"base_point": "abc123", "counts": {"reportAny": 3}}
|
||||
counts.fetch_ci_base_counts(_CHECKER, "abc123", gh=_gh_stub(_live_listing(), _artifact_zip(payload), seen))
|
||||
listing_request = 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():
|
||||
payload = {"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):
|
||||
payload = {"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():
|
||||
listing = {"artifacts": [{"expired": True, "archive_download_url": "https://api.github.com/x/zip"}]}
|
||||
payload = {"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():
|
||||
assert counts.fetch_ci_base_counts(_CHECKER, "abc123", gh=_gh_stub({"artifacts": []}, b"")) is None
|
||||
|
||||
|
||||
def test_fetcher_misses_when_gh_is_unusable(capsys):
|
||||
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():
|
||||
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, capsys):
|
||||
counts.emit_counts(_CHECKER, {"reportAny": 3, "aRule": 1}, tmp_path, "deadbeef")
|
||||
name = _CHECKER.artifact_name("deadbeef")
|
||||
assert json.loads((tmp_path / f"{name}.json").read_text()) == {
|
||||
"base_point": "deadbeef",
|
||||
"counts": {"aRule": 1, "reportAny": 3},
|
||||
}
|
||||
summary = 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):
|
||||
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):
|
||||
written = counts.emit_counts(_CHECKER, {"reportAny": 3}, tmp_path, "deadbeef")
|
||||
payload = 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 = _artifact_zip(payload)
|
||||
assert counts.fetch_ci_base_counts(_CHECKER, "deadbeef", gh=_gh_stub(_live_listing(), listing_zip)) == {
|
||||
"reportAny": 3
|
||||
}
|
||||
|
||||
|
||||
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 counts.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 counts.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 counts.resolve_base_point("main", cwd=repo) == newer_point
|
||||
|
||||
|
||||
def test_head_sha_is_the_checked_out_commit(tmp_path):
|
||||
repo, _, _ = _branched_repo(tmp_path)
|
||||
assert counts.head_sha(cwd=repo) == _git(repo, "rev-parse", "HEAD")
|
||||
|
||||
|
||||
def test_default_cache_dir_lives_under_the_shared_git_dir(tmp_path):
|
||||
repo, _, _ = _branched_repo(tmp_path)
|
||||
assert counts.default_cache_dir(cwd=repo) == repo / ".git" / counts.CACHE_DIR_NAME
|
||||
|
|
@ -12,9 +12,9 @@ import pytest
|
|||
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)"
|
||||
)
|
||||
|
||||
|
|
@ -475,7 +475,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",
|
||||
|
|
@ -528,7 +527,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
|
||||
|
||||
|
||||
|
|
@ -571,7 +570,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,4 +1,3 @@
|
|||
import importlib.util
|
||||
import json
|
||||
import re
|
||||
import shutil
|
||||
|
|
@ -8,80 +7,21 @@ from pathlib import Path
|
|||
|
||||
import pytest
|
||||
|
||||
import lint_base_counts
|
||||
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,23 @@ 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
|
||||
|
||||
|
||||
@_needs_ruff
|
||||
def test_the_checker_identity_is_rekeyed_by_either_ruff_config_or_the_ruff_version():
|
||||
identity = gate.checker_identity()
|
||||
assert identity.name == "ruff-strict"
|
||||
assert identity.fingerprints == (
|
||||
lint_base_counts.sha256_of(gate.STRICT_CONFIG),
|
||||
lint_base_counts.sha256_of(gate.BASE_CONFIG),
|
||||
gate.ruff_version(),
|
||||
)
|
||||
|
||||
|
||||
def test_every_headroom_rule_is_one_the_strict_config_selects():
|
||||
selectors = tuple(_strict_selected())
|
||||
unmeasured = frozenset(code for code in gate.HEADROOM if not code.startswith(selectors))
|
||||
assert unmeasured == frozenset(), (
|
||||
f"the gate never counts {sorted(unmeasured)}, so headroom for them is dead config that reads "
|
||||
"as coverage. Either select them in ruff-strict.toml or drop the headroom entry."
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1,12 +1,12 @@
|
|||
"""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 gate's whole value is that it blames a change only for what it adds. That lives
|
||||
in pure functions, so they are tested directly: `lint_base_counts.evaluate` for the
|
||||
blame rule and `parse_changed_lines` for 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
|
||||
|
|
@ -17,75 +17,48 @@ from contextlib import suppress
|
|||
from pathlib import Path
|
||||
from typing import NamedTuple
|
||||
|
||||
import lint_base_counts
|
||||
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_that_did_not_grow_is_not_a_breach():
|
||||
assert lint_base_counts.evaluate({"TQ001": 10}, {"TQ001": 10}, {}) == ()
|
||||
|
||||
|
||||
def test_a_rule_over_its_limit_that_the_change_added_is_a_breach():
|
||||
breaches = gate.evaluate({"TQ001": 12}, {"TQ001": 10}, _BUDGET)
|
||||
def test_a_rule_the_change_grew_is_a_breach_for_exactly_what_it_added():
|
||||
breaches = lint_base_counts.evaluate({"TQ001": 12}, {"TQ001": 10}, {})
|
||||
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) == ()
|
||||
assert lint_base_counts.evaluate({"TQ001": 14}, {"TQ001": 14}, {}) == ()
|
||||
|
||||
|
||||
def test_a_change_that_reduces_an_over_limit_rule_is_not_blamed():
|
||||
assert gate.evaluate({"TQ001": 13}, {"TQ001": 14}, _BUDGET) == ()
|
||||
def test_a_change_that_reduces_a_rule_is_not_blamed():
|
||||
assert lint_base_counts.evaluate({"TQ001": 13}, {"TQ001": 14}, {}) == ()
|
||||
|
||||
|
||||
def test_a_rule_absent_from_head_counts_as_zero():
|
||||
assert gate.evaluate({}, {}, _BUDGET) == ()
|
||||
assert lint_base_counts.evaluate({}, {}, {}) == ()
|
||||
|
||||
|
||||
def test_over_ceiling_names_only_the_rules_above_their_limit():
|
||||
assert gate.over_ceiling({"TQ001": 11, "TQ003": 5}, _BUDGET) == frozenset({"TQ001"})
|
||||
def test_the_gate_grants_no_headroom_to_any_rule():
|
||||
assert dict(gate.HEADROOM) == {}
|
||||
|
||||
|
||||
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_the_checker_identity_is_keyed_on_the_checker_source():
|
||||
identity = gate.checker_identity()
|
||||
assert identity == lint_base_counts.Checker("test-quality", (lint_base_counts.sha256_of(gate.CHECKER),))
|
||||
|
||||
|
||||
def test_parse_changed_lines_groups_hunks_under_their_own_file():
|
||||
|
|
@ -132,14 +105,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", "TQ008"}
|
||||
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
|
||||
|
|
@ -202,7 +167,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
|
||||
|
||||
_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 lint_base_counts
|
||||
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,17 @@ 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_the_checker_identity_is_keyed_on_the_environment_fingerprints():
|
||||
assert gate.checker_identity() == lint_base_counts.Checker("basedpyright", gate.environment_fingerprints())
|
||||
|
||||
|
||||
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_headroom_is_kept_only_for_the_any_discipline_rules():
|
||||
assert set(gate.HEADROOM) == {"reportAny", "reportExplicitAny"}
|
||||
assert all(isinstance(value, int) and value > 0 for value in gate.HEADROOM.values())
|
||||
|
||||
|
||||
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
|
||||
|
||||
with pytest.raises(SystemExit):
|
||||
gate.cmd_emit_counts({}, tmp_path, "deadbeef")
|
||||
assert list(tmp_path.iterdir()) == []
|
||||
|
||||
|
||||
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):
|
||||
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 plus
|
||||
the headroom the gate script grants, so what is pinned here is the identity that
|
||||
keys those base counts, the headroom the gate deliberately keeps, and the diff scan
|
||||
that turns a breach into file:line.
|
||||
"""
|
||||
|
||||
import importlib.util
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
_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 lint_base_counts
|
||||
import type_discipline_gate as gate
|
||||
|
||||
|
||||
def _budget(limit):
|
||||
return {"LIT006": {"limit": limit}}
|
||||
def test_headroom_is_kept_only_for_the_seeded_final_and_rebind_rules():
|
||||
assert set(gate.HEADROOM) == {"LIT010", "LIT011"}
|
||||
assert all(isinstance(value, int) and value > 0 for value in gate.HEADROOM.values())
|
||||
|
||||
|
||||
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_the_checker_identity_is_keyed_on_the_checker_source():
|
||||
identity = gate.checker_identity()
|
||||
assert identity == lint_base_counts.Checker("type-discipline", (lint_base_counts.sha256_of(gate.CHECKER),))
|
||||
|
||||
|
||||
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_groups_hunks_under_their_own_file():
|
||||
diff = (
|
||||
"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 = gate.parse_changed_lines(diff)
|
||||
assert changed["litellm/a.py"] == {3, 4}
|
||||
assert changed["litellm/b.py"] == {10}
|
||||
|
||||
|
||||
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_handles_several_hunks_in_one_file():
|
||||
diff = (
|
||||
"+++ 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_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_parse_changed_lines_on_an_empty_diff_is_empty():
|
||||
assert gate.parse_changed_lines("") == {}
|
||||
|
||||
|
||||
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():
|
||||
violations = (
|
||||
gate.Violation("litellm/a.py", 3, "LIT006"),
|
||||
gate.Violation("litellm/a.py", 99, "LIT006"),
|
||||
gate.Violation("litellm/b.py", 3, "LIT001"),
|
||||
)
|
||||
kept = gate.introduced(violations, {"litellm/a.py": {3}})
|
||||
assert kept == [gate.Violation("litellm/a.py", 3, "LIT006")]
|
||||
|
|
|
|||
|
|
@ -1,38 +0,0 @@
|
|||
{
|
||||
"LIT001": {
|
||||
"limit": 22180
|
||||
},
|
||||
"LIT002": {
|
||||
"limit": 26729
|
||||
},
|
||||
"LIT003": {
|
||||
"limit": 261
|
||||
},
|
||||
"LIT004": {
|
||||
"limit": 38
|
||||
},
|
||||
"LIT005": {
|
||||
"limit": 0
|
||||
},
|
||||
"LIT006": {
|
||||
"limit": 1035
|
||||
},
|
||||
"LIT007": {
|
||||
"limit": 0
|
||||
},
|
||||
"LIT008": {
|
||||
"limit": 945
|
||||
},
|
||||
"LIT009": {
|
||||
"limit": 0
|
||||
},
|
||||
"LIT010": {
|
||||
"limit": 16426
|
||||
},
|
||||
"LIT011": {
|
||||
"limit": 5506
|
||||
},
|
||||
"LIT012": {
|
||||
"limit": 4486
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue