[Infra] Remove CCI/GHA test duplication and semantically shard proxy DB tests - #26356
Conversation
…DB tests
Split into two related cleanups:
1. Delete CCI jobs that duplicate GHA coverage:
- mcp_testing (tests/mcp_tests) — already run by test-mcp.yml
- litellm_mapped_tests_proxy_part1/part2 (tests/test_litellm/proxy) —
already run across test-unit-proxy-auth.yml, test-unit-proxy-endpoints.yml,
and test-unit-proxy-infra.yml
Add rag_endpoints and realtime_endpoints to test-unit-proxy-endpoints.yml
(they were only covered by the deleted CCI part2 job).
Remove the corresponding workflow wiring, coverage combine entries, and
upload-coverage dependencies in .circleci/config.yml.
2. Re-shard test-unit-proxy-db.yml from 4 alphabetic buckets to 8 semantic
ones (auth-and-jwt, proxy-server, logging-and-callbacks, db-and-spend,
guardrails-budget-hooks, endpoints-and-responses, plus the existing
serial key-generation and test_proxy_utils.py shards). New test files are
placed in whichever group they belong to instead of reshuffling slices.
Add a dist input to _test-unit-services-base.yml so the test_proxy_utils.py
shard can use --dist=worksteal to spread its ~64 (many parametrized)
functions across workers; the default --dist=loadscope pins a single file
to a single worker, which was the root cause of that shard running 10m+.
Low: CI infrastructure changes onlyThis PR removes duplicate CircleCI/GHA test jobs and reshards proxy DB tests into semantic groups. The hardcoded Postgres credentials ( Status: 0 open Posted by Veria AI · 2026-04-23T23:33:13.324Z |
Greptile SummaryThis PR removes three CircleCI jobs ( Confidence Score: 5/5Safe to merge — all remaining findings are minor P2 style issues with no functional impact. No P0 or P1 issues found. The secrets removal is safe (ephemeral localhost container, well-documented). The assert-shard-coverage guard correctly addresses the previously raised concern about uncovered test files. Both remaining comments are cosmetic: a stale header comment and a redundant line in a test fixture. test-unit-proxy-db.yml (stale header comment); test_realtime_webrtc_endpoints.py (redundant master_key assignment in one test body)
|
| Filename | Overview |
|---|---|
| .circleci/config.yml | Removes mcp_testing, litellm_mapped_tests_proxy_part1/part2 jobs and their workflow/coverage references — clean deduplication with no orphaned references. |
| .github/workflows/_test-unit-services-base.yml | Adds dist input, removes GHA environment secret scoping, replaces DATABASE_URL/POSTGRES_* secrets with hardcoded ephemeral localhost credentials for the throwaway service container; well-commented rationale included. |
| .github/workflows/test-unit-proxy-db.yml | Expands matrix from 4 alphabetic shards to 15 semantic shards and adds assert-shard-coverage guard; top-of-file comment still references the now-removed DATABASE_URL secret. |
| .github/workflows/test-unit-proxy-endpoints.yml | Adds rag_endpoints and realtime_endpoints to the test path, filling the coverage gap left by deleted CCI jobs. |
| .github/workflows/test-unit-security.yml | Updates stale comment and removes the now-unnecessary secrets block; the comment correctly explains why push-only trigger is retained. |
| tests/test_litellm/proxy/realtime_endpoints/test_realtime_webrtc_endpoints.py | Converts proxy_app fixture from return to yield with proper teardown to prevent master_key state from leaking across xdist workers; one test still sets master_key directly in the test body (redundant but harmless). |
Reviews (7): Last reviewed commit: "[Infra] Stop attaching push-only postgre..." | Re-trigger Greptile
| fail-fast: false | ||
| matrix: | ||
| include: | ||
| # Key generation tests must NOT run in parallel (event loop conflicts with logging worker) | ||
| # Must run serially — event-loop conflict with the logging worker. | ||
| - test-group: key-generation | ||
| test-path: "tests/proxy_unit_tests/test_key_generate_prisma.py" | ||
| workers: 0 | ||
| timeout: 30 | ||
| - test-group: auth-checks | ||
| test-path: "tests/proxy_unit_tests/test_auth_checks.py tests/proxy_unit_tests/test_user_api_key_auth.py" | ||
| workers: 8 | ||
| dist: loadscope | ||
| timeout: 20 | ||
| # test_proxy_utils.py is large (168+ parametrized tests) — run it on its | ||
| # own matrix so --dist=loadscope doesn't pin all of it to a single xdist | ||
| # worker and push the "remaining" group past the job timeout. | ||
|
|
||
| - test-group: auth-and-jwt | ||
| test-path: >- | ||
| tests/proxy_unit_tests/test_auth_checks.py | ||
| tests/proxy_unit_tests/test_user_api_key_auth.py | ||
| tests/proxy_unit_tests/test_jwt.py | ||
| tests/proxy_unit_tests/test_jwt_key_mapping.py | ||
| tests/proxy_unit_tests/test_proxy_custom_auth.py | ||
| tests/proxy_unit_tests/test_key_generate_dynamodb.py | ||
| tests/proxy_unit_tests/test_deployed_proxy_keygen.py | ||
| workers: 8 | ||
| dist: loadscope | ||
| timeout: 15 | ||
|
|
||
| # Own shard, --dist=worksteal so parametrized cases fan out across workers. | ||
| - test-group: proxy-utils | ||
| test-path: "tests/proxy_unit_tests/test_proxy_utils.py" | ||
| workers: 8 | ||
| timeout: 20 | ||
| - test-group: remaining | ||
| test-path: "tests/proxy_unit_tests --ignore=tests/proxy_unit_tests/test_key_generate_prisma.py --ignore=tests/proxy_unit_tests/test_auth_checks.py --ignore=tests/proxy_unit_tests/test_user_api_key_auth.py --ignore=tests/proxy_unit_tests/test_proxy_utils.py" | ||
| dist: worksteal | ||
| timeout: 15 | ||
|
|
||
| - test-group: proxy-server | ||
| test-path: >- | ||
| tests/proxy_unit_tests/test_proxy_server.py | ||
| tests/proxy_unit_tests/test_proxy_server_keys.py | ||
| tests/proxy_unit_tests/test_proxy_server_caching.py | ||
| tests/proxy_unit_tests/test_proxy_server_langfuse.py | ||
| tests/proxy_unit_tests/test_proxy_server_spend.py | ||
| tests/proxy_unit_tests/test_aproxy_startup.py | ||
| tests/proxy_unit_tests/test_proxy_config_unit_test.py | ||
| tests/proxy_unit_tests/test_proxy_routes.py | ||
| tests/proxy_unit_tests/test_proxy_gunicorn.py | ||
| tests/proxy_unit_tests/test_server_root_path.py | ||
| tests/proxy_unit_tests/test_proxy_pass_user_config.py | ||
| tests/proxy_unit_tests/test_proxy_token_counter.py | ||
| workers: 8 | ||
| dist: loadscope | ||
| timeout: 15 | ||
|
|
||
| - test-group: logging-and-callbacks | ||
| test-path: >- | ||
| tests/proxy_unit_tests/test_custom_callback_input.py | ||
| tests/proxy_unit_tests/test_custom_logger_s3_gcs.py | ||
| tests/proxy_unit_tests/test_proxy_custom_logger.py | ||
| tests/proxy_unit_tests/test_proxy_reject_logging.py | ||
| tests/proxy_unit_tests/test_audit_logs_proxy.py | ||
| tests/proxy_unit_tests/test_search_api_logging.py | ||
| workers: 8 | ||
| dist: loadscope | ||
| timeout: 15 | ||
|
|
||
| - test-group: db-and-spend | ||
| test-path: >- | ||
| tests/proxy_unit_tests/test_prisma_client_backoff_retry.py | ||
| tests/proxy_unit_tests/test_db_schema_changes.py | ||
| tests/proxy_unit_tests/test_db_schema_migration.py | ||
| tests/proxy_unit_tests/test_e2e_pod_lock_manager.py | ||
| tests/proxy_unit_tests/test_skills_db.py | ||
| tests/proxy_unit_tests/test_update_daily_tag_spend.py | ||
| tests/proxy_unit_tests/test_update_spend.py | ||
| tests/proxy_unit_tests/test_project_endpoints_prisma.py | ||
| tests/proxy_unit_tests/test_proxy_encrypt_decrypt.py | ||
| workers: 8 | ||
| dist: loadscope | ||
| timeout: 15 | ||
|
|
||
| - test-group: guardrails-budget-hooks | ||
| test-path: >- | ||
| tests/proxy_unit_tests/test_proxy_setting_guardrails.py | ||
| tests/proxy_unit_tests/test_banned_keyword_list.py | ||
| tests/proxy_unit_tests/test_default_end_user_budget_simple.py | ||
| tests/proxy_unit_tests/test_unit_test_max_model_budget_limiter.py | ||
| tests/proxy_unit_tests/test_zero_cost_model_budget_bypass.py | ||
| tests/proxy_unit_tests/test_unit_test_proxy_hooks.py | ||
| workers: 8 | ||
| dist: loadscope | ||
| timeout: 15 | ||
|
|
||
| - test-group: endpoints-and-responses | ||
| test-path: >- | ||
| tests/proxy_unit_tests/test_blog_posts_endpoint.py | ||
| tests/proxy_unit_tests/test_models_fallback_endpoint.py | ||
| tests/proxy_unit_tests/test_google_endpoint_routing.py | ||
| tests/proxy_unit_tests/test_google_gemini_proxy_request.py | ||
| tests/proxy_unit_tests/test_get_favicon.py | ||
| tests/proxy_unit_tests/test_get_image.py | ||
| tests/proxy_unit_tests/test_ui_path_detection.py | ||
| tests/proxy_unit_tests/test_prompt_test_endpoint.py | ||
| tests/proxy_unit_tests/test_check_batch_cost.py | ||
| tests/proxy_unit_tests/test_check_responses_cost.py | ||
| tests/proxy_unit_tests/test_response_polling_handler.py | ||
| tests/proxy_unit_tests/test_response_polling_pre_call_checks.py | ||
| tests/proxy_unit_tests/test_realtime_cache.py | ||
| tests/proxy_unit_tests/test_proxy_exception_mapping.py | ||
| tests/proxy_unit_tests/test_custom_tokenizer_bug.py | ||
| tests/proxy_unit_tests/test_model_response_typing | ||
| workers: 8 | ||
| timeout: 30 | ||
| dist: loadscope | ||
| timeout: 15 | ||
| uses: ./.github/workflows/_test-unit-services-base.yml | ||
| with: | ||
| test-path: ${{ matrix.test-path }} |
There was a problem hiding this comment.
No catch-all shard for unassigned test files
The old remaining shard used tests/proxy_unit_tests --ignore=... as a directory target, so any new test_*.py file added to tests/proxy_unit_tests/ was automatically picked up. The new matrix replaces it with an exhaustive explicit list and no fallback directory shard. A developer who adds a new test file without also updating this matrix will have that file silently skipped by CI — no failure, no warning.
Consider keeping one "catch-all" shard (or adding a CI step that validates every test_*.py in the directory is covered by the matrix) to prevent future tests from going untested.
| include: | ||
| # Key generation tests must NOT run in parallel (event loop conflicts with logging worker) | ||
| # Must run serially — event-loop conflict with the logging worker. | ||
| - test-group: key-generation | ||
| test-path: "tests/proxy_unit_tests/test_key_generate_prisma.py" | ||
| workers: 0 | ||
| timeout: 30 | ||
| - test-group: auth-checks | ||
| test-path: "tests/proxy_unit_tests/test_auth_checks.py tests/proxy_unit_tests/test_user_api_key_auth.py" | ||
| workers: 8 | ||
| dist: loadscope |
There was a problem hiding this comment.
dist: loadscope on a serial (workers: 0) shard is a no-op
The key-generation matrix entry sets dist: loadscope and workers: 0. In _test-unit-services-base.yml, the --dist flag is only passed in the else branch (i.e., when WORKERS != 0). The dist value is effectively ignored here. While harmless, it may mislead future maintainers into thinking --dist=loadscope is somehow enforced for this serial job.
| include: | |
| # Key generation tests must NOT run in parallel (event loop conflicts with logging worker) | |
| # Must run serially — event-loop conflict with the logging worker. | |
| - test-group: key-generation | |
| test-path: "tests/proxy_unit_tests/test_key_generate_prisma.py" | |
| workers: 0 | |
| timeout: 30 | |
| - test-group: auth-checks | |
| test-path: "tests/proxy_unit_tests/test_auth_checks.py tests/proxy_unit_tests/test_user_api_key_auth.py" | |
| workers: 8 | |
| dist: loadscope | |
| - test-group: key-generation | |
| test-path: "tests/proxy_unit_tests/test_key_generate_prisma.py" | |
| workers: 0 | |
| # dist is ignored when workers=0; serial run avoids event-loop conflicts | |
| timeout: 20 |
…shard-coverage guard Two fixes to proxy-db CI: 1. test_realtime_webrtc_endpoints.py's `proxy_app` fixture mutated the module-global `proxy_server.master_key` without restoring it, leaking state into any test that shared the same xdist worker. Under --dist=loadscope with 2 workers (GHA proxy-endpoints), this caused the google_endpoints tests to fail with "No api key passed in." because user_api_key_auth saw a set master_key and a missing API key on the test request. The fixture now saves and restores the original value. 2. Address the Greptile note that the semantic shard design has no catch-all, so a new test file added to tests/proxy_unit_tests/ without a matrix entry would silently skip CI. Adds an assert-shard-coverage job that enumerates test_*.py files and fails the workflow if any are not referenced by a matrix entry, with a clear message telling the author which semantic shard to place it in. All proxy-db shards now depend on this guard.
Previous run (13.8m total) was bottlenecked by shards with 9-12m wall-clock. Setup + xdist spawn + coverage teardown is ~3m per shard, so each shard's pytest runtime must stay under ~4m to fit inside 7m total. Observed per-shard pytest times (before split): db-and-spend 9:08 (170s outlier: test_aaaasschema_migration_check) proxy-server 7:15 logging-and-callbacks 6:45 guardrails-budget-hooks 6:37 proxy-utils 6:23 auth-and-jwt 6:54 Split 6 shards into 12, keeping key-generation and endpoints-and-responses (already <7m). Adds a `keyword` input to _test-unit-services-base.yml so test_proxy_utils.py can be split by -k expression (same file, two runners). New matrix entries: auth-and-jwt -> auth-checks + jwt-and-keys proxy-server -> proxy-server-core + proxy-runtime logging-and-callbacks -> custom-logging + logging-misc db-and-spend -> schema-migration (isolated 170s test) + db-and-spend guardrails-budget-hooks-> guardrails-hooks + budgets proxy-utils -> proxy-utils-a-h + proxy-utils-i-z (-k split) The -k expression split is verified to cover every one of the 64 test functions in test_proxy_utils.py exactly once. The assert-shard-coverage guard still catches any file not in any shard.
test_db_schema_migration.py has exactly one test, and that test is mostly waiting on prisma subprocesses (~170s: prisma migrate deploy + prisma migrate diff). No CPU-bound Python work inside the test body, and only one test in the file means xdist's parallelism is unused regardless. Previous run on commit 5df9f39: 10.0m wall-clock for the shard, of which 4:56 was silence between step start and pytest banner — the cost of 4 xdist workers each cold-starting (pytest plugin load + litellm import + pytest-cov instrumentation) so that exactly one of them could pick up the single test. Switching to workers: 0 takes the serial pytest branch in the base workflow, which already handles this case correctly (no -n, no --dist). Single-process startup instead of 4. Expected wall-clock: ~6m.
The `_test-unit-services-base.yml` reusable workflow attached every job to the `integration-postgres` GHA environment to read three "secrets": DATABASE_URL, POSTGRES_USER, POSTGRES_PASSWORD. These are not secrets — the postgres service container is spawned per-job on localhost and destroyed with the job, so the user/password are bootstrap values for a throwaway container and the URL is always `postgresql://…@localhost:…`. Each environment attachment produces a "temporarily deployed to integration-postgres" deployment record, which the PR timeline renders as a message per matrix shard per push. With 14 proxy-db shards that's ~14 notifications per push, drowning the PR conversation. Changes: * Hardcode POSTGRES_USER/POSTGRES_PASSWORD/POSTGRES_DB and the derived DATABASE_URL in `_test-unit-services-base.yml`. * Delete the `environment: integration-postgres` attachment. * Delete the `secrets:` declarations on the reusable workflow and on the two callers (test-unit-proxy-db.yml, test-unit-security.yml). * The `services:` container still starts a fresh postgres per job; the connection string now matches what the container boots up with. Security review: no regression. The environment wasn't gating anything real — no protection rules configured, no approval gates, and the branch restriction is already enforced by `on: push: branches: [...]` on both caller workflows. Zizmor pedantic-mode findings are identical before and after (same 6 pre-existing findings, zero new ones). The `integration-postgres` environment and its three "secrets" in repo settings are now unreferenced and can be deleted from repo admin.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 31539530 | Triggered | Generic Password | 66bf890 | .github/workflows/_test-unit-services-base.yml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Summary
Two related CI cleanups:
tests/mcp_tests/ran in both CCImcp_testingand GHAtest-mcp.yml;tests/test_litellm/proxy/*ran in both CCIlitellm_mapped_tests_proxy_part1/part2and GHAtest-unit-proxy-{auth,endpoints,infra}.yml. Removes the CCI copies so there is one source of truth per test directory, matching the design rule of CCI = integration / GHA = unit.test-unit-proxy-db.ymlsemantically. Replaces 4 alphabetic buckets with 8 named groups (auth-and-jwt,proxy-server,logging-and-callbacks,db-and-spend,guardrails-budget-hooks,endpoints-and-responses, plus the existing serialkey-generationandtest_proxy_utils.pyshards). New test files are placed in whichever group they belong to. Adds adistinput to_test-unit-services-base.ymlsotest_proxy_utils.pycan run with--dist=worksteal— the default--dist=loadscopepins a single file to a single xdist worker, which was the root cause of that shard running 10m+.Changes
.circleci/config.yml: removemcp_testing,litellm_mapped_tests_proxy_part1,litellm_mapped_tests_proxy_part2job definitions; remove their references from thebuild_and_testworkflow, theupload-coveragedependency list, and the coverage-combine command..github/workflows/test-unit-proxy-db.yml: expand the matrix from 4 alphabetic shards to 8 semantic shards; usedist: workstealfor the single-fileproxy-utilsshard..github/workflows/_test-unit-services-base.yml: adddistinput (defaultloadscope) and thread it through to pytest..github/workflows/test-unit-proxy-endpoints.yml: addrag_endpointsandrealtime_endpoints(previously only covered by the deleted CCI part2 job).Testing
circleci config pack .circleci/succeeds.tests/test_litellm/proxy/against the remaining GHA workflows —rag_endpointsandrealtime_endpointswere the only gaps and are added here;test_configs/is fixture data.test_*.pyfile undertests/proxy_unit_tests/against the new semantic shards — all 58 test files assigned to exactly one shard (excludingtest_configs/which is fixture data).Type
🚄 Infrastructure
🧹 Refactoring