mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-13 08:52:02 +00:00
fix(lint): run the Router stub guard and e2e type check from make check
make check now runs check_router_constructor_stub.py when litellm/router.py or uv.lock changes, and the e2e basedpyright step on pyproject.toml, uv.lock, pyrightconfig.json, and litellm-proxy-extras Python, matching CI. CI's e2e type check also fires on litellm-proxy-extras Python, which tests/e2e/perf imports. LIT010 treats TypeAliasType like a type variable, since mypy also rejects a Final type alias.
This commit is contained in:
parent
f04d40b2a2
commit
dcb6324986
8 changed files with 84 additions and 15 deletions
4
.github/workflows/test-linting.yml
vendored
4
.github/workflows/test-linting.yml
vendored
|
|
@ -191,10 +191,10 @@ jobs:
|
|||
- name: Check tests/e2e basedpyright (zero errors)
|
||||
if: steps.changes.outputs.decision != 'skip'
|
||||
run: |
|
||||
if git diff --name-only --diff-filter=ACMRD "$GATE_BASE_SHA" HEAD -- ':(glob)tests/e2e/**/*.py' ':(glob)tests/e2e_harness/**/*.py' ':(glob)litellm/**/*.py' pyrightconfig.json pyproject.toml uv.lock | grep -q .; then
|
||||
if git diff --name-only --diff-filter=ACMRD "$GATE_BASE_SHA" HEAD -- ':(glob)tests/e2e/**/*.py' ':(glob)tests/e2e_harness/**/*.py' ':(glob)litellm/**/*.py' ':(glob)litellm-proxy-extras/**/*.py' pyrightconfig.json pyproject.toml uv.lock | grep -q .; then
|
||||
uv run --no-sync basedpyright tests/e2e tests/e2e_harness
|
||||
else
|
||||
echo "No changed tests/e2e, litellm Python, or dependency files; skipping."
|
||||
echo "No changed tests/e2e, litellm, or litellm-proxy-extras Python, or dependency files; skipping."
|
||||
fi
|
||||
|
||||
- name: Run the e2e harness tests
|
||||
|
|
|
|||
|
|
@ -461,7 +461,7 @@ def iter_guard_violations(path: Path, tree: ast.AST) -> Iterator[Violation]:
|
|||
# --------------------------------------------------------------------------- #
|
||||
|
||||
CONSTANT_DECLARATIONS = frozenset(("Final", "TypeAlias"))
|
||||
TYPE_VARIABLE_FACTORIES = frozenset(("TypeVar", "ParamSpec", "TypeVarTuple"))
|
||||
TYPE_VARIABLE_FACTORIES = frozenset(("TypeVar", "ParamSpec", "TypeVarTuple", "TypeAliasType"))
|
||||
CONFIG_SURFACE_PARTS = ("litellm", "__init__.py")
|
||||
SELF_PARAMS = frozenset(("self", "cls"))
|
||||
NESTED_SCOPES = (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef, ast.Lambda)
|
||||
|
|
@ -697,8 +697,8 @@ def iter_final_type_variable_violations(path: Path, tree: ast.AST) -> Iterator[V
|
|||
path,
|
||||
line,
|
||||
"LIT010",
|
||||
f"`{name}` is a type variable declared Final: mypy then reads it as a plain "
|
||||
f"variable and every signature using it as Any, so remove the Final",
|
||||
f"`{name}` is a type variable or type alias declared Final: mypy then reads it as a plain "
|
||||
f"variable instead of a type, so remove the Final",
|
||||
)
|
||||
case _:
|
||||
pass
|
||||
|
|
|
|||
|
|
@ -10,9 +10,13 @@
|
|||
# with origin's current default branch, untracked files included
|
||||
# The per-area checks:
|
||||
# - litellm/ Python -> `make lint` (test-linting.yml's python job)
|
||||
# - tests/e2e and tests/e2e_harness Python
|
||||
# - tests/e2e, tests/e2e_harness, and litellm-proxy-extras Python, pyrightconfig.json,
|
||||
# pyproject.toml, uv.lock
|
||||
# -> `make lint-e2e-basedpyright` (test-linting.yml's e2e type-check step)
|
||||
# + raw HTTP client ban (test-linting.yml's check_e2e_no_raw_requests)
|
||||
# - tests/e2e and tests/e2e_harness Python
|
||||
# -> raw HTTP client ban (test-linting.yml's check_e2e_no_raw_requests)
|
||||
# - litellm/router.py, uv.lock
|
||||
# -> mypy Router stub guard (test-linting.yml's check_router_constructor_stub)
|
||||
# - 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
|
||||
|
|
@ -98,6 +102,7 @@ existing_files() {
|
|||
|
||||
litellm_py_pattern='^litellm/.*\.py$'
|
||||
e2e_py_pattern='^tests/e2e(_harness)?/.*\.py$'
|
||||
e2e_types_pattern='^(tests/e2e(_harness)?/.*\.py|litellm-proxy-extras/.*\.py|pyrightconfig\.json|pyproject\.toml|uv\.lock)$'
|
||||
test_tree_pattern='^(tests/.*\.py|ruff-tests\.toml|scripts/(check_test_quality|test_quality_gate)\.py)$'
|
||||
spec_pattern='^(litellm/.*|enterprise/.*|pyproject\.toml|uv\.lock|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)$'
|
||||
|
|
@ -107,7 +112,11 @@ litellm_py_files=$(scope_match "$litellm_py_pattern")
|
|||
if [ -n "$(scope_match '^(litellm/proxy/mcp_server/|scripts/check_mcp_operation_boundary\.py)')" ]; then
|
||||
uv run --no-sync python scripts/check_mcp_operation_boundary.py || exit 1
|
||||
fi
|
||||
if [ -n "$(scope_match '^(litellm/router\.py|tests/code_coverage_tests/check_router_constructor_stub\.py|uv\.lock)$')" ]; then
|
||||
uv run --no-sync python tests/code_coverage_tests/check_router_constructor_stub.py || exit 1
|
||||
fi
|
||||
e2e_py_files=$(scope_match "$e2e_py_pattern")
|
||||
e2e_types_files=$(scope_match "$e2e_types_pattern")
|
||||
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)
|
||||
|
|
@ -144,7 +153,8 @@ if [ -n "$staged" ]; then
|
|||
printf '%s\n' "$missed" | sed 's/^/ /' >&2
|
||||
}
|
||||
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 "tests/e2e basedpyright" "$e2e_types_pattern" "$e2e_types_files$litellm_py_files"
|
||||
warn_skipped "tests/e2e raw HTTP client ban" "$e2e_py_pattern" "$e2e_py_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"
|
||||
|
|
@ -224,7 +234,7 @@ if [ -n "$litellm_py_files" ]; then
|
|||
set +m
|
||||
fi
|
||||
|
||||
if [ -n "$e2e_py_files" ] && [ -z "$litellm_py_files" ]; then
|
||||
if [ -n "$e2e_types_files" ] && [ -z "$litellm_py_files" ]; then
|
||||
echo "check: type-checking tests/e2e (make lint-e2e-basedpyright)"
|
||||
make lint-e2e-basedpyright || { echo "✗ tests/e2e basedpyright failed. Fix the errors above, then re-run make check." >&2; status=1; }
|
||||
fi
|
||||
|
|
@ -331,13 +341,15 @@ 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 "tests/e2e basedpyright" "$e2e_types_files$litellm_py_files" \
|
||||
"no litellm/, tests/e2e, or litellm-proxy-extras Python, or type-check inputs in scope"
|
||||
summary_item "tests/e2e raw HTTP client ban" "$e2e_py_files" "no tests/e2e Python files in scope"
|
||||
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/, enterprise/, dependency, or generator files in scope"
|
||||
|
||||
if [ -z "$litellm_py_files$e2e_py_files$test_tree_files$ui_prettier_changed$ui_eslint_changed$spec_files" ]; then
|
||||
if [ -z "$litellm_py_files$e2e_types_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
|
||||
|
|
|
|||
|
|
@ -131,7 +131,7 @@ Current limits: Bedrock cannot be mounted in record or replay (SigV4 signs the H
|
|||
|
||||
## Typing
|
||||
|
||||
The harness is fully typed with no error budget: `make lint-e2e-basedpyright` must report zero basedpyright errors, and CI enforces that on any PR touching `tests/e2e/**/*.py`, `tests/e2e_harness/**/*.py`, `litellm/**/*.py`, `pyrightconfig.json`, `pyproject.toml` or `uv.lock`. When a response field is untyped, model it in `models.py` (just the fields you read) and let pydantic validate it, rather than threading a `dict` or `Any` through the test
|
||||
The harness is fully typed with no error budget: `make lint-e2e-basedpyright` must report zero basedpyright errors, and CI enforces that on any PR touching `tests/e2e/**/*.py`, `tests/e2e_harness/**/*.py`, `litellm/**/*.py`, `litellm-proxy-extras/**/*.py`, `pyrightconfig.json`, `pyproject.toml` or `uv.lock`. When a response field is untyped, model it in `models.py` (just the fields you read) and let pydantic validate it, rather than threading a `dict` or `Any` through the test
|
||||
|
||||
## Typed test metadata
|
||||
|
||||
|
|
|
|||
|
|
@ -224,7 +224,7 @@ Mark live tests with `@pytest.mark.e2e` (on the class or the module). Pure cover
|
|||
|
||||
Before you push
|
||||
|
||||
1. Run `make lint-e2e-basedpyright` (or `make check` with your changes staged); the harness is fully typed and the gate allows zero basedpyright errors, enforced in CI on any PR touching `tests/e2e/**/*.py`, `tests/e2e_harness/**/*.py`, `litellm/**/*.py`, `pyrightconfig.json`, `pyproject.toml`, or `uv.lock`
|
||||
1. Run `make lint-e2e-basedpyright` (or `make check` with your changes staged); the harness is fully typed and the gate allows zero basedpyright errors, enforced in CI on any PR touching `tests/e2e/**/*.py`, `tests/e2e_harness/**/*.py`, `litellm/**/*.py`, `litellm-proxy-extras/**/*.py`, `pyrightconfig.json`, `pyproject.toml`, or `uv.lock`
|
||||
|
||||
2. Add the models your test needs to the config your local proxy loads
|
||||
|
||||
|
|
|
|||
|
|
@ -305,7 +305,14 @@ def test_single_leading_underscore_is_not_exempt(tmp_path):
|
|||
|
||||
@pytest.mark.parametrize(
|
||||
"declaration",
|
||||
("TypeVar('_T')", "ParamSpec('_T')", "TypeVarTuple('_T')", "typing.TypeVar('_T')", "typing_extensions.ParamSpec('_T')"),
|
||||
(
|
||||
"TypeVar('_T')",
|
||||
"ParamSpec('_T')",
|
||||
"TypeVarTuple('_T')",
|
||||
"TypeAliasType('_T', int)",
|
||||
"typing.TypeVar('_T')",
|
||||
"typing_extensions.ParamSpec('_T')",
|
||||
),
|
||||
)
|
||||
def test_a_type_variable_stays_bare_and_a_final_one_trips(tmp_path, declaration):
|
||||
assert "LIT010" not in _codes(tmp_path, f"_T = {declaration}\n")
|
||||
|
|
|
|||
|
|
@ -248,7 +248,10 @@ def test_diff_gate_selects_top_level_and_nested_python_files_only(tmp_path: Path
|
|||
(
|
||||
"tests/e2e_harness/top_level_module.py",
|
||||
"tests/e2e_harness/pkg/sub/nested_module.py",
|
||||
"litellm-proxy-extras/litellm_proxy_extras/pkg/nested_module.py",
|
||||
"pyrightconfig.json",
|
||||
"pyproject.toml",
|
||||
"uv.lock",
|
||||
),
|
||||
)
|
||||
def test_e2e_basedpyright_gate_also_fires_on_harness_python_and_pyrightconfig(tmp_path: Path, trigger: str) -> None:
|
||||
|
|
|
|||
|
|
@ -85,6 +85,10 @@ case "$*" in
|
|||
[ -n "${STUB_ARGS_DIR:-}" ] && echo "$*" >> "$STUB_ARGS_DIR/ruff_tests.args"
|
||||
[ "${STUB_FAIL:-}" = "tests-ruff" ] && exit 1
|
||||
;;
|
||||
"run --no-sync python tests/code_coverage_tests/check_router_constructor_stub.py")
|
||||
[ -n "${STUB_ARGS_DIR:-}" ] && echo "$*" >> "$STUB_ARGS_DIR/router_stub.args"
|
||||
[ "${STUB_FAIL:-}" = "router-stub" ] && exit 1
|
||||
;;
|
||||
esac
|
||||
exit 0
|
||||
"""
|
||||
|
|
@ -421,12 +425,55 @@ def test_run_ends_with_a_summary_of_ran_and_skipped_blocks(tmp_path: Path) -> No
|
|||
assert "ran: Python lint (make lint)" in proc.stdout
|
||||
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 "ran: tests/e2e basedpyright" in proc.stdout
|
||||
assert "skipped: tests/e2e raw HTTP client ban (no tests/e2e 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
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"type_input",
|
||||
(
|
||||
"tests/e2e_harness/collector.py",
|
||||
"litellm-proxy-extras/litellm_proxy_extras/utils.py",
|
||||
"pyrightconfig.json",
|
||||
"pyproject.toml",
|
||||
"uv.lock",
|
||||
),
|
||||
)
|
||||
def test_an_e2e_type_check_input_alone_runs_the_e2e_basedpyright_step(tmp_path: Path, type_input: str) -> None:
|
||||
repo, bin_dir = _sandbox(tmp_path)
|
||||
_commit_all(repo, "base")
|
||||
args_dir = tmp_path / "args"
|
||||
args_dir.mkdir()
|
||||
_stage_file(repo, type_input, "x = 1\n")
|
||||
proc = _run(repo, bin_dir, {"STUB_ARGS_DIR": str(args_dir)})
|
||||
assert proc.returncode == 0, proc.stdout + proc.stderr
|
||||
assert "lint-e2e-basedpyright" in _recorded(args_dir, "make.args")
|
||||
assert "ran: tests/e2e basedpyright" in proc.stdout
|
||||
|
||||
|
||||
@pytest.mark.parametrize(("changed", "runs"), (("litellm/router.py", True), ("litellm/foo.py", False)))
|
||||
def test_the_router_stub_guard_runs_only_when_the_router_changes(tmp_path: Path, changed: str, runs: bool) -> None:
|
||||
repo, bin_dir = _sandbox(tmp_path)
|
||||
_commit_all(repo, "base")
|
||||
args_dir = tmp_path / "args"
|
||||
args_dir.mkdir()
|
||||
_stage_file(repo, changed, "x = 3\n")
|
||||
proc = _run(repo, bin_dir, {"STUB_ARGS_DIR": str(args_dir)})
|
||||
assert proc.returncode == 0, proc.stdout + proc.stderr
|
||||
assert (args_dir / "router_stub.args").exists() is runs
|
||||
|
||||
|
||||
def test_a_failing_router_stub_guard_fails_the_run(tmp_path: Path) -> None:
|
||||
repo, bin_dir = _sandbox(tmp_path)
|
||||
_commit_all(repo, "base")
|
||||
_stage_file(repo, "litellm/router.py", "x = 3\n")
|
||||
proc = _run(repo, bin_dir, {"STUB_FAIL": "router-stub"})
|
||||
assert proc.returncode == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"spec_input",
|
||||
(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue