scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-09 22:31:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 87

fix(make check): lint the test tree on tests-only changes like CI does

Browse source 
CI's required lint job runs ruff with ruff-tests.toml over tests/ and the
test-quality budget gate, but scripts/pre_commit_lint.sh only triggered make
lint on litellm/ files, so a tests-only commit passed make check with a no-op
note and then failed CI (a duplicate test name, ruff F811, did exactly that).

When tests/ Python files are in scope and no litellm/ files are, run ruff
with ruff-tests.toml over the changed test files and make lint-test-quality,
with the matching partial-staging warning, summary line, and no-op condition.
This commit is contained in:
mateo-berri 2026-09-04 23:50:01 -07:00
parent aea5358c48
commit de2ba3fab1
2 changed files with 114 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
scripts/pre_commit_lint.sh
View file

@ -12,6 +12,8 @@
# - 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 over ruff-tests.toml + `make lint-test-quality` (test-linting.yml's
# test-tree ruff and test-quality budget 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)
#
@ -88,15 +90,18 @@ existing_files() {
litellm_py_pattern='^litellm/.*\.py$'
e2e_py_pattern='^tests/e2e/.*\.py$'
tests_py_pattern='^tests/.*\.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)$'
# CI's lint job (test-linting.yml) only inspects litellm/, so a tests-only or
# scripts-only commit can't turn it red; scope the trigger there to skip the slow
# make lint when it couldn't catch anything.
# CI's lint job (test-linting.yml) is make lint: litellm/ checks plus two test-tree
# steps (ruff over ruff-tests.toml and the test-quality budget). Trigger the slow
# make lint on litellm/ files only; a tests-only commit runs just those two steps.
litellm_py_files=$(scope_match "$litellm_py_pattern")
e2e_py_files=$(scope_match "$e2e_py_pattern")
tests_py_changed=$(scope_match "$tests_py_pattern")
tests_py_files=$(printf '%s\n' "$tests_py_changed" | existing_files)
# ruff format (and CI's format step) skip enterprise; the rest of make lint covers it.
fmt_files=$(printf '%s\n' "$litellm_py_files" | grep -v '^litellm/enterprise/' | existing_files)
# check-ui-api-types.yml triggers on any file under litellm/proxy or litellm/types
@ -136,6 +141,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)" "$tests_py_pattern" "$tests_py_changed"
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
@ -225,6 +231,16 @@ if [ -n "$e2e_py_files" ]; then
|| { echo "✗ Raw HTTP client import in tests/e2e. Route the call through tests/e2e/e2e_http.py, then re-run make check." >&2; status=1; }
fi
if [ -n "$tests_py_changed" ] && [ -z "$litellm_py_files" ]; then
if [ -n "$tests_py_files" ]; then
echo "check: linting the test tree (ruff check --config ruff-tests.toml, scoped tests files)"
printf '%s\n' "$tests_py_files" | xargs uv run --no-sync ruff check --config ruff-tests.toml \
|| { echo "✗ Test-tree ruff failed. Fix the errors above, then re-run make check." >&2; status=1; }
fi
echo "check: checking the test-quality budget (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; }
fi
dashboard_checks() {
echo "check: linting dashboard (prettier + eslint + lint budgets)"
if [ ! -d ui/litellm-dashboard/node_modules ]; then
@ -313,10 +329,11 @@ 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)" "$tests_py_changed" "no tests/ Python files 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"
if [ -z "$litellm_py_files$e2e_py_files$ui_prettier_changed$ui_eslint_changed$spec_files" ]; then
if [ -z "$litellm_py_files$e2e_py_files$tests_py_changed$ui_prettier_changed$ui_eslint_changed$spec_files" ]; then
echo "check: NOTE - no gating lint check matches the files in scope, so nothing ran:" >&2
printf '%s\n' "$scope" | sed 's/^/ /' >&2
echo " A pass here is a no-op, not a lint verdict." >&2

98
tests/test_litellm/test_pre_commit_lint.py
View file

@ -40,6 +40,9 @@ case "$*" in
sleep 60
fi
;;
lint-test-quality)
[ "${STUB_FAIL:-}" = "test-quality" ] && exit 1
;;
esac
exit 0
"""
@ -69,6 +72,10 @@ case "$*" in
*orjson*)
[ -n "${STUB_BARRIER_DIR:-}" ] && barrier_sync genapi "python dashboard"
;;
"run --no-sync ruff check --config ruff-tests.toml"*)
[ -n "${STUB_ARGS_DIR:-}" ] && echo "$*" >> "$STUB_ARGS_DIR/ruff_tests.args"
[ "${STUB_FAIL:-}" = "tests-ruff" ] && exit 1
;;
esac
exit 0
"""
@ -153,6 +160,13 @@ def _set_base_ref(repo: Path) -> None:
)
def _stage_file(repo: Path, relative: str, body: str) -> None:
path = repo / relative
path.parent.mkdir(parents=True, exist_ok=True)
path.write_text(body)
subprocess.run(["git", "add", relative], cwd=repo, check=True)
def test_nothing_staged_scopes_to_working_tree_diff_and_runs_checks(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path)
_commit_all(repo, "base")
@ -405,6 +419,7 @@ def test_run_ends_with_a_summary_of_ran_and_skipped_blocks(tmp_path: Path) -> No
assert "ran: dashboard lint (prettier + eslint + lint budgets)" in proc.stdout
assert "ran: dashboard API-type sync (npm run gen:api)" in proc.stdout
assert "skipped: tests/e2e checks (basedpyright + raw HTTP client ban) (no tests/e2e Python files in scope)" in proc.stdout
assert "skipped: test-tree lint (ruff-tests.toml + test-quality budget) (no tests/ Python files in scope)" in proc.stdout
assert "check: PASS" in proc.stdout
assert "check: FAIL" not in proc.stdout
@ -412,20 +427,93 @@ def test_run_ends_with_a_summary_of_ran_and_skipped_blocks(tmp_path: Path) -> No
def test_staged_files_matching_no_check_print_an_explicit_noop_note_and_nonempty_log(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path)
_commit_all(repo, "base")
tests_dir = repo / "tests" / "test_litellm"
tests_dir.mkdir(parents=True)
(tests_dir / "test_x.py").write_text("def test_x() -> None: ...\n")
subprocess.run(["git", "add", "tests"], cwd=repo, check=True)
_stage_file(repo, "scripts/tool.py", "def main() -> None: ...\n")
proc = _run(repo, bin_dir, {})
assert proc.returncode == 0, proc.stdout + proc.stderr
assert "no gating lint check matches the files in scope, so nothing ran" in proc.stdout
assert "tests/test_litellm/test_x.py" in proc.stdout
assert "scripts/tool.py" in proc.stdout
assert "a no-op, not a lint verdict" in proc.stdout
assert "check: PASS" in proc.stdout
assert "linting Python" not in proc.stdout
log = (repo / ".git" / "pre_commit_lint.log").read_text()
assert "check: summary" in log
assert "skipped: Python lint (make lint) (no litellm/ Python files in scope)" in log
assert "skipped: test-tree lint (ruff-tests.toml + test-quality budget) (no tests/ Python files in scope)" in log
def test_tests_only_change_runs_test_tree_ruff_on_staged_files_and_the_quality_gate(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path)
_commit_all(repo, "base")
args_dir = tmp_path / "args"
args_dir.mkdir()
_stage_file(repo, "tests/test_a.py", "def test_a() -> None: ...\n")
_stage_file(repo, "tests/test_b.py", "def test_b() -> None: ...\n")
_stage_file(repo, "tests/fixtures/data.json", "{}\n")
proc = _run(repo, bin_dir, {"STUB_ARGS_DIR": str(args_dir)})
assert proc.returncode == 0, proc.stdout + proc.stderr
ruff_args = (args_dir / "ruff_tests.args").read_text().splitlines()
assert ruff_args == ["run --no-sync ruff check --config ruff-tests.toml tests/test_a.py tests/test_b.py"]
assert "ran: test-tree lint (ruff-tests.toml + test-quality budget)" in proc.stdout
assert "no gating lint check matches" not in proc.stdout
assert "linting Python" not in proc.stdout
assert "check: PASS" in proc.stdout
@pytest.mark.parametrize(
("fail", "message"),
[
("tests-ruff", "Test-tree ruff failed"),
("test-quality", "Test-quality budget failed"),
],
)
def test_a_failing_test_tree_check_fails_a_tests_only_run(tmp_path: Path, fail: str, message: str) -> None:
repo, bin_dir = _sandbox(tmp_path)
_commit_all(repo, "base")
_stage_file(repo, "tests/test_a.py", "def test_a() -> None: ...\n")
proc = _run(repo, bin_dir, {"STUB_FAIL": fail})
assert proc.returncode == 1
assert message in proc.stdout + proc.stderr
assert "check: FAIL" in proc.stdout
def test_tests_changed_alongside_litellm_files_defer_to_make_lint(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path)
_commit_all(repo, "base")
args_dir = tmp_path / "args"
args_dir.mkdir()
_stage_file(repo, "litellm/foo.py", "x = 2\n")
_stage_file(repo, "tests/test_a.py", "def test_a() -> None: ...\n")
proc = _run(repo, bin_dir, {"STUB_ARGS_DIR": str(args_dir), "STUB_FAIL": "test-quality"})
assert proc.returncode == 0, proc.stdout + proc.stderr
assert "linting Python" in proc.stdout
assert not (args_dir / "ruff_tests.args").exists()
assert "ran: test-tree lint (ruff-tests.toml + test-quality budget)" in proc.stdout
def test_deleted_test_file_still_runs_the_quality_gate_without_feeding_ruff_the_missing_file(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path)
_stage_file(repo, "tests/test_a.py", "def test_a() -> None: ...\n")
_commit_all(repo, "base")
args_dir = tmp_path / "args"
args_dir.mkdir()
subprocess.run(["git", "rm", "-q", "tests/test_a.py"], cwd=repo, check=True)
proc = _run(repo, bin_dir, {"STUB_ARGS_DIR": str(args_dir), "STUB_FAIL": "test-quality"})
assert proc.returncode == 1
assert "Test-quality budget failed" in proc.stdout + proc.stderr
assert not (args_dir / "ruff_tests.args").exists()
def test_partial_staging_warns_when_test_files_are_left_unstaged(tmp_path: Path) -> None:
repo, bin_dir = _sandbox(tmp_path)
_stage_file(repo, "tests/test_a.py", "def test_a() -> None: ...\n")
_commit_all(repo, "base")
_stage_file(repo, "notes.md", "hi\n")
(repo / "tests" / "test_a.py").write_text("def test_a() -> None:\n assert True\n")
proc = _run(repo, bin_dir, {})
assert proc.returncode == 0, proc.stdout + proc.stderr
assert "SKIPPED test-tree lint (ruff-tests.toml + test-quality budget)" in proc.stdout
assert "tests/test_a.py" in proc.stdout
assert "make lint-test-quality" not in proc.stdout
def test_run_queues_through_the_machine_wide_gate_slot_lock(tmp_path: Path) -> None: