Feat improve read tests #16
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the unit test coverage for serial port read operations and refactors the maximum safe read size from a compile-time constant to a runtime-configurable parameter.
Key changes:
- Added tests for error handling in
readBytes,readUntil, and buffer overflow scenarios - Converted
kMaxSafeReadSizefrom a static constant to a configurable member variablemax_safe_read_size_ - Added getter/setter methods (
setMaxSafeReadSize/getMaxSafeReadSize) for the maximum safe read size
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/test_serial_pty.cpp | Added three new test cases: ReadBytesWithReadFail, ReadUntilWithNullBuffer, and ReadUntilWithOverflowBuffer to improve test coverage |
| src/serial.cpp | Replaced static constant kMaxSafeReadSize with configurable max_safe_read_size_ member variable and updated all references; also unified read function calls to use read_ |
| include/libserial/serial.hpp | Added public getter/setter methods for max safe read size, improved test helper documentation, and converted constant to member variable |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
include/libserial/serial.hpp
Outdated
| * in a single read operation to prevent excessive memory usage. | ||
| * | ||
| * @param size The desired maximum safe read size in bytes | ||
| * @throws SerialException if size cannot be set |
include/libserial/serial.hpp
Outdated
| * | ||
| * Defines the maximum number of bytes that can be read | ||
| * in a single read operation to prevent excessive memory usage. | ||
| * default is 2048 bytes (2KB). |
There was a problem hiding this comment.
Corrected capitalization of 'default' to 'Default'.
Suggested change
| * default is 2048 bytes (2KB). | |
| * Default is 2048 bytes (2KB). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Details
How to test
Screenshots/Logs (optional)
Checklist
make cppcheck/make cpplint/make uncrustifyExample(s) tested (if touched)Risks and rollbacks