Skip to content

test(rust-python-harness): wire existing e2e SDK tests into the matrix - #39463

Merged
ishaan-berri merged 1 commit into
BerriAI:litellm_internal_stagingfrom
ishaan-berri:litellm_rust_python_harness_e2e_parity_sdk
Sep 3, 2026
Merged

ishaan-berri merged 1 commit into
BerriAI:litellm_internal_stagingfrom
ishaan-berri:litellm_rust_python_harness_e2e_parity_sdk

Conversation

@ishaan-berri

@ishaan-berri ishaan-berri commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • The Rust/Python parity harness only tracks 4 SDK functions
  • No harness strategy points at already-existing live SDK tests
  • OCR coverage in a prior version of this PR was tracked per test file

How it solves it:

  • Adds chat_completions and transcription as SDK function columns, backed by existing rust_bridge tests
  • Adds a fourth strategy folder, existing_e2e_test_sdk, wiring already-existing live tests (tests/ocr_tests/, chat completion and Whisper transcription tests) into the matrix
  • Lets a selector end in / to cover a whole test folder, so tests/ocr_tests/ is tracked as one folder instead of per file

User Flow

Not applicable: this only changes an internal developer-facing test harness under tests/rust-python-harness/, not a proxy or SDK endpoint.

Relevant issues

Linear ticket

Pre-Submission checklist

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/test_rust_python_harness.py -v
  • 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

Screenshots / Proof of Fix

Before (7a81ae9)

  1. poetry run python -m tests.rust-python-harness --list shows only 4 SDK function columns and 3 strategy folders

After (ec50a9a)

  1. poetry run python -m tests.rust-python-harness --list shows chat_completions and transcription columns and the new existing_e2e_test_sdk strategy with tests/ocr_tests/ as a folder selector
  2. python -m pytest tests/ocr_tests/ -q run against real provider APIs: 25 passed, 8 skipped in 17.84s, confirming the folder selector collects and runs the same live tests as before

Type

🧹 Refactoring
✅ Test

Caveats (if any)

Low

  • messages, responses, and count_tokens stay planned under existing_e2e_test_sdk; no existing live SDK-level test was found for them yet

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

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@devin-ai-integration devin-ai-integration Bot added the risk:medium Moderate-risk functional change label Sep 3, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor

PR #39463 labeled risk:medium (15 files, +384/-63, all confined to the tests/rust-python-harness tooling; no sensitive areas). No enterprise label, so Linear routing was skipped.

@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds Python and Rust variants for running existing SDK end-to-end tests through the parity harness

  • Expands strategy manifests into environment-specific rows
  • Groups pytest runs by environment and combines their dashboard state
  • Adds OCR, chat-completion, and transcription selectors and updates SDK-function mappings

Confidence Score: 4/5

The combined parity view needs backend-aware summaries and rerun commands before merging; the remaining findings are non-blocking maintainability issues

Distinct variant cells are retained, but aggregate outcomes and failure guidance collapse or omit backend identity for identical pytest node IDs

Files Needing Attention: tests/rust-python-harness/runner.py, tests/rust-python-harness/ui.py, tests/rust-python-harness/catalog.py

Important Files Changed

Filename Overview
tests/rust-python-harness/runner.py Adds environment switching and grouped pytest execution, but combined reporting loses backend identity for shared node IDs
tests/rust-python-harness/catalog.py Adds nested manifest discovery and variant expansion with repository-style violations around comments and mutable collections
tests/rust-python-harness/strategies/e2e_parity/sdk/strategy.json Defines Python and Rust variants and maps existing SDK end-to-end suites to bridge functions
tests/rust-python-harness/strategies/e2e_parity/runner.py Adds a standalone runner for the e2e parity family with an unnecessary explanatory module docstring
tests/test_rust_python_harness.py Updates harness tests for new function names, nested strategies, variants, and folder selectors, but does not cover combined variant reporting

Reviews (1): Last reviewed commit: "feat(rust-python-harness): add existing_..." | Re-trigger Greptile

Comment thread tests/rust-python-harness/runner.py Outdated
Comment on lines +237 to +241
combined.results.update(sub_run.results)
combined.current_nodeid = sub_run.current_nodeid
for failure in sub_run.failures:
if failure not in combined.failures:
combined.failures.append(failure)

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 When identical nodes run under both variants, merged failures omit the backend and rerun commands can execute the wrong path.

Comment thread tests/rust-python-harness/runner.py Outdated
Comment on lines +220 to +228
"""Run `cases` once per distinct owning-strategy environment.

A strategy that declares `variants` (for example Python vs. Rust via
`LITELLM_RUST`) produces one `HarnessCase` per variant, each carrying
its own `strategy_id`. This groups cases by their strategy's
environment overrides, runs pytest once per group under that
environment, and merges every group's results into one combined run so
a variant that fails is never silently overwritten by one that passes.
"""

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 These lengthy docstrings narrate straightforward orchestration, creating prose that maintainers must synchronize without explaining complex business logic.

Context Used: CLAUDE.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!

Comment thread tests/rust-python-harness/catalog.py Outdated
@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@ishaan-berri
ishaan-berri force-pushed the litellm_rust_python_harness_e2e_parity_sdk branch from 749a750 to ec50a9a Compare September 3, 2026 02:28
@ishaan-berri ishaan-berri changed the title feat(rust-python-harness): add existing_e2e_test_sdk strategy under strategies/e2e_parity/sdk test(rust-python-harness): wire existing e2e SDK tests into the matrix Sep 3, 2026
@yujonglee-berri
yujonglee-berri self-requested a review September 3, 2026 02:32
@ishaan-berri
ishaan-berri enabled auto-merge (squash) September 3, 2026 02:38
Adds chat_completions and transcription as SDK function columns, backed
by the existing rust_bridge test files. Adds a fourth strategy folder,
existing_e2e_test_sdk, that points at already-existing live-API SDK
tests (tests/ocr_tests/ as a whole folder, plus chat completion and
Whisper transcription tests) instead of writing new parity tests.
Extends selector_matches_node with trailing-slash folder selectors so
a whole test folder can back one matrix cell.
@ishaan-berri
ishaan-berri force-pushed the litellm_rust_python_harness_e2e_parity_sdk branch from ec50a9a to fc0b032 Compare September 3, 2026 02:40
@ishaan-berri
ishaan-berri merged commit bcd3e2d into BerriAI:litellm_internal_staging Sep 3, 2026
75 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:medium Moderate-risk functional change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants