Skip to content

[None][fix] Remove leftover onboardBlocks param in kvCacheManagerTest#13107

Merged
eopXD merged 1 commit intoNVIDIA:mainfrom
eopXD:fix/remove-onboard-blocks-test-leftover
Apr 16, 2026
Merged

[None][fix] Remove leftover onboardBlocks param in kvCacheManagerTest#13107
eopXD merged 1 commit intoNVIDIA:mainfrom
eopXD:fix/remove-onboard-blocks-test-leftover

Conversation

@eopXD
Copy link
Copy Markdown
Collaborator

@eopXD eopXD commented Apr 16, 2026

Description

Commit ec34644 ("[None][chore] Remove onboard block switch for KV cache manager (#12449)") removed the onboardBlocks parameter from the KVCacheManager constructor but missed the StoreBlocksForReuseWithPinDoesNotCreateGhostFreeBlocks test case in kvCacheManagerTest.cpp, causing a compilation error.

This PR removes the leftover onboardBlocks variable and its usage from the constructor call.

Test Coverage

  • The fix is removing a stale argument from an existing test. No new tests needed.
  • The affected test (StoreBlocksForReuseWithPinDoesNotCreateGhostFreeBlocks) will now compile and run correctly.

PR Checklist

GitHub Bot Help

Command Description
/bot run Run default CI pipeline
/bot help List all bot commands

Summary by CodeRabbit

  • Tests
    • Simplified a regression test for the KV cache manager by removing an unused constructor parameter, streamlining test configuration while maintaining test coverage.

Commit ec34644 removed the onboardBlocks parameter from the
KVCacheManager constructor but missed the
StoreBlocksForReuseWithPinDoesNotCreateGhostFreeBlocks test case,
causing a compilation error.

Signed-off-by: Yueh-Ting Chen <yueh.ting.chen@gmail.com>
@eopXD eopXD requested a review from a team as a code owner April 16, 2026 06:08
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bae36f06-b586-4748-b27d-58aa63e75b5a

📥 Commits

Reviewing files that changed from the base of the PR and between b01ff5e and ff56626.

📒 Files selected for processing (1)
  • cpp/tests/unit_tests/batch_manager/kvCacheManagerTest.cpp

📝 Walkthrough

Walkthrough

The regression test StoreBlocksForReuseWithPinDoesNotCreateGhostFreeBlock removes an unused onboardBlocks parameter from the KVCacheManager constructor call. The local constant definition is deleted and the parameter is omitted, while all other test logic remains intact.

Changes

Cohort / File(s) Summary
Test Parameter Cleanup
cpp/tests/unit_tests/batch_manager/kvCacheManagerTest.cpp
Removed onboardBlocks parameter from KVCacheManager constructor call in regression test; deleted associated local constant definition.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: removing a leftover parameter from a test case after a previous refactor.
Description check ✅ Passed The description follows the template structure with sections for Description, Test Coverage, and PR Checklist. It clearly explains the issue, references the prior commit that caused it, and describes the fix.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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.

@eopXD
Copy link
Copy Markdown
Collaborator Author

eopXD commented Apr 16, 2026

/bot run --disable-fail-fast

@eopXD
Copy link
Copy Markdown
Collaborator Author

eopXD commented Apr 16, 2026

/bot skip --comment "Skipping as this is a small change and compile error should be fixed as soon as possible in the main branch"

@eopXD eopXD enabled auto-merge (squash) April 16, 2026 06:20
@eopXD
Copy link
Copy Markdown
Collaborator Author

eopXD commented Apr 16, 2026

/bot skip --comment "Skipping as this is a small change and compile error should be fixed as soon as possible in the main branch"

2 similar comments
@eopXD
Copy link
Copy Markdown
Collaborator Author

eopXD commented Apr 16, 2026

/bot skip --comment "Skipping as this is a small change and compile error should be fixed as soon as possible in the main branch"

@eopXD
Copy link
Copy Markdown
Collaborator Author

eopXD commented Apr 16, 2026

/bot skip --comment "Skipping as this is a small change and compile error should be fixed as soon as possible in the main branch"

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #43676 [ skip ] triggered by Bot. Commit: ff56626 Link to invocation

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #43676 [ skip ] completed with state SUCCESS. Commit: ff56626
Skipping testing for commit ff56626

Link to invocation

@eopXD eopXD merged commit 90fe0d3 into NVIDIA:main Apr 16, 2026
8 checks passed
chienchunhung pushed a commit to chienchunhung/TensorRT-LLM that referenced this pull request Apr 16, 2026
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.

3 participants