mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-10 22:41:41 +00:00
fix(ci): tier the risk floor by fork only and stop counting conditional skips
The author factor now only raises a PR to high when it comes from a fork, so an internal human-opened docs or test-only PR can reach the low tier the one-pager's tier 0 needs. Over the last 400 staging merges that moves the low count from 2 to 29 with no reverted merge among them Skip markers only count when they silence a test unconditionally: pytest.mark.skipif, unittest.skipIf/skipUnless, a guarded pytest.skip(), and Playwright's conditional test.skip(cond, reason) no longer rank a PR high. Test directories anywhere in the tree count as test files, and the enterprise auth and management endpoint packages join the always-human paths
This commit is contained in:
parent
9d5066df6b
commit
fe45044035
3 changed files with 65 additions and 21 deletions
8
.github/risk-tiers.yml
vendored
8
.github/risk-tiers.yml
vendored
|
|
@ -17,6 +17,8 @@ paths:
|
|||
- "litellm/proxy/custom_hooks/**"
|
||||
- "enterprise/enterprise_hooks/**"
|
||||
- "enterprise/litellm_enterprise/proxy/hooks/**"
|
||||
- "enterprise/litellm_enterprise/proxy/auth/**"
|
||||
- "enterprise/litellm_enterprise/proxy/management_endpoints/**"
|
||||
- "litellm/secret_managers/**"
|
||||
- "litellm/proxy/pass_through_endpoints/**"
|
||||
- "litellm/passthrough/**"
|
||||
|
|
@ -52,11 +54,7 @@ size:
|
|||
|
||||
tests:
|
||||
files:
|
||||
- "tests/**"
|
||||
- "**/tests/**"
|
||||
- "**/*.test.*"
|
||||
- "**/*.spec.*"
|
||||
- "**/__tests__/**"
|
||||
|
||||
authors:
|
||||
low:
|
||||
- "devin-ai-integration[bot]"
|
||||
|
|
|
|||
18
.github/scripts/risk_tier.py
vendored
18
.github/scripts/risk_tier.py
vendored
|
|
@ -17,7 +17,9 @@ from pydantic import BaseModel, ConfigDict, Field
|
|||
Tier = Literal["low", "medium", "high"]
|
||||
|
||||
TEST_DEF_RE: Final = re.compile(r"^\s*(?:async\s+)?def\s+test_|^\s*(?:it|test)\(")
|
||||
SKIP_RE: Final = re.compile(r"pytest\.(?:mark\.)?(?:skip|xfail)|unittest\.skip|\b(?:it|test|describe)\.skip\(|\bxit\(")
|
||||
SKIP_RE: Final = re.compile(
|
||||
r"pytest\.mark\.(?:skip(?!if)|xfail)|unittest\.skip\b|\b(?:it|test|describe)\.skip\(\s*[\"'`]|\bx(?:it|test|describe)\("
|
||||
)
|
||||
ASSERT_RE: Final = re.compile(r"^\s*assert\b|\bexpect\(")
|
||||
GLOB_TOKEN_RE: Final = re.compile(r"(\*\*/|\*\*|\*|\?)")
|
||||
DIFF_BLOCK_SEPARATOR: Final = "\ndiff --git "
|
||||
|
|
@ -58,18 +60,12 @@ class TestRules(BaseModel):
|
|||
files: tuple[str, ...]
|
||||
|
||||
|
||||
class AuthorRules(BaseModel):
|
||||
model_config = ConfigDict(extra="forbid", frozen=True)
|
||||
low: tuple[str, ...]
|
||||
|
||||
|
||||
class RiskConfig(BaseModel):
|
||||
model_config = ConfigDict(extra="forbid", frozen=True)
|
||||
paths: PathRules
|
||||
modules: ModuleRules
|
||||
size: SizeRules
|
||||
tests: TestRules
|
||||
authors: AuthorRules
|
||||
|
||||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
|
|
@ -291,12 +287,10 @@ def tests_factor(changes: Sequence[FileChange], rules: Rules) -> Factor:
|
|||
return Factor("tests", "medium", "production code changed with no test touched")
|
||||
|
||||
|
||||
def author_factor(author: str, from_fork: bool, rules: Rules) -> Factor:
|
||||
def author_factor(author: str, from_fork: bool) -> Factor:
|
||||
if from_fork:
|
||||
return Factor("author", "high", f"`{author}` from a fork")
|
||||
if author in rules.config.authors.low:
|
||||
return Factor("author", "low", f"`{author}` on an internal branch")
|
||||
return Factor("author", "medium", f"`{author}` on an internal branch, human-opened")
|
||||
return Factor("author", "low", f"`{author}` on an internal branch")
|
||||
|
||||
|
||||
def classify(changes: Sequence[FileChange], author: str, from_fork: bool, rules: Rules) -> Verdict:
|
||||
|
|
@ -305,7 +299,7 @@ def classify(changes: Sequence[FileChange], author: str, from_fork: bool, rules:
|
|||
modules_factor(changes, rules),
|
||||
size_factor(changes, rules),
|
||||
tests_factor(changes, rules),
|
||||
author_factor(author, from_fork, rules),
|
||||
author_factor(author, from_fork),
|
||||
)
|
||||
return Verdict(highest([factor.tier for factor in factors]), factors)
|
||||
|
||||
|
|
|
|||
|
|
@ -159,9 +159,40 @@ def test_removed_test_function_is_high(risk_tier, rules):
|
|||
assert _factor(verdict, "tests").reason == "1 test(s) removed"
|
||||
|
||||
|
||||
def test_added_skip_marker_is_high(risk_tier, rules):
|
||||
diff = _file_diff("tests/test_litellm/test_a.py", added=('@pytest.mark.skip(reason="flaky")',))
|
||||
assert _factor(_verdict(risk_tier, rules, diff), "tests").tier == "high"
|
||||
@pytest.mark.parametrize(
|
||||
"marker",
|
||||
[
|
||||
'@pytest.mark.skip(reason="flaky")',
|
||||
"@pytest.mark.skip",
|
||||
'pytestmark = pytest.mark.skip("whole module is flaky")',
|
||||
'@pytest.mark.xfail(reason="broken since the refactor")',
|
||||
'@unittest.skip("flaky")',
|
||||
],
|
||||
)
|
||||
def test_unconditional_skip_marker_is_high(risk_tier, rules, marker):
|
||||
diff = _file_diff("tests/test_litellm/test_a.py", added=(marker, *NEW_TEST))
|
||||
factor = _factor(_verdict(risk_tier, rules, diff), "tests")
|
||||
assert factor.tier == "high"
|
||||
assert factor.reason == "1 skip marker(s) added"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"guard",
|
||||
[
|
||||
'@pytest.mark.skipif(not os.getenv("OPENAI_API_KEY"), reason="needs a real key")',
|
||||
' if not os.getenv("OPENAI_API_KEY"):',
|
||||
' pytest.skip("needs a real key")',
|
||||
'@unittest.skipIf(sys.platform == "win32", "posix only")',
|
||||
'@unittest.skipUnless(HAS_REDIS, "needs redis")',
|
||||
],
|
||||
)
|
||||
def test_conditional_skip_is_not_a_silenced_test(risk_tier, rules, guard):
|
||||
diff = _file_diff("tests/test_litellm/test_a.py", added=(guard, *NEW_TEST)) + _file_diff(
|
||||
"litellm/llms/anthropic/chat/x.py", added=("x = 1",)
|
||||
)
|
||||
factor = _factor(_verdict(risk_tier, rules, diff), "tests")
|
||||
assert factor.tier == "low"
|
||||
assert factor.reason == "1 test(s) added"
|
||||
|
||||
|
||||
def test_weakened_assertions_are_high(risk_tier, rules):
|
||||
|
|
@ -199,6 +230,20 @@ def test_production_change_without_any_test_is_medium(risk_tier, rules):
|
|||
[
|
||||
(('it("hides the notice", () => {', " expect(screen.queryByText(notice)).toBeNull();", "});"), "low"),
|
||||
(('it.skip("hides the notice", () => {', "});"), "high"),
|
||||
(("test.skip('hides the notice', async () => {", "});"), "high"),
|
||||
(('xit("hides the notice", () => {', "});"), "high"),
|
||||
(
|
||||
(
|
||||
'test("hides the notice", async ({ page }) => {',
|
||||
" test.skip(!process.env.UI_BASE_URL, 'needs a UI');",
|
||||
"});",
|
||||
),
|
||||
"low",
|
||||
),
|
||||
(
|
||||
('test("hides the notice", async ({ page }) => {', " if (!process.env.UI_BASE_URL) test.skip();", "});"),
|
||||
"low",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_typescript_tests_count_like_python_ones(risk_tier, rules, added, expected):
|
||||
|
|
@ -212,7 +257,7 @@ def test_typescript_tests_count_like_python_ones(risk_tier, rules, added, expect
|
|||
("author", "from_fork", "expected"),
|
||||
[
|
||||
(DEVIN, False, "low"),
|
||||
("mateo-berri", False, "medium"),
|
||||
("mateo-berri", False, "low"),
|
||||
(DEVIN, True, "high"),
|
||||
("jairandresdiazp", True, "high"),
|
||||
],
|
||||
|
|
@ -235,12 +280,19 @@ def test_author_factor(risk_tier, rules, author, from_fork, expected):
|
|||
("scripts/type_check_gate.py", "high"),
|
||||
(".github/risk-tiers.yml", "high"),
|
||||
("enterprise/litellm_enterprise/proxy/hooks/x.py", "high"),
|
||||
("enterprise/litellm_enterprise/proxy/auth/x.py", "high"),
|
||||
("enterprise/litellm_enterprise/proxy/management_endpoints/x.py", "high"),
|
||||
("litellm/proxy/pass_through_endpoints/x.py", "high"),
|
||||
("litellm/llms/bedrock/passthrough/x.py", "medium"),
|
||||
("ui/litellm-dashboard/src/components/networking.tsx", "medium"),
|
||||
("litellm/types/proxy/x.py", "medium"),
|
||||
("ui/litellm-dashboard/src/app/login/LoginPage.test.tsx", "low"),
|
||||
("tests/e2e/x.py", "low"),
|
||||
("litellm-proxy-extras/tests/test_x.py", "low"),
|
||||
("helm/litellm-helm/tests/x.yaml", "low"),
|
||||
("ui/litellm-dashboard/tests/x.spec.ts", "low"),
|
||||
("litellm-rust/crates/core/tests/x.rs", "low"),
|
||||
("enterprise/litellm_enterprise/proxy/common_utils/x.py", "medium"),
|
||||
("cookbook/x.ipynb", "low"),
|
||||
("model_prices_and_context_window.json", "low"),
|
||||
("litellm/model_prices_and_context_window_backup.json", "low"),
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue