diff --git a/.github/workflows/test-linting.yml b/.github/workflows/test-linting.yml index 89ca60ad9d1..bd7343ea222 100644 --- a/.github/workflows/test-linting.yml +++ b/.github/workflows/test-linting.yml @@ -376,10 +376,10 @@ jobs: exit 0 fi files="$(git diff --name-only "$base" HEAD)" - if grep -Eq '^(litellm/(proxy|types)/|ui/litellm-dashboard/(src/lib/http/schema\.d\.ts|scripts/gen-api-types\.mjs|package(-lock)?\.json)$|\.github/workflows/test-linting\.yml$)' <<< "$files"; then + if grep -Eq '^(litellm/|enterprise/|(pyproject\.toml|uv\.lock)$|ui/litellm-dashboard/(src/lib/http/schema\.d\.ts|scripts/gen-api-types\.mjs|package(-lock)?\.json)$|\.github/workflows/test-linting\.yml$)' <<< "$files"; then echo "relevant=true" >> "$GITHUB_OUTPUT" else - echo "No proxy, types or generator changes in this pull request, nothing to verify." + echo "No litellm, enterprise, dependency or generator changes in this pull request, nothing to verify." echo "relevant=false" >> "$GITHUB_OUTPUT" fi diff --git a/.github/workflows/test-terraform-provider.yml b/.github/workflows/test-terraform-provider.yml index ff9db13bd25..6aeb66ed37c 100644 --- a/.github/workflows/test-terraform-provider.yml +++ b/.github/workflows/test-terraform-provider.yml @@ -12,6 +12,7 @@ on: paths: - "terraform/provider/**" - "litellm/proxy/**" + - "enterprise/**" - ".github/workflows/test-terraform-provider.yml" permissions: diff --git a/scripts/pre_commit_lint.sh b/scripts/pre_commit_lint.sh index 371884fb791..95b5942fa8b 100755 --- a/scripts/pre_commit_lint.sh +++ b/scripts/pre_commit_lint.sh @@ -18,7 +18,8 @@ # -> ruff over ruff-tests.toml + `make lint-test-quality` (test-linting.yml's # test-tree ruff and test-quality gate steps) # - dashboard -> prettier + eslint + lint budgets (test-linting.yml's ui job) -# - proxy/types -> regenerate the lazy OpenAPI snapshot and dashboard API types, fail on drift (test-linting.yml's ui-api-types job) +# - spec_pattern -> regenerate the lazy OpenAPI snapshot and dashboard API types, fail on drift +# (test-linting.yml's ui-api-types job) # # Each block is skipped when no matching files are in scope, so unrelated commits # stay fast. This is intentionally not auto-installed as a git hook (see @@ -98,7 +99,7 @@ existing_files() { litellm_py_pattern='^litellm/.*\.py$' e2e_py_pattern='^tests/e2e(_harness)?/.*\.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))$' +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)$' ui_eslint_pattern='^ui/litellm-dashboard/.*\.(js|jsx|ts|tsx|mjs|cjs)$' @@ -110,9 +111,6 @@ e2e_py_files=$(scope_match "$e2e_py_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) -# test-linting.yml triggers on any file under litellm/proxy or litellm/types -# (Prisma schema and configs included, not just Python) plus the generator and its -# lockfiles, so match that whole trigger set rather than a Python subset. spec_files=$(scope_match "$spec_pattern") # CI's ui job runs prettier over a wider extension set than eslint; keep that # split so this flags exactly what the job would. @@ -337,7 +335,7 @@ summary_item "tests/e2e checks (basedpyright + raw HTTP client ban)" "$e2e_py_fi 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" +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 echo "check: NOTE - no gating lint check matches the files in scope, so nothing ran:" >&2 diff --git a/tests/unit/test_lint_workflow_diff_gates.py b/tests/unit/test_lint_workflow_diff_gates.py index 7b1d7802d53..5d3c0d80aa6 100644 --- a/tests/unit/test_lint_workflow_diff_gates.py +++ b/tests/unit/test_lint_workflow_diff_gates.py @@ -1,5 +1,6 @@ from __future__ import annotations +import fnmatch import re import shlex import subprocess @@ -7,6 +8,7 @@ from pathlib import Path from typing import Final import pytest +import yaml WORKFLOW: Final = Path(__file__).resolve().parents[2] / ".github" / "workflows" / "test-linting.yml" DIFF_GATE: Final = re.compile(r'git diff --name-only --diff-filter=\w+ "\$GATE_BASE_SHA" HEAD -- (.+?) \|') @@ -100,3 +102,101 @@ def test_e2e_harness_gate_fires_on_any_e2e_or_harness_file_its_dependency_manife (trigger, "tests/e2e/ui/spec.ts", "tests/e2e/ui/pkg/page.py", "elsewhere/pyproject.toml", "tests/e2e_other/module.py"), ) assert selected == frozenset({trigger}) + + +UI_API_TYPES_DETECT: Final[str] = next( + step["run"] + for step in yaml.safe_load(WORKFLOW.read_text())["jobs"]["ui-api-types"]["steps"] + if step.get("id") == "changes" +) +PRE_COMMIT_LINT: Final = WORKFLOW.parents[2] / "scripts" / "pre_commit_lint.sh" +TERRAFORM_PROVIDER_WORKFLOW: Final = WORKFLOW.with_name("test-terraform-provider.yml") + + +def _ui_api_types_relevant(tmp_path: Path, changed: str) -> bool: + _git(tmp_path, "init", "-q", "-b", "main") + _git(tmp_path, "config", "user.email", "t@t") + _git(tmp_path, "config", "user.name", "t") + _git(tmp_path, "commit", "-q", "--allow-empty", "-m", "base") + _git(tmp_path, "switch", "-q", "-c", "pr") + target: Final = tmp_path / changed + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("x = 1\n") + _git(tmp_path, "add", "-A") + _git(tmp_path, "commit", "-qm", "change") + _git(tmp_path, "switch", "-q", "main") + _git(tmp_path, "commit", "-q", "--allow-empty", "-m", "main moved") + _git(tmp_path, "merge", "-q", "--no-ff", "--no-edit", "pr") + output: Final = tmp_path / "github_output" + subprocess.run( + ["bash", "-c", UI_API_TYPES_DETECT], + cwd=tmp_path, + check=True, + capture_output=True, + env={"PATH": "/usr/bin:/bin", "GITHUB_OUTPUT": str(output)}, + ) + return output.read_text().strip() == "relevant=true" + + +@pytest.mark.parametrize( + "changed", + ( + "litellm/proxy/proxy_server.py", + "litellm/types/router.py", + "litellm/tracing/generated/models.py", + "litellm/router_strategy/complexity_router/config.py", + "litellm/models/credentials.py", + "enterprise/litellm_enterprise/types/proxy/audit_logging_endpoints.py", + "pyproject.toml", + "uv.lock", + "ui/litellm-dashboard/scripts/gen-api-types.mjs", + "ui/litellm-dashboard/package-lock.json", + "ui/litellm-dashboard/src/lib/http/schema.d.ts", + ), +) +def test_ui_api_types_check_runs_on_a_pull_request_touching_any_spec_input(tmp_path: Path, changed: str) -> None: + assert _ui_api_types_relevant(tmp_path, changed) + + +@pytest.mark.parametrize( + "changed", + ( + "litellm-rust/crates/traces/src/lib.rs", + "litellm-proxy-extras/litellm_proxy_extras/utils.py", + "tests/unit/test_router.py", + "ui/litellm-dashboard/src/app/page.tsx", + "docs/my-website/docs/proxy/config.md", + "litellm-dashboard/pyproject.toml", + ), +) +def test_ui_api_types_check_skips_a_pull_request_that_cannot_change_the_spec(tmp_path: Path, changed: str) -> None: + assert not _ui_api_types_relevant(tmp_path, changed) + + +def _first_single_quoted_capture(pattern: str, text: str) -> str: + match: Final = re.search(pattern, text, re.MULTILINE) + assert match is not None, pattern + return match.group(1) + + +def _tracked_paths_matching(extended_regex: str) -> frozenset[str]: + tracked: Final = _git(WORKFLOW.parents[2], "ls-files") + selected: Final = subprocess.run(["grep", "-E", extended_regex], input=tracked, capture_output=True, text=True) + return frozenset(selected.stdout.splitlines()) + + +def test_make_check_runs_the_api_type_sync_on_the_same_tracked_files_as_ci() -> None: + ci_trigger: Final = _first_single_quoted_capture(r"grep -Eq '([^']+)'", UI_API_TYPES_DETECT) + make_check_trigger: Final = _first_single_quoted_capture(r"^spec_pattern='([^']+)'$", PRE_COMMIT_LINT.read_text()) + ci_selected: Final = _tracked_paths_matching(ci_trigger) + assert "pyproject.toml" in ci_selected + assert _tracked_paths_matching(make_check_trigger) == ci_selected - {".github/workflows/test-linting.yml"} + + +def test_a_pull_request_changing_any_route_file_runs_the_terraform_provider_endpoint_audit() -> None: + triggers: Final = yaml.safe_load(TERRAFORM_PROVIDER_WORKFLOW.read_text())[True]["pull_request"]["paths"] + route_files: Final = _git( + WORKFLOW.parents[2], "grep", "-l", "-E", r"APIRouter\(", "--", "*.py", ":(exclude)tests/**" + ).splitlines() + assert route_files + assert [name for name in route_files if not any(fnmatch.fnmatch(name, pattern) for pattern in triggers)] == [] diff --git a/tests/unit/test_pre_commit_lint.py b/tests/unit/test_pre_commit_lint.py index 7c9b98c1c94..ef84ab9fbc8 100644 --- a/tests/unit/test_pre_commit_lint.py +++ b/tests/unit/test_pre_commit_lint.py @@ -427,6 +427,34 @@ def test_run_ends_with_a_summary_of_ran_and_skipped_blocks(tmp_path: Path) -> No assert "check: FAIL" not in proc.stdout +@pytest.mark.parametrize( + "spec_input", + ( + "litellm/tracing/generated/models.py", + "litellm/models/credentials.py", + "enterprise/litellm_enterprise/types/proxy/audit_logging_endpoints.py", + "pyproject.toml", + "uv.lock", + ), +) +def test_a_staged_spec_input_outside_proxy_and_types_runs_the_api_type_sync(tmp_path: Path, spec_input: str) -> None: + repo, bin_dir = _sandbox(tmp_path) + _commit_all(repo, "base") + _stage_file(repo, spec_input, "x = 1\n") + proc = _run(repo, bin_dir, {}) + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "ran: dashboard API-type sync (npm run gen:api)" in proc.stdout + + +def test_a_staged_rust_crate_change_skips_the_api_type_sync(tmp_path: Path) -> None: + repo, bin_dir = _sandbox(tmp_path) + _commit_all(repo, "base") + _stage_file(repo, "litellm-rust/crates/traces/src/lib.rs", "fn main() {}\n") + proc = _run(repo, bin_dir, {}) + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "skipped: dashboard API-type sync (npm run gen:api)" in proc.stdout + + 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") diff --git a/ui/litellm-dashboard/AGENTS.md b/ui/litellm-dashboard/AGENTS.md index 3453a144e89..274dcff1f48 100644 --- a/ui/litellm-dashboard/AGENTS.md +++ b/ui/litellm-dashboard/AGENTS.md @@ -4,7 +4,7 @@ Never put LiteLLM tokens or API keys in `localStorage`. `localStorage` survives When you fix lint violations that are grandfathered in `eslint-suppressions.json`, run `eslint . --prune-suppressions` and commit the updated baseline so the gate ratchets down instead of leaving a stale suppression -`src/lib/http/schema.d.ts` is generated from the proxy's OpenAPI spec. Never hand-edit it. After changing a backend route or response model that the dashboard consumes, run `npm run gen:api` and commit the result (CI `lint / ui-api-types` enforces this) +`src/lib/http/schema.d.ts` is generated from the proxy's OpenAPI spec. Never hand-edit it. After changing any backend route or request or response model, or bumping a dependency, run `npm run gen:api` and commit the result, since the spec includes every route, not only the ones the dashboard calls (CI `lint / ui-api-types` enforces this) Tests come in three tiers, named by the standard definitions. `Foo.test.tsx` is a unit test: one module, collaborators replaced by doubles, no multi-component tree, and it should run in milliseconds. `Foo.integration.test.tsx` renders a real component tree with real children and only stubs the network boundary. It costs seconds per case, so it earns its place by proving wiring that a unit test cannot reach. Browser-level tests live in `tests/e2e/ui/` as Playwright specs against a live proxy