[TRTLLM-9228][infra] Verify thirdparty C++ process
Description
This change adds two tests to help ensure that thirdparty C++ code is integrated according to the process descripted in 3rdparty/cpp-thirdparty.md. The desired process requires folks to use FetchContent_Declare in 3rdparty/CMakeLists.txt. The two tests added here search the following deviations of the desired process:
- uses of FetchContent_Declare or ExternalProject_Add outside of 3rdparty/CMakeLists.txt
- new git-submodules added to the repo
Both scripts have the ability to allow-list exemptions if there are any cases in the future where a deviation is warranted. The scripts live in 3rdparty/* where CODEOWNERS ensures that process reviewers will be required on any new exemptions added to the allowlists.
Summary by CodeRabbit
- Tests
- Added comprehensive validation suite for third-party dependency management
- Includes automated checks for CMake configuration patterns and git submodule organization
- Ensures adherence to project dependency policies
โ๏ธ Tip: You can customize this high-level summary in your review settings.
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.
-
[x] 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.
๐ Walkthrough
Walkthrough
Two new test modules are introduced to enforce third-party dependency management policies: one scans CMake files for unauthorized third-party integrations outside the designated 3rdparty directory, and another validates git submodule declarations. Both tests are integrated into the repository's test suite.
Changes
| Cohort / File(s) | Summary |
|---|---|
Third-party Policy Enforcement Tests 3rdparty/test_cmake_third_party.py, 3rdparty/test_git_modules.py |
Added two new test modules enforcing third-party dependency management. The CMake test scans for FetchContent_Declare and ExternalProject_Add directives outside 3rdparty/; the Git submodules test validates .gitmodules entries against an allowlist and enforces 3rdparty/ path prefix. Both include CLI entry points and pytest test cases. |
Test Suite Integration tests/integration/test_lists/test-db/l0_a10.yml |
Added entries for the two new third-party policy tests to the PyTorch test group list. |
Estimated code review effort
๐ฏ 3 (Moderate) | โฑ๏ธ ~20 minutes
- Filter and ignore logic in
test_cmake_third_party.py: Verify the DirectoryFilter and FileFilter implementations correctly exclude intended paths and select relevant CMake files - Violation detection patterns: Ensure FetchContent_Declare and ExternalProject_Add pattern matching is robust and captures intended violations
- Config parsing in
test_git_modules.py: Review configparser usage and path validation logic for correctness - Error handling and exit codes: Confirm both scripts properly aggregate violations, report to stderr, and return correct exit statuses
- Test integration: Validate the test list entries are properly formatted and discoverable by the test harness
Pre-merge checks and finishing touches
โ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | โ ๏ธ Warning | Docstring coverage is 46.15% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
โ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | โ Passed | The title clearly and specifically describes the main change: adding tests to verify the thirdparty C++ integration process. |
| Description check | โ Passed | The description explains the purpose, the two test types, and how exemptions are managed through allowlists in CODEOWNERS, though Test Coverage section is empty. |
โจ Finishing touches
- [ ] ๐ Generate docstrings
๐งช Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
/bot run
PR_Github #25598 [ run ] triggered by Bot. Commit: b9d7489
PR_Github #25598 [ run ] completed with state FAILURE. Commit: b9d7489
/LLM/main/L0_MergeRequest_PR pipeline #19392 completed with status: 'FAILURE'
/bot run
PR_Github #26462 [ run ] triggered by Bot. Commit: 775583f
PR_Github #26462 [ run ] completed with state FAILURE. Commit: 775583f
/LLM/main/L0_MergeRequest_PR pipeline #20112 completed with status: 'FAILURE'
/bot run
PR_Github #27021 [ run ] triggered by Bot. Commit: 5264baf
PR_Github #27021 [ run ] completed with state FAILURE. Commit: 5264baf
/LLM/main/L0_MergeRequest_PR pipeline #20602 completed with status: 'FAILURE'
/bot run
/bot run
PR_Github #27161 [ run ] triggered by Bot. Commit: dd05f5b
PR_Github #27161 [ run ] completed with state SUCCESS. Commit: dd05f5b
/LLM/main/L0_MergeRequest_PR pipeline #20725 completed with status: 'FAILURE'
/bot run --disable-fail-fast
PR_Github #27359 [ run ] triggered by Bot. Commit: cc578e2
PR_Github #27359 [ run ] completed with state SUCCESS. Commit: cc578e2
/LLM/main/L0_MergeRequest_PR pipeline #20902 completed with status: 'SUCCESS'
Pipeline passed with automatic retried tests. Check the rerun report for details.