mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-10 22:41:41 +00:00
fix(check): run CI's whole-tree test ruff and widen the test-tree trigger
The scoped xargs list missed a ruff-tests.toml rule change and skipped ruff on deletions, so the block now runs test-linting.yml's exact command over tests/. ruff-tests.toml, test-quality-budget.json, and scripts/check_test_quality.py trigger the block too, and it sits after the background launches so the dashboard and gen:api jobs overlap it.
This commit is contained in:
mateo-berri
parent
de2ba3fab1
commit
d22962248c
2 changed files with 96 additions and 42 deletions
|
|
@ -12,7 +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
|
||||
# - tests/ Python, ruff-tests.toml, test-quality-budget.json, scripts/check_test_quality.py
|
||||
# -> 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)
|
||||
|
|
@ -90,18 +91,17 @@ existing_files() {
|
|||
|
||||
litellm_py_pattern='^litellm/.*\.py$'
|
||||
e2e_py_pattern='^tests/e2e/.*\.py$'
|
||||
tests_py_pattern='^tests/.*\.py$'
|
||||
test_tree_pattern='^(tests/.*\.py|ruff-tests\.toml|test-quality-budget\.json|scripts/check_test_quality\.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) 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.
|
||||
# make lint on litellm/ files only; without them, the test-tree steps run on their own below.
|
||||
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)
|
||||
test_tree_files=$(scope_match "$test_tree_pattern")
|
||||
# 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
|
||||
|
|
@ -141,7 +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 "test-tree lint (ruff-tests.toml + test-quality budget)" "$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
|
||||
|
|
@ -231,16 +231,6 @@ 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
|
||||
|
|
@ -304,6 +294,15 @@ if [ -n "$spec_files" ]; then
|
|||
set +m
|
||||
fi
|
||||
|
||||
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)"
|
||||
make lint-test-quality \
|
||||
|| { echo "✗ Test-quality budget failed. Fix the errors above, then re-run make check." >&2; status=1; }
|
||||
fi
|
||||
|
||||
if [ -n "${python_pid:-}" ]; then
|
||||
wait "$python_pid" || status=1
|
||||
cat "$python_log"; rm -f "$python_log"
|
||||
|
|
@ -329,11 +328,12 @@ 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 "test-tree lint (ruff-tests.toml + test-quality budget)" "$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"
|
||||
|
||||
if [ -z "$litellm_py_files$e2e_py_files$tests_py_changed$ui_prettier_changed$ui_eslint_changed$spec_files" ]; then
|
||||
if [ -z "$litellm_py_files$e2e_py_files$test_tree_files$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
|
||||
|
|
|
|||
|
|
@ -11,6 +11,12 @@ 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_SKIPPED = (
|
||||
"skipped: test-tree lint (ruff-tests.toml + test-quality budget) "
|
||||
"(no tests/ Python files or test-tree lint inputs in scope)"
|
||||
)
|
||||
|
||||
BARRIER_HELPER = """barrier_sync() {
|
||||
touch "$STUB_BARRIER_DIR/$1.started"
|
||||
|
|
@ -30,6 +36,7 @@ BARRIER_HELPER = """barrier_sync() {
|
|||
|
||||
MAKE_STUB = """#!/bin/sh
|
||||
. "$STUB_BIN/barrier.sh"
|
||||
[ -n "${STUB_ARGS_DIR:-}" ] && echo "$*" >> "$STUB_ARGS_DIR/make.args"
|
||||
case "$*" in
|
||||
lint)
|
||||
[ "${STUB_FAIL:-}" = "make-lint" ] && exit 1
|
||||
|
|
@ -419,7 +426,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 TEST_TREE_SKIPPED in proc.stdout
|
||||
assert "check: PASS" in proc.stdout
|
||||
assert "check: FAIL" not in proc.stdout
|
||||
|
||||
|
|
@ -438,41 +445,83 @@ def test_staged_files_matching_no_check_print_an_explicit_noop_note_and_nonempty
|
|||
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
|
||||
assert TEST_TREE_SKIPPED in log
|
||||
|
||||
|
||||
def test_tests_only_change_runs_test_tree_ruff_on_staged_files_and_the_quality_gate(tmp_path: Path) -> None:
|
||||
def _recorded(args_dir: Path, name: str) -> list[str]:
|
||||
path = args_dir / name
|
||||
return path.read_text().splitlines() if path.exists() else []
|
||||
|
||||
|
||||
def test_tests_only_change_runs_the_whole_test_tree_ruff_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 _recorded(args_dir, "ruff_tests.args") == [WHOLE_TREE_RUFF]
|
||||
assert _recorded(args_dir, "make.args") == ["lint-test-quality"]
|
||||
assert TEST_TREE_RAN 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"),
|
||||
],
|
||||
"changed",
|
||||
["ruff-tests.toml", "test-quality-budget.json", "scripts/check_test_quality.py", "tests/e2e/test_x.py"],
|
||||
)
|
||||
def test_a_failing_test_tree_check_fails_a_tests_only_run(tmp_path: Path, fail: str, message: str) -> None:
|
||||
def test_test_tree_lint_inputs_trigger_the_test_tree_checks(tmp_path: Path, changed: str) -> None:
|
||||
repo, bin_dir = _sandbox(tmp_path)
|
||||
_commit_all(repo, "base")
|
||||
args_dir = tmp_path / "args"
|
||||
args_dir.mkdir()
|
||||
_stage_file(repo, changed, "x = 1\n")
|
||||
proc = _run(repo, bin_dir, {"STUB_ARGS_DIR": str(args_dir)})
|
||||
assert proc.returncode == 0, proc.stdout + proc.stderr
|
||||
assert _recorded(args_dir, "ruff_tests.args") == [WHOLE_TREE_RUFF]
|
||||
assert "lint-test-quality" in _recorded(args_dir, "make.args")
|
||||
assert TEST_TREE_RAN in proc.stdout
|
||||
|
||||
|
||||
def test_nothing_staged_tests_only_working_tree_change_runs_the_test_tree_checks(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")
|
||||
_set_base_ref(repo)
|
||||
args_dir = tmp_path / "args"
|
||||
args_dir.mkdir()
|
||||
(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 "nothing staged; scoping to the working tree's diff" in proc.stdout
|
||||
assert _recorded(args_dir, "ruff_tests.args") == [WHOLE_TREE_RUFF]
|
||||
assert _recorded(args_dir, "make.args") == ["lint-test-quality"]
|
||||
|
||||
|
||||
def test_a_failing_test_tree_ruff_fails_the_run_and_still_runs_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")
|
||||
proc = _run(repo, bin_dir, {"STUB_ARGS_DIR": str(args_dir), "STUB_FAIL": "tests-ruff"})
|
||||
assert proc.returncode == 1
|
||||
assert "Test-tree ruff failed" in proc.stdout + proc.stderr
|
||||
assert "check: FAIL" in proc.stdout
|
||||
assert _recorded(args_dir, "make.args") == ["lint-test-quality"]
|
||||
|
||||
|
||||
def test_a_failing_quality_gate_fails_a_tests_only_run(tmp_path: Path) -> 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})
|
||||
proc = _run(repo, bin_dir, {"STUB_FAIL": "test-quality"})
|
||||
assert proc.returncode == 1
|
||||
assert message in proc.stdout + proc.stderr
|
||||
assert "Test-quality budget failed" in proc.stdout + proc.stderr
|
||||
assert "check: FAIL" in proc.stdout
|
||||
|
||||
|
||||
|
|
@ -486,34 +535,39 @@ def test_tests_changed_alongside_litellm_files_defer_to_make_lint(tmp_path: Path
|
|||
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
|
||||
assert _recorded(args_dir, "ruff_tests.args") == []
|
||||
assert _recorded(args_dir, "make.args") == ["lint"]
|
||||
assert TEST_TREE_RAN in proc.stdout
|
||||
|
||||
|
||||
def test_deleted_test_file_still_runs_the_quality_gate_without_feeding_ruff_the_missing_file(tmp_path: Path) -> None:
|
||||
def test_deleted_test_file_still_runs_the_test_tree_checks(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()
|
||||
proc = _run(repo, bin_dir, {"STUB_ARGS_DIR": str(args_dir)})
|
||||
assert proc.returncode == 0, proc.stdout + proc.stderr
|
||||
assert _recorded(args_dir, "ruff_tests.args") == [WHOLE_TREE_RUFF]
|
||||
assert _recorded(args_dir, "make.args") == ["lint-test-quality"]
|
||||
assert TEST_TREE_RAN in proc.stdout
|
||||
|
||||
|
||||
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")
|
||||
args_dir = tmp_path / "args"
|
||||
args_dir.mkdir()
|
||||
_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, {})
|
||||
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 "tests/test_a.py" in proc.stdout
|
||||
assert "make lint-test-quality" not in proc.stdout
|
||||
assert _recorded(args_dir, "ruff_tests.args") == []
|
||||
assert _recorded(args_dir, "make.args") == []
|
||||
|
||||
|
||||
def test_run_queues_through_the_machine_wide_gate_slot_lock(tmp_path: Path) -> None:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue