testing: Add test scenarios for OBJ plugin gtest #666
Conversation
|
👋 Hi mamar123! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
|
/ok to test e4f4acf |
|
/build |
Signed-off-by: Menachem <[email protected]>
mamar123
force-pushed
the
obj_gtest_enhance_pr
branch
from
e4f4acf to
35bd079
Compare
|
/ok to test 35bd079 |
| localBackendEngine_, localBackendEngine_, local_agent_name, local_agent_name, mem_cfg); | ||
| transfer.setupMems(); | ||
|
|
||
| nixl_xfer_op_t op = NIXL_WRITE; |
There was a problem hiding this comment.
Not sure why a stack variable assist here.
| } | ||
|
|
||
| TEST_P(setupObjTestFixture, writeWithOffsetsTest) { | ||
| transferMemConfig mem_cfg{.numEntries_ = 2}; |
There was a problem hiding this comment.
Is 2 a must? if so, please use a named constant, otherwise a random value.
| localBackendEngine_, localBackendEngine_, local_agent_name, local_agent_name); | ||
|
|
||
| nixlMetaDesc meta_desc; | ||
| meta_desc.devId = 1; |
There was a problem hiding this comment.
Is 1 a must? if so please use named constant, otherwise a random value
| uint8_t src_byte = getRandomInt(0, 255); | ||
|
|
||
| // First, write entire buffer to OBJ in a single chunk (no offsets) | ||
| transferMemConfig mem_cfg_write{.numEntries_ = 1, .entrySize_ = 128, .srcBufByte_ = src_byte}; |
There was a problem hiding this comment.
Same note regarding 1, 128
| } | ||
|
|
||
| TEST_P(setupObjTestFixture, readWithOffsetsTest) { | ||
| uint8_t src_byte = getRandomInt(0, 255); |
There was a problem hiding this comment.
To have less hard coded numbers, I would suggest:
uint8_t src_byte = getRandomInt(0, std::numeric_limits<uint8_t>::max());
| TEST_P(setupObjTestFixture, dramAsDstMemTest) { | ||
| transferHandler<DRAM_SEG, DRAM_SEG> transfer( | ||
| localBackendEngine_, localBackendEngine_, local_agent_name, local_agent_name); | ||
| ASSERT_EQ(transfer.prepareTransfer(NIXL_WRITE), NIXL_ERR_INVALID_PARAM); | ||
| } |
There was a problem hiding this comment.
(All around)
When you have a single place to use a class you can do that creating a temporary instance with shorter life span (gone after function call), i.e.:
TEST_P(setupObjTestFixture, dramAsDstMemTest) {
ASSERT_EQ(transferHandler<DRAM_SEG, DRAM_SEG>(localBackendEngine_, localBackendEngine_, local_agent_name, local_agent_name).prepareTransfer(NIXL_WRITE), NIXL_ERR_INVALID_PARAM);
}
4 participants
No description provided.