From d1933c92c036eb37aa3b53f78342401de6111129 Mon Sep 17 00:00:00 2001 From: Kerry Lu Date: Wed, 11 Nov 2026 17:39:10 -0800 Subject: [PATCH] ci(unit): split proxy-infra-root in two, isolate subprocess tests, make its file exclusions take effect (#51316) pytest --ignore is a no-op for a file the shell glob already expanded, so the 25 files proxy-infra-root listed under --ignore still ran there as well as in their own shards (805 duplicate tests, 18% of the shard's time). Switch them to --deselect, which pytest applies to explicit arguments, and teach assert_ci_coverage.py to read --deselect and [..] character-class globs. Split the rest by first letter into proxy-infra-root-a-p and proxy-infra-root-q-z, and give the three files that spawn a child python (test_component_allowlists, test_prometheus_metrics_server, test_proxy_cli) their own 2-worker proxy-subprocess shard so the children stop competing with 4 xdist workers on a 4 vCPU runner. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .github/scripts/assert_ci_coverage.py | 6 +- .github/workflows/test-unit.yml | 79 ++++++++++++++++++--------- codecov.yaml | 6 +- tests/unit/test_assert_ci_coverage.py | 26 +++++++++ 4 files changed, 86 insertions(+), 31 deletions(-) diff --git a/.github/scripts/assert_ci_coverage.py b/.github/scripts/assert_ci_coverage.py index b443fd9e0d9..9dcae329b59 100644 --- a/.github/scripts/assert_ci_coverage.py +++ b/.github/scripts/assert_ci_coverage.py @@ -27,11 +27,11 @@ TEST_PATH_KEYS = frozenset({"test-path", "test-paths", "test_path"}) DOCKERFILE_INPUT_KEYS = frozenset({"file", "dockerfile"}) TEST_RUNNER_RE = re.compile(r"\bpytest\b|\bhelm unittest\b|\bplaywright test\b|\bpython[0-9.]*\s") IMAGE_BUILD_RE = re.compile(r"\bdocker\s+(?:buildx\s+)?build\b") -TEST_TOKEN_RE = re.compile(r"tests/[A-Za-z0-9_./*?-]+") -IGNORE_ARG_RE: Final = re.compile(r"--ignore(?:-glob)?[= ](\S+)") +TEST_TOKEN_RE = re.compile(r"tests/[A-Za-z0-9_./*?\[\]-]+") +IGNORE_ARG_RE: Final = re.compile(r"--(?:ignore(?:-glob)?|deselect)[= ](\S+)") DOCKERFILE_TOKEN_RE = re.compile(r"[A-Za-z0-9_./-]*Dockerfile[A-Za-z0-9_.-]*") COMMENT_RE = re.compile(r"^\s*#.*$", re.MULTILINE) -GLOB_CHARS = frozenset("*?") +GLOB_CHARS = frozenset("*?[") DEDICATED_INTEGRATION_WORKFLOWS: Final = MappingProxyType( { "image-scan.yml": "tests/integration/database/image/", diff --git a/.github/workflows/test-unit.yml b/.github/workflows/test-unit.yml index 6f980b458fc..f36c232791a 100644 --- a/.github/workflows/test-unit.yml +++ b/.github/workflows/test-unit.yml @@ -673,34 +673,31 @@ jobs: job-timeout-minutes: 60 dist: loadscope - - shard: proxy-infra-root + - shard: proxy-infra-root-a-p test-path: |- - tests/unit/proxy/test_*.py - --ignore=tests/unit/proxy/test_aproxy_startup.py - --ignore=tests/unit/proxy/test_credential_slot_registry.py - --ignore=tests/unit/proxy/test_custom_callback_input.py - --ignore=tests/unit/proxy/test_custom_logger_s3_gcs.py - --ignore=tests/unit/proxy/test_custom_tokenizer_bug.py - --ignore=tests/unit/proxy/test_db_schema_changes.py - --ignore=tests/unit/proxy/test_deprecated_key_grace_period.py - --ignore=tests/unit/proxy/test_get_favicon.py - --ignore=tests/unit/proxy/test_get_image.py - --ignore=tests/unit/proxy/test_prisma_client_backoff_retry.py - --ignore=tests/unit/proxy/test_prompt_test_endpoint.py - --ignore=tests/unit/proxy/test_proxy_config_unit_test.py - --ignore=tests/unit/proxy/test_proxy_custom_auth.py - --ignore=tests/unit/proxy/test_proxy_reject_logging.py - --ignore=tests/unit/proxy/test_proxy_server.py - --ignore=tests/unit/proxy/test_proxy_setting_guardrails.py - --ignore=tests/unit/proxy/test_proxy_token_counter.py - --ignore=tests/unit/proxy/test_proxy_utils.py - --ignore=tests/unit/proxy/test_reducto_ocr_route.py - --ignore=tests/unit/proxy/test_response_polling_pre_call_checks.py - --ignore=tests/unit/proxy/test_server_root_path.py - --ignore=tests/unit/proxy/test_ui_path_detection.py - --ignore=tests/unit/proxy/test_unit_test_proxy_hooks.py - --ignore=tests/unit/proxy/test_update_spend.py - --ignore=tests/unit/proxy/test_zero_cost_model_budget_bypass.py + tests/unit/proxy/test_[_a-p]*.py + --deselect=tests/unit/proxy/test_aproxy_startup.py + --deselect=tests/unit/proxy/test_component_allowlists.py + --deselect=tests/unit/proxy/test_credential_slot_registry.py + --deselect=tests/unit/proxy/test_custom_callback_input.py + --deselect=tests/unit/proxy/test_custom_logger_s3_gcs.py + --deselect=tests/unit/proxy/test_custom_tokenizer_bug.py + --deselect=tests/unit/proxy/test_db_schema_changes.py + --deselect=tests/unit/proxy/test_deprecated_key_grace_period.py + --deselect=tests/unit/proxy/test_get_favicon.py + --deselect=tests/unit/proxy/test_get_image.py + --deselect=tests/unit/proxy/test_prisma_client_backoff_retry.py + --deselect=tests/unit/proxy/test_prometheus_metrics_server.py + --deselect=tests/unit/proxy/test_prompt_test_endpoint.py + --deselect=tests/unit/proxy/test_proxy_cli.py + --deselect=tests/unit/proxy/test_proxy_config_unit_test.py + --deselect=tests/unit/proxy/test_proxy_custom_auth.py + --deselect=tests/unit/proxy/test_proxy_reject_logging.py + --deselect=tests/unit/proxy/test_proxy_server.py + --deselect=tests/unit/proxy/test_proxy_setting_guardrails.py + --deselect=tests/unit/proxy/test_proxy_token_counter.py + --deselect=tests/unit/proxy/test_proxy_utils.py + --deselect=tests/unit/proxy/test_unit_test_proxy_hooks.py python-version: "3.12" workers: 4 reruns: 0 @@ -708,6 +705,34 @@ jobs: job-timeout-minutes: 60 dist: loadscope + - shard: proxy-infra-root-q-z + test-path: |- + tests/unit/proxy/test_[q-z]*.py + --deselect=tests/unit/proxy/test_reducto_ocr_route.py + --deselect=tests/unit/proxy/test_response_polling_pre_call_checks.py + --deselect=tests/unit/proxy/test_server_root_path.py + --deselect=tests/unit/proxy/test_ui_path_detection.py + --deselect=tests/unit/proxy/test_update_spend.py + --deselect=tests/unit/proxy/test_zero_cost_model_budget_bypass.py + python-version: "3.12" + workers: 4 + reruns: 0 + timeout-minutes: 20 + job-timeout-minutes: 60 + dist: loadscope + + - shard: proxy-subprocess + test-path: |- + tests/unit/proxy/test_component_allowlists.py + tests/unit/proxy/test_prometheus_metrics_server.py + tests/unit/proxy/test_proxy_cli.py + python-version: "3.12" + workers: 2 + reruns: 0 + timeout-minutes: 20 + job-timeout-minutes: 60 + dist: loadscope + - shard: caching-local test-path: >- tests/unit/caching diff --git a/codecov.yaml b/codecov.yaml index 419a28ce6c6..d671d883120 100644 --- a/codecov.yaml +++ b/codecov.yaml @@ -58,7 +58,11 @@ flag_management: carryforward: false - name: proxy-infra carryforward: false - - name: proxy-infra-root + - name: proxy-infra-root-a-p + carryforward: false + - name: proxy-infra-root-q-z + carryforward: false + - name: proxy-subprocess carryforward: false - name: caching-local carryforward: false diff --git a/tests/unit/test_assert_ci_coverage.py b/tests/unit/test_assert_ci_coverage.py index 4127432bd3d..0a50bef385d 100644 --- a/tests/unit/test_assert_ci_coverage.py +++ b/tests/unit/test_assert_ci_coverage.py @@ -343,6 +343,32 @@ def test_a_file_its_only_shard_ignores_is_not_covered_by_that_shards_glob() -> N assert selection.covers("tests/unit/proxy/test_update_spend.py") is False +def test_a_deselected_file_is_treated_like_an_ignored_one() -> None: + # --ignore is a no-op for a file the shell glob already expanded, --deselect is what drops it + scalars: Final = ( + coverage.Scalar( + key="test-path", + value="tests/unit/proxy/test_[q-z]*.py\n--deselect=tests/unit/proxy/test_update_spend.py", + ), + ) + assert coverage._invoked_test_tokens(scalars) == frozenset({"tests/unit/proxy/test_[q-z]*.py"}) + (selection,) = coverage._invoked_selections(scalars) + assert selection.covers("tests/unit/proxy/test_route_llm_request.py") is True + assert selection.covers("tests/unit/proxy/test_update_spend.py") is False + + +def test_a_character_class_glob_in_a_test_path_is_one_token_that_names_only_its_letters() -> None: + (selection,) = coverage._invoked_selections( + (coverage.Scalar(key="test-path", value="tests/unit/proxy/test_[_a-p]*.py"),) + ) + assert selection.included == frozenset({"tests/unit/proxy/test_[_a-p]*.py"}) + assert selection.covers("tests/unit/proxy/test__types.py") is True + assert selection.covers("tests/unit/proxy/test_caching_routes.py") is True + assert selection.covers("tests/unit/proxy/test_route_llm_request.py") is False + assert coverage._token_names("tests/unit/proxy/test_[_a-p]*.py", "tests/unit/proxy/test_admin_mcp.py") is True + assert coverage._token_names("tests/unit/proxy/test_[_a-p]*.py", "tests/unit/proxy/test_route_llm_request.py") is False + + def test_check_shards_passes_on_the_repo_as_it_stands(capsys): assert coverage._check_shards() == 0