diff --git a/.github/risk-tiers.yml b/.github/risk-tiers.yml index afec788e0aa..51110e058c6 100644 --- a/.github/risk-tiers.yml +++ b/.github/risk-tiers.yml @@ -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]" diff --git a/.github/scripts/risk_tier.py b/.github/scripts/risk_tier.py index fa448eec704..2996f780664 100644 --- a/.github/scripts/risk_tier.py +++ b/.github/scripts/risk_tier.py @@ -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) diff --git a/tests/test_litellm/test_github_risk_tier.py b/tests/test_litellm/test_github_risk_tier.py index c09432df621..722deb0603b 100644 --- a/tests/test_litellm/test_github_risk_tier.py +++ b/tests/test_litellm/test_github_risk_tier.py @@ -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"),