mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-08 22:21:35 +00:00
ci(lint): gate top-level tests/e2e and litellm files in the diff-scoped lint steps
Without :(glob), git matches 'tests/e2e/**/*.py' with * crossing slashes, so the pattern needs at least one directory below tests/e2e and a top-level file never matches. PR #39209 added tests/e2e/test_junit_properties.py with three basedpyright errors and the e2e step printed "No changed tests/e2e Python files; skipping." The ruff format step's 'litellm/**/*.py' skipped litellm/main.py and the other top-level modules the same way. :(glob) makes /**/ match zero or more directories, so both gates now select top-level and nested files alike. A regression test runs the workflow's own pathspecs against a throwaway repo and locks that in for every diff-scoped gate.
This commit is contained in:
parent
e2c3f51c46
commit
b0aafabdee
3 changed files with 58 additions and 4 deletions
4
.github/workflows/test-linting.yml
vendored
4
.github/workflows/test-linting.yml
vendored
|
|
@ -113,7 +113,7 @@ jobs:
|
|||
- name: Check ruff format
|
||||
if: steps.changes.outputs.decision != 'skip'
|
||||
run: |
|
||||
git diff --name-only --diff-filter=ACMR "$GATE_BASE_SHA" HEAD -- 'litellm/**/*.py' | grep -v '^litellm/enterprise/' > "$RUNNER_TEMP/ruff_format_files.txt" || true
|
||||
git diff --name-only --diff-filter=ACMR "$GATE_BASE_SHA" HEAD -- ':(glob)litellm/**/*.py' | grep -v '^litellm/enterprise/' > "$RUNNER_TEMP/ruff_format_files.txt" || true
|
||||
if [ ! -s "$RUNNER_TEMP/ruff_format_files.txt" ]; then
|
||||
echo "No changed litellm Python files to check with ruff format."
|
||||
exit 0
|
||||
|
|
@ -172,7 +172,7 @@ 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 -- 'tests/e2e/**/*.py' | grep -q .; then
|
||||
if git diff --name-only --diff-filter=ACMRD "$GATE_BASE_SHA" HEAD -- ':(glob)tests/e2e/**/*.py' | grep -q .; then
|
||||
uv run --no-sync basedpyright tests/e2e
|
||||
else
|
||||
echo "No changed tests/e2e Python files; skipping."
|
||||
|
|
|
|||
4
Makefile
4
Makefile
|
|
@ -147,8 +147,8 @@ lint-install:
|
|||
# Diff-scoped format check, mirroring test-linting.yml's "Check ruff format" step:
|
||||
# only the litellm Python files changed vs the base are checked, so a pre-existing
|
||||
# format issue elsewhere doesn't block an unrelated commit. Git pathspecs match
|
||||
# recursively, so 'litellm/*.py' covers nested modules and the top-level files that
|
||||
# CI's 'litellm/**/*.py' skips, which makes this target a superset of the CI step.
|
||||
# recursively, so 'litellm/*.py' covers top-level files and nested modules alike,
|
||||
# the same set CI's ':(glob)litellm/**/*.py' selects.
|
||||
lint-format-check-changed: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE)
|
||||
@files=$$(git diff --name-only --diff-filter=ACMR origin/litellm_internal_staging...HEAD -- 'litellm/*.py' | grep -v '^litellm/enterprise/' || true); \
|
||||
if [ -z "$$files" ]; then \
|
||||
|
|
|
|||
54
tests/test_litellm/test_lint_workflow_diff_gates.py
Normal file
54
tests/test_litellm/test_lint_workflow_diff_gates.py
Normal file
|
|
@ -0,0 +1,54 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import shlex
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from typing import Final
|
||||
|
||||
import pytest
|
||||
|
||||
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 -- (.+?) \|')
|
||||
GATES: Final = tuple(tuple(shlex.split(gate.group(1))) for gate in DIFF_GATE.finditer(WORKFLOW.read_text()))
|
||||
|
||||
|
||||
def _git(cwd: Path, *args: str) -> str:
|
||||
return subprocess.run(["git", *args], cwd=cwd, check=True, capture_output=True, text=True).stdout
|
||||
|
||||
|
||||
def _scoped_root(pathspec: str) -> str:
|
||||
return re.sub(r"^:\([^)]*\)", "", pathspec).split("*", 1)[0]
|
||||
|
||||
|
||||
def _changed_files_selected_by(tmp_path: Path, pathspecs: tuple[str, ...], files: tuple[str, ...]) -> frozenset[str]:
|
||||
_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")
|
||||
for name in files:
|
||||
target = tmp_path / name
|
||||
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")
|
||||
return frozenset(
|
||||
_git(tmp_path, "diff", "--name-only", "--diff-filter=ACMRD", "HEAD~1", "HEAD", "--", *pathspecs).split()
|
||||
)
|
||||
|
||||
|
||||
def test_workflow_still_carries_the_ruff_format_and_e2e_basedpyright_diff_gates() -> None:
|
||||
assert frozenset(_scoped_root(gate[0]) for gate in GATES) == frozenset({"litellm/", "tests/e2e/"})
|
||||
|
||||
|
||||
@pytest.mark.parametrize("pathspecs", GATES, ids=" ".join)
|
||||
def test_diff_gate_selects_top_level_and_nested_python_files_only(tmp_path: Path, pathspecs: tuple[str, ...]) -> None:
|
||||
root = _scoped_root(pathspecs[0])
|
||||
top_level = f"{root}top_level_module.py"
|
||||
nested = f"{root}pkg/sub/nested_module.py"
|
||||
selected = _changed_files_selected_by(
|
||||
tmp_path,
|
||||
pathspecs,
|
||||
(top_level, nested, f"{root}notes.md", "elsewhere/top_level_module.py", "elsewhere/pkg/nested_module.py"),
|
||||
)
|
||||
assert selected == frozenset({top_level, nested})
|
||||
Loading…
Add table
Reference in a new issue