feat: parameterize request plane tests

[biswapanda]

Overview:

parameterize pytest to run unit tests with both request planes: nats and tcp

Details:

Where should the reviewer start?

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

• closes GitHub issue: #xxx

Summary by CodeRabbit

• Tests
  • Enhanced test infrastructure with parameterized runtime fixture to validate multiple transport configurations, improving test coverage and reliability across different deployment scenarios.

[coderabbitai]

Walkthrough

The runtime fixture in conftest.py is parametrized to execute tests with two transport options: tcp and nats. The fixture now dynamically selects the transport plane based on test parameters instead of hardcoding nats.

Changes

Cohort / File(s)	Summary
Test Configuration Parameterization lib/bindings/python/tests/conftest.py	Added params=["tcp", "nats"] to runtime fixture decorator to enable multi-transport test execution, replacing hardcoded transport with dynamic request.param selection

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐰 wiggles whiskers excitedly
Two transports now dance in the test parade,
Tcp and nats, a daring cascade,
No hardcoding here, the fixture's remade,
Hopping through parameters, no longer delayed! 🚀

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description lacks substantive content in critical sections like Details and Where should the reviewer start, with placeholder text in Related Issues.	Fill in the Details section describing specific changes (parametrizing the runtime fixture for tcp/nats), specify which files reviewers should examine (lib/bindings/python/tests/conftest.py), and replace the placeholder GitHub issue number with the actual issue being addressed.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'parameterize request plane tests' directly relates to the main change, which parametrizes the runtime fixture to support multiple transport options (tcp and nats) in tests.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

lib/bindings/python/tests/conftest.py (1)

405-440: Parametrized runtime fixture implementation looks correct; consider minor clarifications

Using params=["tcp", "nats"] and request.param to feed request_plane into DistributedRuntime is idiomatic pytest and will correctly run each runtime-using test against both transport modes. Lifecycle logic and the singleton/forked guard remain unchanged, so there are no new correctness or isolation issues from this change.

Two small, optional improvements you might consider:

• Add a note to the runtime docstring that tests are executed for both "tcp" and "nats" request planes so readers understand the expanded coverage.
• Optionally give the params explicit IDs, e.g. params=[pytest.param("tcp", id="tcp"), pytest.param("nats", id="nats")], to make test names a bit clearer in pytest output.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 164b0c2 and 188448f.

📒 Files selected for processing (1)

• lib/bindings/python/tests/conftest.py (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

lib/bindings/python/tests/conftest.py (2)

lib/runtime/src/distributed.rs (2)

• request_plane (438-440)
• runtime (291-293)

lib/bindings/python/src/dynamo/_core.pyi (1)

• DistributedRuntime (35-58)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)

• GitHub Check: vllm (amd64)
• GitHub Check: trtllm (arm64)
• GitHub Check: vllm (arm64)
• GitHub Check: operator (arm64)
• GitHub Check: operator (amd64)
• GitHub Check: trtllm (amd64)
• GitHub Check: sglang (arm64)
• GitHub Check: sglang (amd64)
• GitHub Check: Build and Test - dynamo

[biswapanda]

@CodeRabbit review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[nnshah1]

@pvijayakrish , @biswapanda

Is there a way to mark this for post merge / nightly (the "tcp" version?)

[nnshah1]

/ok to test 188448f

[kthui]

Not sure if the goal here is to enable unit testing for both TCP and NATS first, and after switching the default from NATS to TCP, all other tests including E2E will be switched to testing on TCP and the unit tests will still provide minimal testing on NATS?

[biswapanda]

Not sure if the goal here is to enable unit testing for both TCP and NATS first, and after switching the default from NATS to TCP, all other tests including E2E will be switched to testing on TCP and the unit tests will still provide minimal testing on NATS?

In my head, general guideline is - we'd test whatever features customers are using,
we'd support tests as long as there are customers using a feature

1. test both nats + tcp
2. when all users have moved off nats => disable nats test and remove them

tcp is always on from now on. nats will be off once all users are not using nats anymore.