[None][test] use skip_less_mpi_world_size instead of skip_less_device

[crazydemo]

Dev Engineer Review

• Updated multi-GPU test skip conditions from skip_less_device to skip_less_mpi_world_size.
• The changes target tests that require multiple MPI processes.
• Test logic remains unchanged.
• The updates are consistent across the DeepSeek, Kimi, Qwen, MiniMax, GLM, Step-3.7, and eight-process end-to-end tests.
• No API, configuration, or test-list changes were identified.
• Disaggregated serving tests were not included in the update.

QA Engineer Review

• Modified test files:
  • tests/integration/defs/accuracy/test_llm_api_pytorch.py
  • tests/integration/defs/test_e2e.py
• Modified tests use MPI world-size availability for multi-process requirements.
• No test functions were added or removed.
• Existing test coverage remains unchanged.
• Test-list coverage was not modified.
• Verdict: sufficient.

Description

Test Coverage

PR Checklist

Please review the following before submitting your PR:

• PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

• PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

• Test cases are provided for new code paths (see test instructions)

• If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

• Any new dependencies have been scanned for license and vulnerabilities

• CODEOWNERS updated if ownership changes

• Documentation updated as needed

• Update tava architecture diagram if there is a significant design change in PR.

• The reviewers assigned automatically/manually are appropriate for the PR.

• Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1b6d8abf-0357-4d12-adb9-8bac138d17ad

📥 Commits

Reviewing files that changed from the base of the PR and between 83ed5e7 and 1fdde22.

📒 Files selected for processing (2)

• tests/integration/defs/accuracy/test_llm_api_pytorch.py
• tests/integration/defs/test_e2e.py

🚧 Files skipped from review as they are similar to previous changes (2)

• tests/integration/defs/accuracy/test_llm_api_pytorch.py
• tests/integration/defs/test_e2e.py

---

Walkthrough

The integration tests now check for eight MPI processes instead of eight local devices before running multi-GPU scenarios. Test logic remains unchanged.

Changes

MPI test gating

Layer / File(s)	Summary
PyTorch LLM API prerequisites tests/integration/defs/accuracy/test_llm_api_pytorch.py	Fourteen model accuracy tests now use MPI world-size checks for eight-process requirements.
End-to-end test prerequisites tests/integration/defs/test_e2e.py	Seven end-to-end tests and parameterizations now use eight-process MPI world-size checks.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

• NVIDIA/TensorRT-LLM#17392: Both PRs modify multi-GPU test skip conditions in the same integration test files.

Suggested reviewers: schetlur-nv, chienchunhung, qijune

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description includes the template and checklist but leaves the required Description and Test Coverage sections empty.	Add a short explanation of the issue and solution, and list the relevant tests or CI coverage.
Docstring Coverage	⚠️ Warning	Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows the required format and clearly describes the test skip-logic change.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[nv-xtf]

LGTM. Just a note: we should avoid putting these disagg tests into multi-node stages — launch_disaggregated_llm slices GPUs by local index

[crazydemo]

LGTM. Just a note: we should avoid putting these disagg tests into multi-node stages — launch_disaggregated_llm slices GPUs by local index

Thanks. Revert changes on test_disaggreagted_serving.py

[crazydemo]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #64911 [ run ] triggered by Bot. Commit: 1fdde22 Link to invocation

[fredricz-20070104]

Review summary - CONCERNS

Verdict: Test-only marker swap; it is likely correct for multi-node stages but carries a real coverage-regression risk on single-node runs that should be confirmed before merge.

Concerns

1. [MAJOR] tests/integration/defs/accuracy/test_llm_api_pytorch.py:3324 (and every other switched decorator) - skip_less_device(8) -> skip_less_mpi_world_size(8) changes the gating semantics
   • What is wrong: skip_less_device(N) gates on the number of visible/local CUDA devices; skip_less_mpi_world_size(N) gates on the MPI world size. These are not equivalent. Several of these LLM-API accuracy tests (e.g. TestDeepSeekV3 tp=8, TestQwen3_235B_A22B.test_fp8, TestGLM52.test_nvfp4) run in a single process that internally spawns the tp workers, in which case the MPI world size is 1 even though 8 GPUs are present.
   • How it fails: on a single-node 8-GPU CI host launched without an 8-rank mpirun, skip_less_mpi_world_size(8) sees world_size(1) < 8 and skips the test. Cases that previously ran under skip_less_device(8) then silently stop running with no failing signal — a coverage regression. It only keeps running if these tests are always launched under an 8-rank MPI world.
   • Suggested fix: confirm in the PR description that every touched test is executed via trtllm-llmapi-launch/mpirun with world_size == device_count in the stage(s) that own it. Where a test genuinely runs single-process, keep skip_less_device.

Minor notes (non-blocking)

• tests/integration/defs/accuracy/test_llm_api_pytorch.py:4137 and tests/integration/defs/test_e2e.py:1456 - mixed gating within one parametrize (4-GPU case keeps skip_less_device(4), 8-GPU case becomes skip_less_mpi_world_size(8)). Probably intentional, but add a one-line comment explaining why only the 8-way case uses world-size gating.

QA view

• Test coverage: unknown - this changes the skip predicate itself, not a production path. Whether the tests still execute depends on the launch mode (MPI world size vs local devices), which the diff does not reveal.
• SM coverage: architecture-independent - the skip_pre_blackwell/skip_pre_hopper/skip_pre_ada guards are untouched; only the device/world-size predicate changed.
• Test code: mixed gating within single parametrize blocks; also cannot confirm from the diff that skip_less_mpi_world_size is a registered pytest marker (unregistered -> collection error).
• Test time: unknown - no bodies/models/parametrisations changed; net effect depends on whether more environments now run (multi-node) or fewer run (single-node world_size=1).
• Needs /qa-verify: yes - this is a test-infra gating change; QA should confirm the 8-way tests are launched with world_size==8 in their stages and are not silently skipped on single-node 8-GPU hosts.

Possible new issues

• Silent skip of 8-GPU tests on single-node hosts where MPI world size is 1.
• If skip_less_mpi_world_size is not registered, pytest collection errors out.
• Reviewer nv-xtf noted launch_disaggregated_llm slices GPUs by local index in multi-node stages; moving these tests into multi-node stages could misplace GPUs (PR states disagg tests were excluded).

What I could not verify

• The definition of skip_less_mpi_world_size and whether it is a registered marker.
• The actual launch mechanism (single-process vs N-rank MPI) for each touched test in its CI stage - this is the crux of whether coverage is preserved or regressed.

Automated review by NVCortex Lite, run by @fredricz-20070104.

[fredricz-20070104]

Review summary - Approve (non-blocking)

Approving so this is not blocked on me. The points raised in my review comment above are non-blocking — please read them and address what you agree with before merging.

Worth doing before this is relied on: This modifies test infrastructure gating. QA must confirm the touched 8-way tests are actually launched with MPI world_size==8 in their CI stages (so they still run) and are not silently skipped on single-node 8-GPU hosts, and that skip_less_mpi_world_size is a registered marker.

Automated review by NVCortex Lite, run by @fredricz-20070104.

[tensorrt-cicd]

PR_Github #64911 [ run ] completed with state SUCCESS. Commit: 1fdde22
/LLM/main/L0_MergeRequest_PR pipeline #52752 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

• Please check the failed tests and fix your PR
• If you cannot view the failures, ask the CI triggerer to share details
• Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation