[protocol] Add unittest for libhoth_payload_update_read_chunk#241
[protocol] Add unittest for libhoth_payload_update_read_chunk#241esnguyen wants to merge 1 commit intogoogle:mainfrom
Conversation
This function is being used within libhoth host command and needs a check. Signed-off-by: Ellis Nguyen <sarzanguyen@google.com>
esnguyen
force-pushed
the
esnguyen/add_unittest_payload_chunk_read
branch
from
ebffcbe to
7978751
Compare
| } | ||
|
|
||
| MATCHER_P2(IsReadRequest, offset, len, "") { | ||
| const uint8_t* data = static_cast<const uint8_t*>(arg); |
There was a problem hiding this comment.
would it be a good idea to check that (struct hoth_host_request *)arg has appropriate data as well? like struct version and command? Without those checks, this matcher might also match other host commands which have appropriate values in the data.
| EXPECT_CALL(mock_, receive) | ||
| .WillOnce(DoAll(CopyResp(expected_data, len), Return(LIBHOTH_OK))); | ||
|
|
||
| char temp_path[] = "/tmp/libhoth_payload_read_XXXXXX"; |
There was a problem hiding this comment.
minor nit: Would it be a good idea to use testing::TempDir? Although it might just return /tmp as well.
| EXPECT_EQ(libhoth_payload_update_read_chunk(&hoth_dev_, fd, len, offset), | ||
| PAYLOAD_UPDATE_OK); | ||
|
|
||
| lseek(fd, 0, SEEK_SET); |
There was a problem hiding this comment.
nit. Would it be a good idea to check the return value here as well?
| ASSERT_EQ(read(fd, actual_data, len), static_cast<ssize_t>(len)); | ||
| EXPECT_THAT(actual_data, ::testing::ElementsAreArray(expected_data)); | ||
|
|
||
| close(fd); |
There was a problem hiding this comment.
nit: These might not execute if any of the asserts above fail after opening the file successfully.
There was a problem hiding this comment.
Looking at man 2 open, you might able to use O_TMPFILE (with other appropriate flags) to create an unnamed temporary file
2 participants
This function is being used within libhoth host command and needs a check.