mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(ci): stop the diff scope from selecting the whole unit suite
Timing the gate against three real open pull requests turned up two ways the test selection went wide. A private module never matched its test file, since litellm/_redis.py globs test__redis*.py while the file is test_redis.py, and the miss fell through to the mirror directory. For a module at the top of litellm/ that directory is tests/test_litellm itself, so every mutant would have run the entire unit suite. The same fallback pulled all of tests/test_litellm/caching into a caching diff and picked up an unrelated order-dependent s3 test, which fails on an unmutated tree and aborts the run before a single mutant executes. Private modules now also try the underscore-stripped spelling, and the directory fallback stops at the mirror root instead of returning it. Overload stubs are deduplicated too: they repeat the implementation's name, so a touched signature asked for the same function three times and burned three slots of --max-functions.
This commit is contained in:
parent
ab5232eee0
commit
e810dfa827
2 changed files with 76 additions and 4 deletions
|
|
@ -143,13 +143,24 @@ def changed_test_files(root: Path, diff: str) -> tuple[str, ...]:
|
|||
|
||||
|
||||
def mapped_tests(root: Path, path: str) -> tuple[str, ...]:
|
||||
"""Find the tests mirroring a production file, per tests/test_litellm/readme.md."""
|
||||
"""Find the tests mirroring a production file, per tests/test_litellm/readme.md.
|
||||
|
||||
A private module drops its leading underscores on the way to a test name, so
|
||||
``litellm/_redis.py`` is covered by ``test_redis.py``. Missing that spelling used
|
||||
to fall through to the directory, and for a module at the top of ``litellm/`` the
|
||||
directory is the entire unit suite: every mutant would run all of it.
|
||||
"""
|
||||
relative: Final = Path(path).relative_to("litellm")
|
||||
mirror_dir: Final = Path(TEST_MIRROR_ROOT) / relative.parent
|
||||
named: Final = tuple(sorted(str(p.relative_to(root)) for p in (root / mirror_dir).glob(f"test_{relative.stem}*.py")))
|
||||
stems: Final = (relative.stem, relative.stem.lstrip("_"))
|
||||
named: Final = tuple(
|
||||
sorted({str(p.relative_to(root)) for stem in stems for p in (root / mirror_dir).glob(f"test_{stem}*.py")})
|
||||
)
|
||||
if named:
|
||||
return named
|
||||
return (str(mirror_dir),) if (root / mirror_dir).is_dir() else ()
|
||||
if mirror_dir == Path(TEST_MIRROR_ROOT) or not (root / mirror_dir).is_dir():
|
||||
return ()
|
||||
return (str(mirror_dir),)
|
||||
|
||||
|
||||
def trampoline_units(source: str) -> tuple[TrampolineUnit, ...]:
|
||||
|
|
@ -200,7 +211,8 @@ def build_scope(root: Path, base: str, max_functions: int) -> Scope:
|
|||
merge_base: Final = _git(root, "merge-base", base, "HEAD").strip()
|
||||
diff: Final = _git(root, "diff", "-U0", merge_base, "--", ".")
|
||||
files: Final = parse_unified_diff(root, diff)
|
||||
all_globs: Final = tuple(glob for changed in files for glob in globs_for(root, changed))
|
||||
# `@overload` stubs repeat their implementation's name, and mutmut mangles them all the same way.
|
||||
all_globs: Final = tuple(dict.fromkeys(glob for changed in files for glob in globs_for(root, changed)))
|
||||
tests: Final = tuple(
|
||||
sorted({*(t for changed in files for t in mapped_tests(root, changed.path)), *changed_test_files(root, diff)})
|
||||
)
|
||||
|
|
|
|||
|
|
@ -94,6 +94,35 @@ def coerce(value: str) -> str:
|
|||
return value
|
||||
'''
|
||||
|
||||
REDIS = '''\
|
||||
def get_redis_client(url: str) -> str:
|
||||
return url
|
||||
'''
|
||||
|
||||
OVERLOADED = '''\
|
||||
from typing import overload
|
||||
|
||||
|
||||
@overload
|
||||
def embedding(
|
||||
model: str,
|
||||
stream: bool,
|
||||
) -> str: ...
|
||||
|
||||
|
||||
@overload
|
||||
def embedding(
|
||||
model: str,
|
||||
) -> str: ...
|
||||
|
||||
|
||||
def embedding(
|
||||
model: str,
|
||||
stream: bool = False,
|
||||
) -> str:
|
||||
return model
|
||||
'''
|
||||
|
||||
|
||||
def _run(root: Path, *args: str) -> None:
|
||||
subprocess.run(("git", *args), cwd=root, check=True, capture_output=True)
|
||||
|
|
@ -116,6 +145,11 @@ def repo(tmp_path: Path) -> Path:
|
|||
_write(root, "litellm/router.py", ROUTER)
|
||||
_write(root, "litellm/proxy/auth/auth_checks.py", AUTH_CHECKS)
|
||||
_write(root, "litellm/types/utils.py", TYPES)
|
||||
_write(root, "litellm/_redis.py", REDIS)
|
||||
_write(root, "litellm/_unmapped.py", REDIS)
|
||||
_write(root, "litellm/main.py", OVERLOADED)
|
||||
_write(root, "tests/test_litellm/test_redis.py", "def test_redis(): pass\n")
|
||||
_write(root, "tests/test_litellm/test_main.py", "def test_main(): pass\n")
|
||||
_write(root, "tests/test_litellm/test_router.py", "def test_router(): pass\n")
|
||||
_write(root, "tests/test_litellm/proxy/auth/test_auth_checks.py", "def test_auth(): pass\n")
|
||||
_write(root, "tests/e2e/test_live_proxy.py", "def test_live(): pass\n")
|
||||
|
|
@ -238,6 +272,32 @@ def test_test_selection_is_the_mirrored_file_plus_changed_unit_tests(repo: Path)
|
|||
)
|
||||
|
||||
|
||||
def test_private_module_maps_to_its_test_file_without_the_underscore(repo: Path) -> None:
|
||||
"""`litellm/_redis.py` is covered by `test_redis.py`, not `test__redis.py`."""
|
||||
_write(repo, "litellm/_redis.py", REDIS.replace("return url", "return url.strip()"))
|
||||
_commit(repo)
|
||||
|
||||
assert scope_of(repo).tests == ("tests/test_litellm/test_redis.py",)
|
||||
|
||||
|
||||
def test_an_unmapped_top_level_module_never_selects_the_whole_unit_suite(repo: Path) -> None:
|
||||
"""Falling back to tests/test_litellm would run every unit test once per mutant."""
|
||||
_write(repo, "litellm/_unmapped.py", REDIS.replace("return url", "return url.strip()"))
|
||||
_commit(repo)
|
||||
|
||||
scope = scope_of(repo)
|
||||
assert scope.globs, "the changed function still has to be reported"
|
||||
assert scope.tests == ()
|
||||
|
||||
|
||||
def test_overloaded_function_is_requested_once(repo: Path) -> None:
|
||||
"""@overload stubs share the implementation's mangled name, so repeating it just burns the cap."""
|
||||
_write(repo, "litellm/main.py", OVERLOADED.replace(" model: str,", " model: str | None,"))
|
||||
_commit(repo)
|
||||
|
||||
assert scope_of(repo).globs == ("litellm.main.x_embedding__mutmut_*",)
|
||||
|
||||
|
||||
def test_e2e_tests_are_excluded_from_the_selection(repo: Path) -> None:
|
||||
"""tests/e2e needs a live proxy; including one aborts the run at mutmut's clean-test check."""
|
||||
_write(repo, "litellm/router.py", ROUTER.replace("if m]", "if m and m.strip()]"))
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue