Repository navigation
Conversation
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
There was a problem hiding this comment.
Pull request overview
Adds new Catch2 unit tests in the HIP test suite to validate expected behavior of several MemPool APIs when invoked during stream capture (relaxed mode allowed; global/thread-local modes return hipErrorStreamCaptureUnsupported).
Changes:
- Added new
*_Capturetest cases for multiple MemPool APIs (create/destroy/trim/set-get attribute/set-get access/export/import). - Registered the new tests in
catch/config/configs/unit/memory.yamlwith[alloc, capture]tags.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| projects/hip-tests/catch/unit/memory/hipMemPoolTrimTo.cc | Adds Unit_hipMemPoolTrimTo_Capture stream-capture behavior test. |
| projects/hip-tests/catch/unit/memory/hipMemPoolSetGetAttribute.cc | Adds Unit_hipMemPoolSetAttribute_Capture and Unit_hipMemPoolGetAttribute_Capture tests. |
| projects/hip-tests/catch/unit/memory/hipMemPoolSetGetAccess.cc | Adds Unit_hipMemPoolSetAccess_Capture and Unit_hipMemPoolGetAccess_Capture tests. |
| projects/hip-tests/catch/unit/memory/hipMemPoolImportPointer.cc | Adds Unit_hipMemPoolImportPointer_Capture test. |
| projects/hip-tests/catch/unit/memory/hipMemPoolImportFromShareableHandle.cc | Adds Unit_hipMemPoolImportFromShareableHandle_Capture test. |
| projects/hip-tests/catch/unit/memory/hipMemPoolExportToShareableHandle.cc | Adds Unit_hipMemPoolExportToShareableHandle_Capture test. |
| projects/hip-tests/catch/unit/memory/hipMemPoolExportPointer.cc | Adds Unit_hipMemPoolExportPointer_Capture test. |
| projects/hip-tests/catch/unit/memory/hipMemPoolDestroy.cc | Adds Unit_hipMemPoolDestroy_Capture test. |
| projects/hip-tests/catch/unit/memory/hipMemPoolCreate.cc | Adds Unit_hipMemPoolCreate_Capture test. |
| projects/hip-tests/catch/config/configs/unit/memory.yaml | Registers the new capture tests and tags them. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
55ac8c4 to
457afee
Compare
b1fd4aa to
a786bc9
Compare
Add CHECK_STREAM_CAPTURE_SUPPORTED() to 11 MemPool API functions so they return hipErrorStreamCaptureUnsupported when called during non-relaxed stream capture. Add corresponding stream capture tests for each affected API: hipMemPoolTrimTo, hipMemPoolSetAttribute, hipMemPoolGetAttribute, hipMemPoolSetAccess, hipMemPoolGetAccess, hipMemPoolCreate, hipMemPoolDestroy, hipMemPoolExportToShareableHandle, hipMemPoolImportFromShareableHandle, hipMemPoolExportPointer, hipMemPoolImportPointer.
a786bc9 to
5bf43e8
Compare
mangupta
left a comment
There was a problem hiding this comment.
🤖 AI-assisted pre-review (for @mangupta)
[IMPORTANT] Public-API behavior change has no contract test / uncovered_apis rationale and TEST_PLAN.md is not regenerated
The PR changes a public-API invariant for eleven APIs (they must now return hipErrorStreamCaptureUnsupported during non-relaxed capture) but adds coverage only under catch/unit/memory/. There is no projects/hip-tests/catch/contract/ test with an @asserts: API - invariant annotation, no entry added to projects/hip-tests/catch/contract/uncovered_apis.txt, and no update to the generated projects/hip-tests/catch/TEST_PLAN.md even though the test configuration (catch/config/configs/unit/memory.yaml) changed.
Suggested fix: Add a catch/contract/ test asserting the capture invariant for the affected mempool APIs (using the format in catch/contract/AUTHORING.md), or record a concrete rationale in catch/contract/uncovered_apis.txt, and regenerate catch/TEST_PLAN.md so it reflects the new cases and config entries.
|
🤖 AI-assisted pre-review (for @mangupta) Re-reviewed the updated head ( |
|
🤖 AI-assisted pre-review (for @mangupta) Re-reviewed the updated head ( |
|
🤖 AI-assisted pre-review (for @mangupta) Re-reviewed the updated head ( |
Motivation
add stream capture checks and tests for MemPool APIs
Technical Details
Add CHECK_STREAM_CAPTURE_SUPPORTED() to 11 MemPool API functions
so they return hipErrorStreamCaptureUnsupported when called
during non-relaxed stream capture.
Add corresponding stream capture tests for each affected API:
hipMemPoolTrimTo, hipMemPoolSetAttribute, hipMemPoolGetAttribute,
hipMemPoolSetAccess, hipMemPoolGetAccess, hipMemPoolCreate,
hipMemPoolDestroy, hipMemPoolExportToShareableHandle,
hipMemPoolImportFromShareableHandle, hipMemPoolExportPointer,
hipMemPoolImportPointer.
Issue Tracking
ROCM-22689
Test Plan
The tests in this PR
Test Result
Passed with the clr change in this PR.
Submission Checklist