[None][test] fix MPI pool fixture parametrization

[cascade812]

Description

Fix the shared mpi_pool_executor fixture so it no longer parametrizes worker counts at the fixture definition level. Tests that need a specific MPI pool size already pass it with indirect parametrization, and the fixture now defaults to 2 workers when no parameter is provided.

Failed tests
GB10-PyTorch-1/test_unittests.py::test_unittests_v2[unittest/_torch/modeling -k "modeling_out_of_tree"]
_torch/modeling/test_modeling_pixtral.py::test_tensor_parallelism: duplicate parametrization of 'mpi_pool_executor'

Root cause

With newer pytest, collecting a directory that contains tests indirectly parametrizing mpi_pool_executor fails because the fixture itself also declares params=[2, 4, 8]. This can break unrelated -k selections during collection, such as the out-of-tree modeling bridge.

Validation

• PYTHONPYCACHEPREFIX=/private/tmp/tensorrt-llm-pycache python3 -m py_compile tests/unittest/conftest.py
• git diff --check -- tests/unittest/conftest.py

Full pytest was not run locally because this environment does not have the repo test dependencies/GPU setup.

Summary by CodeRabbit

• Tests

  • Simplified test framework configuration for worker pool handling with streamlined defaults.

• Chores

  • Updated copyright year to 2026.

[coderabbitai]

📝 Walkthrough

Walkthrough

The mpi_pool_executor pytest fixture in tests/unittest/conftest.py is changed from a parametrized fixture with fixed worker counts [2, 4, 8] to a non-parametrized fixture that reads num_workers from request.param when provided, defaulting to 2. The SPDX copyright year is also updated from 2024 to 2026.

Changes

mpi_pool_executor fixture update

Layer / File(s)	Summary
mpi_pool_executor fixture: optional param with default tests/unittest/conftest.py	Fixture decorator changed from @pytest.fixture(scope="module", params=[2, 4, 8]) to @pytest.fixture(scope="module"); num_workers is now set via getattr(request, "param", 2) instead of always iterating over three fixed values. Copyright year updated to 2026.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: fixing the MPI pool fixture parametrization by removing params from the fixture definition level.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Description check	✅ Passed	The PR description clearly explains the issue, root cause, validation performed, and the solution for the pytest fixture parametrization problem.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

🧹 Nitpick comments (1)

tests/unittest/conftest.py (1)

340-345: 💤 Low value

Fixture change looks good; consider updating the docstring.

The change from params=[2, 4, 8] to optional request.param with a default of 2 correctly implements the intended behavior. The getattr(request, "param", 2) pattern properly handles both indirect parametrization and non-parametrized cases.

However, the docstring still says "Start an MPIPoolExecutor with request.param workers" without mentioning the default. Consider updating it for clarity.

📝 Suggested docstring update

     """
-    Start an MPIPoolExecutor with `request.param` workers.
+    Start an MPIPoolExecutor with the number of workers specified via
+    indirect parametrization (default: 2 workers).
     """

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unittest/conftest.py` around lines 340 - 345, Update the docstring for
the mpi_pool_executor fixture to reflect the new behavior. The current docstring
states "Start an MPIPoolExecutor with `request.param` workers" but does not
mention the default value. Modify the docstring to clarify that it starts an
MPIPoolExecutor with the number of workers specified by request.param,
defaulting to 2 if request.param is not provided (for non-parametrized usage).

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/unittest/conftest.py`:
- Around line 340-345: Update the docstring for the mpi_pool_executor fixture to
reflect the new behavior. The current docstring states "Start an MPIPoolExecutor
with `request.param` workers" but does not mention the default value. Modify the
docstring to clarify that it starts an MPIPoolExecutor with the number of
workers specified by request.param, defaulting to 2 if request.param is not
provided (for non-parametrized usage).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 996e2669-9b81-4581-852a-8b1b8dcc4d8c

📥 Commits

Reviewing files that changed from the base of the PR and between 4f46653 and b9d8dea.

📒 Files selected for processing (1)

• tests/unittest/conftest.py

[cascade812]

/bot run

[tensorrt-cicd]

PR_Github #54151 [ run ] triggered by Bot. Commit: b9d8dea Link to invocation

[tensorrt-cicd]

PR_Github #54151 [ run ] completed with state SUCCESS. Commit: b9d8dea
/LLM/main/L0_MergeRequest_PR pipeline #43234 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

[cascade812]

/bot run

[tensorrt-cicd]

PR_Github #54162 [ run ] triggered by Bot. Commit: b9d8dea Link to invocation

[cascade812]

/bot run

[tensorrt-cicd]

PR_Github #54169 [ run ] triggered by Bot. Commit: 387289f Link to invocation

[tensorrt-cicd]

PR_Github #54162 [ run ] completed with state ABORTED. Commit: b9d8dea

Link to invocation

[cascade812]

/bot run

[tensorrt-cicd]

PR_Github #54170 [ run ] triggered by Bot. Commit: 387289f Link to invocation

[tensorrt-cicd]

PR_Github #54169 [ run ] completed with state ABORTED. Commit: 387289f

Link to invocation

[tensorrt-cicd]

PR_Github #54170 [ run ] completed with state FAILURE. Commit: 387289f
/LLM/main/L0_MergeRequest_PR pipeline #43253 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

[cascade812]

/bot run

[tensorrt-cicd]

PR_Github #54175 [ run ] triggered by Bot. Commit: ff5feb5 Link to invocation

[tensorrt-cicd]

PR_Github #54175 [ run ] completed with state FAILURE. Commit: ff5feb5
/LLM/main/L0_MergeRequest_PR pipeline #43258 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

[cascade812]

/bot run

[tensorrt-cicd]

PR_Github #54190 [ run ] triggered by Bot. Commit: ff5feb5 Link to invocation