Skip to content

test(lazy_imports): check star-import exports against globals(), not dir() - #39958

Closed
yuneng-berri wants to merge 1 commit into
litellm_internal_stagingfrom
litellm_/ci-failure-triage-c22c87
Closed

test(lazy_imports): check star-import exports against globals(), not dir()#39958
yuneng-berri wants to merge 1 commit into
litellm_internal_stagingfrom
litellm_/ci-failure-triage-c22c87

Conversation

@yuneng-berri

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

How it solves it:

  • Check each name against globals() instead of dir()
  • Verified the test now fails when a bogus name is appended to __all__

User Flow

Before: a contributor opens any PR against litellm_internal_staging and the misc unit-test jobs go red without them touching anything related

  1. They push a branch and open a PR
  2. The Unit Tests workflow runs misc / Run tests on Python 3.10, 3.11, 3.13 and 3.14
  3. All four fail on tests/test_litellm/test_lazy_imports.py::test_star_import_exports_public_api with an assertion listing AI21ChatConfig, AI21Config, ... as missing
  4. Reruns fail the same way, and the same jobs are red on staging's own push runs

After: the same PR gets green misc jobs

  1. They push a branch and open a PR
  2. The Unit Tests workflow runs misc / Run tests on Python 3.10, 3.11, 3.13 and 3.14
  3. All four pass, and the test still fails if a name in litellm.__all__ stops being importable

Relevant issues

Introduced by #39121

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

This is a test-only change with no proxy or LLM behavior involved, so the proof is the check the test runs, executed by hand in a fresh interpreter, plus the test itself with and without a deliberately broken export

Before (bf51dea)

The check as written in the test

  1. python -c 'from litellm import *; import litellm; missing = [n for n in litellm.__all__ if n not in dir()]; print(len(missing), "of", len(litellm.__all__))'
  2. Output: 1519 of 1519

The same check against globals()

  1. python -c 'from litellm import *; import litellm; missing = [n for n in litellm.__all__ if n not in globals()]; print(len(missing), "of", len(litellm.__all__))'
  2. Output: 0 of 1519, so star import was never broken

The test

  1. pytest tests/test_litellm/test_lazy_imports.py::test_star_import_exports_public_api -q
  2. Output: 1 failed, AssertionError: ['AI21ChatConfig', 'AI21Config', 'ALL_RESPONSES_API_TOOL_PARAMS', ...] (same as CI on all four Python versions)

After (e4296b9)

The check as written in the test

  1. python -c 'from litellm import *; import litellm; missing = [n for n in litellm.__all__ if n not in globals()]; print(len(missing), "of", len(litellm.__all__))'
  2. Output: 0 of 1519

The same check against globals()

  1. Identical to the case above now that the test uses globals()
  2. Output: 0 of 1519

The test

  1. pytest tests/test_litellm/test_lazy_imports.py -q
  2. Output: 22 passed, 1 skipped

Mutation check: the test catches a broken export

  1. Temporarily change litellm/__init__.py to __all__ = list(STAR_IMPORT_PUBLIC_NAMES) + ["bogus_not_exported"]
  2. pytest tests/test_litellm/test_lazy_imports.py::test_star_import_exports_public_api -q
  3. Output: 1 failed, AttributeError: module 'litellm' has no attribute 'bogus_not_exported'
  4. Revert the change and the test passes again

Type

✅ Test

Caveats (if any)

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

…dir()

dir() inside a list comprehension only sees the comprehension's own scope,
so every name in litellm.__all__ read as missing and the test could never
pass. Star import itself was fine; the check is now against the module
globals, and appending a bogus name to __all__ makes it fail as intended.
@yuneng-berri
yuneng-berri requested a review from a team September 5, 2026 22:18
@greptile-apps

greptile-apps Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR corrects the lazy-import regression test to inspect the module namespace populated by from litellm import *

  • Replaces comprehension-scoped dir() with globals()
  • Preserves validation of every name in litellm.__all__
  • Changes test code only, with no production or security behavior affected

Confidence Score: 5/5

The PR appears safe to merge because the corrected assertion accurately checks the namespace populated by the star import

The subprocess uses a fresh module-level namespace, so globals() contains the imported public names without unrelated pre-population, and no actionable failures remain

Important Files Changed

Filename Overview
tests/test_litellm/test_lazy_imports.py Correctly checks star-imported names in the fresh subprocess module namespace

Reviews (1): Last reviewed commit: "test(lazy_imports): check star-import ex..." | Re-trigger Greptile

@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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