Skip to content

Conversation

@EmmaQiaoCh
Copy link
Collaborator

@EmmaQiaoCh EmmaQiaoCh commented Nov 19, 2025

Summary by CodeRabbit

  • Tests
    • Expanded test coverage for GB10 Tensorrt configurations in the post-merge testing pipeline
    • Added integration test for LLM API quickstart functionality across specific hardware and system configurations

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)

  • 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

/bot [-h] ['run', 'kill', 'skip', 'reuse-pipeline'] ...

Provide a user friendly way for developers to interact with a Jenkins server.

Run /bot [-h|--help] to print this help message.

See details below for each supported subcommand.

run [--reuse-test (optional)pipeline-id --disable-fail-fast --skip-test --stage-list "A10-PyTorch-1, xxx" --gpu-type "A30, H100_PCIe" --test-backend "pytorch, cpp" --add-multi-gpu-test --only-multi-gpu-test --disable-multi-gpu-test --post-merge --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" --detailed-log --debug(experimental)]

Launch build/test pipelines. All previously running jobs will be killed.

--reuse-test (optional)pipeline-id (OPTIONAL) : Allow the new pipeline to reuse build artifacts and skip successful test stages from a specified pipeline or the last pipeline if no pipeline-id is indicated. If the Git commit ID has changed, this option will be always ignored. The DEFAULT behavior of the bot is to reuse build artifacts and successful test results from the last pipeline.

--disable-reuse-test (OPTIONAL) : Explicitly prevent the pipeline from reusing build artifacts and skipping successful test stages from a previous pipeline. Ensure that all builds and tests are run regardless of previous successes.

--disable-fail-fast (OPTIONAL) : Disable fail fast on build/tests/infra failures.

--skip-test (OPTIONAL) : Skip all test stages, but still run build stages, package stages and sanity check stages. Note: Does NOT update GitHub check status.

--stage-list "A10-PyTorch-1, xxx" (OPTIONAL) : Only run the specified test stages. Examples: "A10-PyTorch-1, xxx". Note: Does NOT update GitHub check status.

--gpu-type "A30, H100_PCIe" (OPTIONAL) : Only run the test stages on the specified GPU types. Examples: "A30, H100_PCIe". Note: Does NOT update GitHub check status.

--test-backend "pytorch, cpp" (OPTIONAL) : Skip test stages which don't match the specified backends. Only support [pytorch, cpp, tensorrt, triton]. Examples: "pytorch, cpp" (does not run test stages with tensorrt or triton backend). Note: Does NOT update GitHub pipeline status.

--only-multi-gpu-test (OPTIONAL) : Only run the multi-GPU tests. Note: Does NOT update GitHub check status.

--disable-multi-gpu-test (OPTIONAL) : Disable the multi-GPU tests. Note: Does NOT update GitHub check status.

--add-multi-gpu-test (OPTIONAL) : Force run the multi-GPU tests in addition to running L0 pre-merge pipeline.

--post-merge (OPTIONAL) : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline.

--extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" (OPTIONAL) : Run the ordinary L0 pre-merge pipeline and specified test stages. Examples: --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx".

--detailed-log (OPTIONAL) : Enable flushing out all logs to the Jenkins console. This will significantly increase the log volume and may slow down the job.

--debug (OPTIONAL) : Experimental feature. Enable access to the CI container for debugging purpose. Note: Specify exactly one stage in the stage-list parameter to access the appropriate container environment. Note: Does NOT update GitHub check status.

For guidance on mapping tests to stage names, see docs/source/reference/ci-overview.md
and the scripts/test_to_stage_mapping.py helper.

kill

kill

Kill all running builds associated with pull request.

skip

skip --comment COMMENT

Skip testing for latest commit on pull request. --comment "Reason for skipping build/test" is required. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.

reuse-pipeline

reuse-pipeline

Reuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.

@EmmaQiaoCh EmmaQiaoCh requested review from a team as code owners November 19, 2025 13:17
@EmmaQiaoCh EmmaQiaoCh requested review from kxdc and poweiw November 19, 2025 13:17
@EmmaQiaoCh EmmaQiaoCh marked this pull request as draft November 19, 2025 13:17
@EmmaQiaoCh
Copy link
Collaborator Author

/bot run --stage-list "GB10-Tensorrt-Post-Merge-1"

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 19, 2025

📝 Walkthrough

Walkthrough

Updates the Jenkins shared library reference to a feature branch and adds a new GB10 Tensorrt test configuration to the SBSA Slurm test suite. A corresponding test definition file is created to specify the test group conditions and test cases.

Changes

Cohort / File(s) Change Summary
Jenkins Configuration Update
jenkins/L0_Test.groovy
Updated library annotation to reference bloom-jenkins-shared-lib@emma/add_spark_for_slurm instead of @main. Added new SBSA Slurm test entry GB10-Tensorrt-Post-Merge-1 with parameters ["gb10x", "l0_gb10", 1, 1].
Test Definition
tests/integration/test_lists/l0_gb10.xml
Created new test configuration file defining test group l0_gb10 with conditions for GB10 GPU on aarch64 Ubuntu systems. Configured to run at post-merge stage with tensorrt backend, containing test reference llmapi/test_llm_examples.py::test_llmapi_quickstart_atexit.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify that the Jenkins library branch reference emma/add_spark_for_slurm is correct and intentional (temporary branch or merged feature).
  • Confirm the new test entry parameters (gb10x, l0_gb10, 1, 1) align with naming conventions and expected GPU/configuration mappings.
  • Validate that the XML test conditions (GPU filter, system specs) correctly target GB10 hardware configurations.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is largely unfilled; it contains only the template structure and placeholder comments without substantive content in the Description or Test Coverage sections. Add explicit description explaining the infrastructure change (Spark library integration, GB10 test addition), and clearly list the test coverage provided by l0_gb10.xml and the new SBSA test entry.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: enabling single-GPU CI on Spark, directly reflecting the PR's purpose of adding new GB10 test configuration with single-GPU parameters.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent 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 46dd988 and 1481e89.

📒 Files selected for processing (2)
  • jenkins/L0_Test.groovy (2 hunks)
  • tests/integration/test_lists/l0_gb10.xml (1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-09-09T09:40:45.658Z
Learnt from: fredricz-20070104
Repo: NVIDIA/TensorRT-LLM PR: 7645
File: tests/integration/test_lists/qa/llm_function_core.txt:648-648
Timestamp: 2025-09-09T09:40:45.658Z
Learning: In TensorRT-LLM test lists, it's common and intentional for the same test to appear in multiple test list files when they serve different purposes (e.g., llm_function_core.txt for comprehensive core functionality testing and llm_function_core_sanity.txt for quick sanity checks). This duplication allows tests to be run in different testing contexts.

Applied to files:

  • tests/integration/test_lists/l0_gb10.xml
  • jenkins/L0_Test.groovy
📚 Learning: 2025-09-17T02:48:52.732Z
Learnt from: tongyuantongyu
Repo: NVIDIA/TensorRT-LLM PR: 7781
File: tests/integration/test_lists/waives.txt:313-313
Timestamp: 2025-09-17T02:48:52.732Z
Learning: In TensorRT-LLM, `tests/integration/test_lists/waives.txt` is specifically for waiving/skipping tests, while other test list files like those in `test-db/` and `qa/` directories are for different test execution contexts (pre-merge, post-merge, QA tests). The same test appearing in both waives.txt and execution list files is intentional - the test is part of test suites but will be skipped due to the waiver.

Applied to files:

  • tests/integration/test_lists/l0_gb10.xml
📚 Learning: 2025-08-26T09:49:04.956Z
Learnt from: pengbowang-nv
Repo: NVIDIA/TensorRT-LLM PR: 7192
File: tests/integration/test_lists/test-db/l0_dgx_b200.yml:56-72
Timestamp: 2025-08-26T09:49:04.956Z
Learning: In TensorRT-LLM test configuration files, the test scheduling system handles wildcard matching with special rules that prevent duplicate test execution even when the same tests appear in multiple yaml files with overlapping GPU wildcards (e.g., "*b200*" and "*gb200*").

Applied to files:

  • tests/integration/test_lists/l0_gb10.xml
  • jenkins/L0_Test.groovy
📚 Learning: 2025-08-13T11:07:11.772Z
Learnt from: Funatiq
Repo: NVIDIA/TensorRT-LLM PR: 6754
File: tests/integration/test_lists/test-db/l0_a30.yml:41-47
Timestamp: 2025-08-13T11:07:11.772Z
Learning: In TensorRT-LLM test configuration files like tests/integration/test_lists/test-db/l0_a30.yml, TIMEOUT values are specified in minutes, not seconds.

Applied to files:

  • tests/integration/test_lists/l0_gb10.xml
📚 Learning: 2025-08-06T13:58:07.506Z
Learnt from: galagam
Repo: NVIDIA/TensorRT-LLM PR: 6487
File: tests/unittest/_torch/auto_deploy/unit/singlegpu/test_ad_trtllm_bench.py:1-12
Timestamp: 2025-08-06T13:58:07.506Z
Learning: In TensorRT-LLM, test files (files under tests/ directories) do not require NVIDIA copyright headers, unlike production source code files. Test files typically start directly with imports, docstrings, or code.

Applied to files:

  • tests/integration/test_lists/l0_gb10.xml
📚 Learning: 2025-07-28T17:06:08.621Z
Learnt from: moraxu
Repo: NVIDIA/TensorRT-LLM PR: 6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.

Applied to files:

  • tests/integration/test_lists/l0_gb10.xml
📚 Learning: 2025-08-29T14:07:45.863Z
Learnt from: EmmaQiaoCh
Repo: NVIDIA/TensorRT-LLM PR: 7370
File: tests/unittest/trt/model_api/test_model_quantization.py:24-27
Timestamp: 2025-08-29T14:07:45.863Z
Learning: In TensorRT-LLM's CI infrastructure, pytest skip markers (pytest.mark.skip) are properly honored even when test files have __main__ blocks that call test functions directly. The testing system correctly skips tests without requiring modifications to the __main__ block execution pattern.

Applied to files:

  • tests/integration/test_lists/l0_gb10.xml
📚 Learning: 2025-08-11T20:09:24.389Z
Learnt from: achartier
Repo: NVIDIA/TensorRT-LLM PR: 6763
File: tests/integration/defs/triton_server/conftest.py:16-22
Timestamp: 2025-08-11T20:09:24.389Z
Learning: In the TensorRT-LLM test infrastructure, the team prefers simple, direct solutions (like hard-coding directory traversal counts) over more complex but robust approaches when dealing with stable directory structures. They accept the maintenance cost of updating tests if the layout changes.

Applied to files:

  • tests/integration/test_lists/l0_gb10.xml
📚 Learning: 2025-08-01T15:14:45.673Z
Learnt from: yibinl-nvidia
Repo: NVIDIA/TensorRT-LLM PR: 6506
File: examples/models/core/mixtral/requirements.txt:3-3
Timestamp: 2025-08-01T15:14:45.673Z
Learning: In TensorRT-LLM, examples directory can have different dependency versions than the root requirements.txt file. Version conflicts between root and examples dependencies are acceptable because examples are designed to be standalone and self-contained.

Applied to files:

  • tests/integration/test_lists/l0_gb10.xml
⏰ 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). (1)
  • GitHub Check: Pre-commit Check
🔇 Additional comments (3)
tests/integration/test_lists/l0_gb10.xml (1)

1-17: Code review verified and approved.

The test reference llmapi/test_llm_examples.py::test_llmapi_quickstart_atexit exists in the codebase, the single-test configuration is consistent with other hardware platform configurations (l0_h100.yml, l0_a100.yml, l0_gb300.yml, etc.), and the wildcard pattern *gb10* follows the established naming convention used throughout the test-db directory. No issues found.

jenkins/L0_Test.groovy (2)

1-1: Verify the current status of the feature branch reference before merging.

The feature branch emma/add_spark_for_slurm no longer exists (returned 404), but I cannot definitively confirm whether it was already merged to main or deleted. Before this PR is merged, manually verify:

  1. Whether the feature branch was successfully merged to main in the bloom-jenkins-shared-lib repository
  2. If merged, confirm the library reference is updated to main (or appropriate stable branch)
  3. If not merged, coordinate the merge or update the reference accordingly

This ensures the pipeline will not break due to a stale or missing branch reference.


2763-2763: The review comment is based on a misunderstanding of how the test configuration system works.

The test scheduling system uses wildcard matching to connect test definitions to runtime infrastructure. The l0_gb10.xml file contains gpu: ['*gb10*'] which will match any partition name containing "gb10" at runtime. The platform identifier "gb10x" is an external Slurm infrastructure name, not something that needs to be defined in the codebase.

This approach is identical to the existing GB200 and GH200 configurations:

  • l0_gb200_multi_gpus.yml specifies gpu: ['*gb200*'] and maps to platform identifiers like gb200-trtllm and gb200-x4-oci
  • No explicit platform definitions exist in the codebase for these either

The code change is correct and requires no modifications.

Likely an incorrect or invalid review comment.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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

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

@tensorrt-cicd
Copy link
Collaborator

PR_Github #25051 [ run ] triggered by Bot. Commit: 1481e89

@tensorrt-cicd
Copy link
Collaborator

PR_Github #25051 [ run ] completed with state FAILURE. Commit: 1481e89
/LLM/main/L0_MergeRequest_PR pipeline #18932 (Partly Tested) completed with status: 'FAILURE'

@EmmaQiaoCh
Copy link
Collaborator Author

/bot run --stage-list "GB10-Tensorrt-Post-Merge-1"

@tensorrt-cicd
Copy link
Collaborator

PR_Github #25121 [ run ] triggered by Bot. Commit: 1481e89

@tensorrt-cicd
Copy link
Collaborator

PR_Github #25121 [ run ] completed with state FAILURE. Commit: 1481e89
/LLM/main/L0_MergeRequest_PR pipeline #18990 (Partly Tested) completed with status: 'FAILURE'

@EmmaQiaoCh
Copy link
Collaborator Author

/bot run --stage-list "GB10-Tensorrt-Post-Merge-1"

@tensorrt-cicd
Copy link
Collaborator

PR_Github #25178 [ run ] triggered by Bot. Commit: dc89ec7

@tensorrt-cicd
Copy link
Collaborator

PR_Github #25178 [ run ] completed with state SUCCESS. Commit: dc89ec7
/LLM/main/L0_MergeRequest_PR pipeline #19037 (Partly Tested) completed with status: 'FAILURE'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants