From 368401e85cd7793771139d3710dcf0b604e8ebb4 Mon Sep 17 00:00:00 2001 From: Joshua Valluru <326636767+joshua-berri@users.noreply.github.com> Date: Sat, 19 Sep 2026 17:03:38 -0700 Subject: [PATCH] test(e2e): complete OAuth triggers and preserve failure diagnostics --- .github/e2e-stack/assert_tests_ran.py | 15 ++++---- .github/workflows/test-mcp-oauth-e2e.yml | 3 ++ .../test_e2e_changed_gate.py | 35 +++++++++++++++++-- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/.github/e2e-stack/assert_tests_ran.py b/.github/e2e-stack/assert_tests_ran.py index 1b051f860cc..3af49007b1e 100644 --- a/.github/e2e-stack/assert_tests_ran.py +++ b/.github/e2e-stack/assert_tests_ran.py @@ -18,11 +18,6 @@ def main() -> int: return 1 cases: Final = tuple(report.iter("testcase")) expected_count: Final = os.environ.get("E2E_REQUIRED_TEST_COUNT") - if expected_count is not None and ( - len(cases) != int(expected_count) or any(case.find("skipped") is not None for case in cases) - ): - _ = sys.stdout.write("::error::required test count was not met or a required case was skipped\n") - return 1 passed: Final = frozenset( case.get("file") for case in cases if all(case.find(tag) is None for tag in ("skipped", "failure", "error")) ) @@ -43,9 +38,10 @@ def main() -> int: skipped: Final = sum(case.get("file") == path and case.find("skipped") is not None for case in cases) _ = sys.stdout.write(f"{path}: {collected} collected, {skipped} skipped\n") for case in cases: - if case.get("file") != path or all(case.find(tag) is None for tag in ("failure", "error")): + if case.get("file") != path or all(case.find(tag) is None for tag in ("failure", "error", "skipped")): continue - _ = sys.stdout.write(f" failed: {case.get('classname', '')}::{case.get('name', '')}\n") + outcome = "skipped" if case.find("skipped") is not None else "failed" + _ = sys.stdout.write(f" {outcome}: {case.get('classname', '')}::{case.get('name', '')}\n") for prop in case.findall("./properties/property"): name = prop.get("name", "") value = prop.get("value", "") @@ -53,6 +49,11 @@ def main() -> int: r"[A-Za-z0-9_.:<>-]{1,240}", value ): _ = sys.stdout.write(f" {name}: {value}\n") + if expected_count is not None and ( + len(cases) != int(expected_count) or any(case.find("skipped") is not None for case in cases) + ): + _ = sys.stdout.write("::error::required test count was not met or a required case was skipped\n") + return 1 if ( selected and not missing diff --git a/.github/workflows/test-mcp-oauth-e2e.yml b/.github/workflows/test-mcp-oauth-e2e.yml index 034b9fe49ec..5c07a68714e 100644 --- a/.github/workflows/test-mcp-oauth-e2e.yml +++ b/.github/workflows/test-mcp-oauth-e2e.yml @@ -12,6 +12,9 @@ on: - 'litellm/experimental_mcp_client/**' - 'litellm/proxy/_experimental/mcp_server/**' - 'litellm/proxy/auth/**' + - 'litellm/proxy/management_endpoints/mcp_management_endpoints.py' + - 'litellm/proxy/_types.py' + - 'litellm/types/mcp_server/mcp_server_manager.py' - 'litellm/proxy/management_endpoints/*sso*.py' - 'litellm/proxy/management_endpoints/sso/**' - 'litellm/proxy/common_utils/encrypt_decrypt_utils.py' diff --git a/tests/code_coverage_tests/test_e2e_changed_gate.py b/tests/code_coverage_tests/test_e2e_changed_gate.py index 707566c0333..e9fa3c5af0b 100644 --- a/tests/code_coverage_tests/test_e2e_changed_gate.py +++ b/tests/code_coverage_tests/test_e2e_changed_gate.py @@ -1,3 +1,4 @@ +import os import subprocess import sys import xml.etree.ElementTree as ET @@ -229,7 +230,10 @@ def test_an_unusable_secret_is_named_without_printing_its_value( @pytest.mark.parametrize("phase", ("setup", "call", "teardown")) -def test_oauth_failure_diagnostics_do_not_publish_private_payloads(tmp_path: Path, phase: str) -> None: +@pytest.mark.parametrize("required_count", ("1", "4")) +def test_oauth_failure_diagnostics_do_not_publish_private_payloads( + tmp_path: Path, phase: str, required_count: str +) -> None: suite: Final = ET.Element("testsuite") case: Final = ET.SubElement(suite, "testcase", file=SELECTED[0]) private: Final = "private-token-in-exception-message" @@ -247,10 +251,37 @@ def test_oauth_failure_diagnostics_do_not_publish_private_payloads(tmp_path: Pat report: Final = tmp_path / "report.xml" ET.ElementTree(suite).write(report) result: Final = subprocess.run( - [sys.executable, "-I", str(GATE), str(report), SELECTED[0]], capture_output=True, text=True + [sys.executable, "-I", str(GATE), str(report), SELECTED[0]], + capture_output=True, + text=True, + env={**os.environ, "E2E_REQUIRED_TEST_COUNT": required_count}, ) assert result.returncode == 1 assert f"oauth_failure_phase: {phase}" in result.stdout assert "oauth_exception_type: AssertionError" in result.stdout assert "oauth_frame: oauth_gateway.py:120:start" in result.stdout assert private not in result.stdout + result.stderr + + +@pytest.mark.parametrize( + ("count", "skip", "expected"), ((0, False, 1), (3, False, 1), (4, False, 0), (5, False, 1), (4, True, 1)) +) +def test_required_count_reports_cases_before_rejecting(tmp_path: Path, count: int, skip: bool, expected: int) -> None: + suite = ET.Element("testsuite") + for index in range(count): + case = ET.SubElement(suite, "testcase", file=SELECTED[0], classname="OAuth", name=f"variant{index}") + if skip and index == 0: + ET.SubElement(case, "skipped", message="private-skip-reason") + report = tmp_path / "report.xml" + ET.ElementTree(suite).write(report) + result = subprocess.run( + [sys.executable, "-I", str(GATE), str(report), SELECTED[0]], + env={**os.environ, "E2E_REQUIRED_TEST_COUNT": "4"}, + capture_output=True, + text=True, + ) + assert result.returncode == expected + assert f"{count} collected, {int(skip)} skipped" in result.stdout + if skip: + assert "skipped: OAuth::variant0" in result.stdout + assert "private-skip-reason" not in result.stdout + result.stderr