Skip to content

ci(tests): wire tests/unit into CircleCI and keep draining GHA shards green - #42103

Merged
yuneng-berri merged 4 commits into
mainfrom
litellm_test_unit_ci_wiring
Sep 20, 2026
Merged

yuneng-berri merged 4 commits into
mainfrom
litellm_test_unit_ci_wiring

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • tests/unit exists but no CI job runs it
  • Legacy GHA shards fail with exit 5 once they drain to empty
  • TQ008 (patched internals) blocks moving tests, no longer wanted as a gate

How it solves it:

  • CircleCI unit job: prisma generate, pytest with xdist, no reruns, junit artifact
  • tests/unit/conftest.py blocks every socket except loopback via pytest-socket
  • _test-unit-base.yml treats exit 5 or missing TEST_PATH as green with one log line
  • TQ008 removed from the checker, its budget and its tests

User Flow

Before: a contributor moves a test file into tests/unit and gets no CI signal for it, while the shard it came from breaks once it empties

  1. They push a branch that moves the last file out of a tests/test_litellm shard directory into tests/unit
  2. Nothing on CircleCI or GitHub collects the moved test, so a broken test still reports green
  3. The old shard's "Run tests" step fails with pytest exit code 5 (no tests collected) and the PR turns red

After: the same move is collected by CircleCI and the emptied shard keeps posting green

  1. They push the same branch
  2. The CircleCI unit job on the branch collects tests/unit/**/test_*.py and fails if the moved test fails, or if it opens a socket to anything other than loopback
  3. The old shard's "Run tests" step logs pytest collected no tests from <path>; passing and exits 0

Relevant issues

Phase 0 of the legacy test migration. Wave 1 branches rebase onto this once merged

Affected release

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

Before (d5d12ed)

CircleCI unit job

  1. No unit job exists in .circleci/config.yml, so nothing collects tests/unit

GHA shard pointed at an empty directory

  1. pytest <empty dir> exits 5 and the "Run tests" step fails

Test quality checker

  1. test-quality-budget.json carries "TQ008": {"limit": 10993} and the checker emits it

After (adadeac)

CircleCI unit job

  1. CircleCI unit job on this branch, green with tests/unit holds no test_*.py files; nothing to run: CIRCLECI_LINK_PENDING

GHA shard pointed at an empty directory

  1. Commit 3c094db pointed the mcp-integration shard's test-path at tests/unit (no test files); the run posted green with pytest collected no tests from tests/unit; passing: https://github.com/BerriAI/litellm/actions/runs/35496061005/job/106039184618
  2. Reverted in adadeac

Test quality checker

  1. make lint-test-quality (base origin/main): OK: every TQ rule is within its test-suite ceiling (base origin/main)
  2. uv run --no-sync pytest tests/test_litellm/test_check_test_quality.py tests/test_litellm/test_test_quality_gate.py -p no:rerunfailures: 85 passed in 4.02s
  3. uv run --no-sync python .github/scripts/assert_ci_coverage.py: OK: 2792 test files and 9 Dockerfiles are each invoked by at least one job or carry an explicit allowlist entry.

Socket block

  1. Throwaway tests/unit/test_probe.py (deleted before commit): socket.create_connection(("1.1.1.1", 80)) raises SocketConnectBlockedError, a connect to a listening 127.0.0.1 socket succeeds. pytest tests/unit -p no:rerunfailures -p no:pytest-retry --timeout=90 -n 2: 2 passed

Type

🚄 Infrastructure
✅ Test

Caveats (if any)

Medium

  • pytest-socket lifts its restriction in its own teardown hook, so the conftest re-arms it in pytest_runtest_setup as well as the session fixture
  • test-quality-budget.json is edited on a PR branch on purpose: the TQ008 entry goes away with the rule. The non-gating budget-ratchet check is red for exactly that reason (a dropped rule reads as a loosened ceiling)

Low

  • tests/unit is not in SHARDED_ROOTS of assert_ci_coverage.py; that census expects GHA shards and this job is CircleCI
  • The GHA "Save coverage report" step warns about a missing coverage.xml when a shard exits early with nothing to run

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

Link to Devin session: https://app.devin.ai/sessions/ebd84a741e7d41ccab2f9ad9e16c6d32
Open in Devin Desktop: https://app.devin.ai/desktop/session/ebd84a741e7d41ccab2f9ad9e16c6d32?variant=devin

yuneng-berri and others added 2 commits September 20, 2026 07:05
… green

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration
devin-ai-integration Bot requested a review from a team September 20, 2026 07:07
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@greptile-apps

greptile-apps Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 3/5

This PR is not safe to merge until unit tests run on intended pull-request branches and socket restrictions cover collection-time code

Findings

  1. P1 Unit Job Skips Branches ▶
  2. P1 Collection Network Remains Open ▶
  3. P2 Forbidden Budget Edit ▶
  4. P2 Socket Policy Lacks Tests ▶

Summary

This PR adds CircleCI execution for tests/unit, permits drained GitHub Actions shards to exit successfully, adds loopback-only socket handling, and retires TQ008

  • The CircleCI job currently excludes pull-request branches not named litellm_*
  • Socket restrictions are installed too late to cover collection-time imports
  • The budget edit and missing regression test conflict with repository instructions

Reviews (1) · Last reviewed commit: "ci(tests): share the loopback allow list..."

Comment thread .circleci/config.yml
Comment on lines +3077 to +3078
- unit:
filters: *main_branches

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Unit Job Skips Branches

Only main and litellm_* branches run this job, so other pull requests can merge broken tests/unit tests.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment thread tests/unit/conftest.py
Comment on lines +14 to +16
@pytest.fixture(autouse=True, scope="session")
def block_external_sockets() -> Iterator[None]:
_allow_loopback_only()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Collection Network Remains Open

Blocking starts during test setup, so module imports can make external connections during collection and bypass the unit suite's network policy.

Comment thread test-quality-budget.json
Comment on lines 23 to 25
"TQ009": {
"limit": 59
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Forbidden Budget Edit

This deletes a test-quality-budget.json entry, violating the repository directive that budget files must not change on PR branches. Revert before merging.

Context Used: AGENTS.md (source)

Comment thread tests/unit/conftest.py
Comment on lines +10 to +11
def _allow_loopback_only() -> None:
socket_allow_hosts(LOOPBACK_HOSTS, allow_unix_socket=True)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Socket Policy Lacks Tests

This adds socket enforcement without a committed regression test, violating the repository requirement for meaningful feature tests. Add one before merging.

Context Used: AGENTS.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@codecov

codecov Bot commented Sep 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_test_unit_ci_wiring (adadeac) with main (58065d4)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (d5d12ed) during the generation of this report, so 58065d4 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

This branch is waiting to be deployed

1 waiting deployment
e2e-changed — adadeac2 Waiting Sep 20, 2026 by devin-ai-integration[bot] via oauth #26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant