Repository navigation
[TRTLLM-17261][infra] Give the trtllm-serve test directories the owners of the code they test - #21407
JunyiXu-nv wants to merge 1 commit into
Conversation
…rs of the code they test Add three CODEOWNERS rules so that test ownership follows source ownership, as it already does for /tests/unittest/llmapi and /tests/unittest/_torch/visual_gen: - /tests/test_common/serve -> @NVIDIA/trt-llm-runtime-devs (shared fixtures and fakes for trtllm-serve tests) - /tests/unittest/serve -> @NVIDIA/trt-llm-runtime-devs (tests mirroring tensorrt_llm/serve/, owned by runtime-devs) - /tests/unittest/serve/api/visual_gen -> @NVIDIA/trt-llm-torch-visual-gen-devs (VisualGen serving tests) Without these rules the directories fall to @NVIDIA/trt-llm-devs through the broad /tests/test_common and /tests/unittest fallback rules. The directories do not exist on main yet; a CODEOWNERS rule that matches no file is valid to GitHub and takes effect as soon as the upcoming test-only change creates them. Each line is placed after the broader rule it overrides, because the last matching pattern wins. No behavior change: no code, test or test-list change. Signed-off-by: JunyiXu-nv <219237550+JunyiXu-nv@users.noreply.github.com>
|
/bot run |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Assign owners to serve test paths .github/CODEOWNERS |
The runtime team is assigned /tests/test_common/serve and /tests/unittest/serve. The VisualGen team is assigned /tests/unittest/serve/api/visual_gen. |
Estimated code review effort: 1 (Trivial) | ~3 minutes
Change: Other
Suggested reviewers: tburt-nv
Merge Risk: 
cfd3d
-
This metadata-only change is ready to merge; the new ownership rules have no demonstrated functional impact.
Pre-merge checks: 
-
Check name Status Explanation Title check The title names the ticket, the type, and the main change: the trtllm-serve test directories are given the owners of the code they test. It matches the three CODEOWNERS rules in the diff. It follows t… 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. Description check The pull request description is complete and follows the required structure. It explains the purpose, affected CODEOWNERS rules, behavior, related work, validation checks, test coverage, and checklist… Full details: Title check
-
Explanation
The title names the ticket, the type, and the main change: the trtllm-serve test directories are given the owners of the code they test. It matches the three CODEOWNERS rules in the diff. It follows the repository title template, and the title is short and specific.
Full details: Description check
-
Explanation
The pull request description is complete and follows the required structure. It explains the purpose, affected CODEOWNERS rules, behavior, related work, validation checks, test coverage, and checklist status.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
|
PR_Github #81900 [ run ] triggered by Bot. Commit: |
|
PR_Github #81900 [ run ] completed with state |
Dev Engineer Review
.github/CODEOWNERSadds three ownership rules for serve test directories. The rules assign/tests/test_common/serveand/tests/unittest/serveto@NVIDIA/trt-llm-runtime-devs, and/tests/unittest/serve/api/visual_gento@NVIDIA/trt-llm-torch-visual-gen-devs. These are metadata changes; the supplied diff shows no code changes.QA Engineer Review
No test changes.
Per-File QA Perspective
.github/CODEOWNERS: The update changes review ownership for the specified test directories. It does not change test behavior or CI workflows.Description
Adds three
.github/CODEOWNERSrules so that ownership of the trtllm-serve test directories follows ownership of the code they test, as it already does for/tests/unittest/llmapiand/tests/unittest/_torch/visual_gen. Part of the trtllm-serve refactor (epic TRTLLM-17056), task TRTLLM-17261 (pre-move guards and landing zone)..github/CODEOWNERS:138tests/test_common/serve/— shared fixtures and fakes that trtllm-serve tests build servers with@NVIDIA/trt-llm-runtime-devs.github/CODEOWNERS:159tests/unittest/serve/— tests mirroringtensorrt_llm/serve/@NVIDIA/trt-llm-runtime-devs.github/CODEOWNERS:382tests/unittest/serve/api/visual_gen/— VisualGen serving tests@NVIDIA/trt-llm-torch-visual-gen-devstensorrt_llm/serve/belongs to@NVIDIA/trt-llm-runtime-devsand its VisualGen serving part to the VisualGen team, so the tests go to the same teams. Without these rules the three directories fall to@NVIDIA/trt-llm-devsthrough the broad/tests/test_commonand/tests/unittestfallback rules. Each new line is placed after the broader rule it overrides (the file resolves by last matching pattern) and in sorted position within its section, with one owning team per line: several owners on one line are alternatives, which would make the intended team's review optional.The directories do not exist on
mainyet: they are created by #21406 ("[TRTLLM-17261][test] Add route, middleware-order and shim-identity snapshots and a CPU test seam for trtllm-serve", open), which may merge before or after this PR. A CODEOWNERS rule whose path matches no file is valid to GitHub and simply takes effect once the directories land; until this PR merges, files there are owned by@NVIDIA/trt-llm-devs.Related: #21170 ("[TRTLLM-17261][infra] Add trtllm-serve CI coverage lint and a multi-GPU trigger for chat_tokenization.py", open) adds a CI lint whose data file lists required CODEOWNERS rules. That PR is not on
main, so this PR does not touch its data file; whichever of the two merges second adds the three test-directory rows there, and from then on the lint checks the owners of these directories on every run.Once this PR is on
main, the component labeler (.github/scripts/label_component.py) adds the advisoryVisualGenlabel to PRs that change files undertests/unittest/serve/api/visual_gen/.Behavior
No behavior change: no code, test or test-list change. The only effects are the required reviewers for the three (not yet existing) test directories and the advisory
VisualGenlabel.Moves
None.
CI bookkeeping
legacy-files.txt/ruff-legacy-baseline.json: noneTest Coverage
No test changes (CI metadata only). Checks run locally on the PR head, rebased onto
mainata9be7004c7:git diff origin/main -- .github/CODEOWNERS: exactly the three added lines, nothing removed; no other file changed..github/scripts/label_component.py), base vs this PR:main(base)tests/test_common/serve/fakes.py@NVIDIA/trt-llm-devs@NVIDIA/trt-llm-runtime-devstests/unittest/serve/test_layering.py@NVIDIA/trt-llm-devs@NVIDIA/trt-llm-runtime-devstests/unittest/serve/api/visual_gen/test_routes_visual_gen.py@NVIDIA/trt-llm-devs@NVIDIA/trt-llm-torch-visual-gen-devstests/test_common/llm_data.py(neighbor)@NVIDIA/trt-llm-devs@NVIDIA/trt-llm-devs(unchanged)tests/unittest/api_stability/test_serve_routes.py(neighbor)@NVIDIA/trt-llm-noncommitted-api-review-committee/tests/unittest/serveline before the broad/tests/unittestrule makes the owner check print@NVIDIA/trt-llm-devsagain, confirming the last-match placement is load-bearing.pre-commit run --files .github/CODEOWNERS: passes, twice, with the file unmodified by hooks.gh api "repos/NVIDIA/TensorRT-LLM/codeowners/errors?ref=refs/pull/21407/head"): 16 "Unknown owner" errors, all pre-existing —diffof the error sources againstmain's is empty (the same 16 errors exist onmainat lines 480-508; on this head they sit 3 lines lower, matching the three insertions). No error on the three new lines..github/CODEOWNERSre-checked at rebase time ([None][chore] Add build system codeowners #7683, [None][chore] Update code owners for KVCacheManagerV2 #11977, [None][feat] Add PrimsTS MoE with Rubin locality domains #19115, [None][feat] Muse Spark Open modeling_v2 TEP target: low-latency optimizations for GB300 and Vera Rubin #20625, [None][feat] MiniMax-M3.1 NVFP4 context (prefill) path on Rubin (SM107) #21039, [None][feat] Staircase: Rubin GLM Optimized Low Latency (Conc up to 56) #21380): none adds a rule matching these paths or touches the neighboring lines.OSS-Compliance-Checkstage runs because.github/CODEOWNERSchanged.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-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin 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.🤖 Generated with Claude Code