Repository navigation
[https://nvbugs/6988813][test] Unwaive remote MPI submit_sync - #21205
Conversation
Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
|
/bot run --disable-fail-fast --disable-reuse-test --stage-list "CPU-Generic-x86-1,CPU-Generic-x86-2,CPU-Generic-arm-1,CPU-Generic-arm-2" |
|
PR_Github #81239 [ run ] triggered by Bot. Commit: |
|
PR_Github #81239 [ run ] completed with state |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Remove the test waiver tests/integration/test_lists/waives.txt |
The skip-list entry for test_remote_mpi_session[submit_sync] and its NVBUG reference are removed. |
Priority:
Estimated code review effort: 1 (Trivial) | ~2 minutes
Change: Other
Possibly related PRs
- NVIDIA/TensorRT-LLM#20960: Adds the same test waiver that this change removes.
- NVIDIA/TensorRT-LLM#20434: Changes MPI shutdown behavior and adds stop-helper coverage used by the remote MPI session implementation.
- NVIDIA/TensorRT-LLM#20467: Changes
MpiSession.submitandsubmit_syncscheduling and updates the remote MPI session test harness.
Suggested reviewers: trtllm-agent
Merge Risk: 
e31ef
-
The synchronous remote MPI test is now eligible to run in CI. No actionable merge risk is identified.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly identifies the NVBugs ticket, change type, and primary change: removing the waiver for the remote MPI submit_sync test. Description check The description explains the change, provides detailed verification results and test coverage, documents limitations, and confirms the repository checklist was reviewed. The alternative section names … Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description explains the change, provides detailed verification results and test coverage, documents limitations, and confirms the repository checklist was reviewed. The alternative section names still contain the required information.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files.
-
Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
/bot skip --comment "Test unwaiving only; verified with the targeted CI pipeline." |
|
PR_Github #81271 [ skip ] triggered by Bot. Commit: |
|
PR_Github #81271 [ skip ] completed with state |
|
Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements. |
Summary
Remove the NVBUG 6988813 waiver for
test_remote_mpi_session[submit_sync]so CI exercises it again on ARM and x86.The relevant runtime change is #20434's lightweight MPI shutdown helper. Main also includes #20467's test-harness changes and 120-second subprocess bound. This PR only removes the waiver and validates the combined implementation.
Verification
Tested revision:
e31ef8a72f57682b4f451d3884955236446ba852, based on main3b4eebcf020e621fe47d54c1f6bea2dcfeb6dd36.Targeted CI #66929 completed successfully, covering
CPU-Generic-arm-1,CPU-Generic-arm-2,CPU-Generic-x86-1, andCPU-Generic-x86-2. The trigger disabled test-result reuse.JUnit confirms execution of the exact affected test,
unittest/llmapi/test_mpi_session.py::test_remote_mpi_session[submit_sync]:CPU-Generic-arm-1CPU-Generic-x86-1test_remote_mpi_sessionvariants passed on each architecture (12/12 checks).test_cancel_after_publication_cannot_overtake_request_data; the affected MPI test was absent from the retry report.git diff --checkpassed. Self-review found no actionable issues.This run verifies the unwaived test on the combined current implementation. It does not isolate #20434's effect from #20467 or establish that the historical intermittent failure is eliminated; subsequent CI runs will provide continued coverage.
Dev Engineer Review
Removed the NVBUG 6988813 waiver for
test_remote_mpi_session[submit_sync]. This changes test selection only; no runtime or test-harness code changed.QA Engineer Review
Modified
tests/integration/test_lists/waives.txtto remove the waiver forunittest/llmapi/test_mpi_session.py::test_remote_mpi_session[submit_sync]. The targeted CI run reported this test passed on ARM and x86, without being skipped; it validates the combined implementation, not the isolated effect of either related change. No matching entry was found in the supplied search oftest-db/orqa/lists. Coverage verdict: sufficient for the waiver removal.Per-File QA Perspective
tests/integration/test_lists/waives.txt: Removing this entry allows CI to run the affected MPI test instead of waiving it. The supplied CI results report a pass on ARM and x86.