slbotbm commented on PR #3046:
URL: https://github.com/apache/iggy/pull/3046#issuecomment-4150411892
@seokjin0414 Thanks for the quick turn-around.
I may not have been clear in my review previously, but I wanted you to
implement something more like
```rust
impl ffi::Message {
pub fn new_message(&mut self, payload: Vec<u8>) -> Result<(), String> {
if payload.is_empty() {
return Err("Could not create message: payload must not be
empty".to_string());
}
let payload_length = payload.len() as u32;
*self = Self {
checksum: 0,
id_lo: 0,
id_hi: 0,
offset: 0,
timestamp: 0,
origin_timestamp: 0,
user_headers_length: 0,
payload_length,
reserved: 0,
payload,
user_headers: Vec::new(),
};
Ok(())
}
}
```
The above interface would allow us to call `Message.new_message(...)` from
the cpp side while avoiding ambiguity.
---
`GetStreamsReturnsEmptyAfterCleanup` and
`GetStreamsReturnsStreamAfterCreation` test the `get_streams` function, but
they are in the `messages/` folder. Please move them to their appropriate
fields.
---
The amount of testing is insufficient for the three functions.
Please think about the following and add tests:
- `get_streams`:
- For `GetStreamsReturnsStreamAfterCreation`, verify everything that is
verifiable, not just the name. Also create additional streams and topics inside
them, and verify everything.
- Does the sdk allow calling `get_streams` before `login` and `connect`?
- Does calling `get_stream` and `get_streams` when there is only one one
stream return the same same with the same details?
- What happens when you call `get_streams` repeatedly?
- Lastly, in `GetStreamsReturnsEmptyAfterCleanup`, you are deleting
every stream without ever creating any. Instead of this, let's create some
unique streams, delete those, and then verify whether 0 is returned or not.
- `send_messages`:
- What happens when you call send_messages before login and connect?
- What happens when you give invalid topic and / or stream identifiers
to the function?
- What happens when you give wrong topic and / or stream identifiers to
the function?
- What happens when you give wrong partitioning kind and / or value to
the function? (also test the relevant limits)
- Verify that the messages are being stored in partitions according to
the strategy we have strategy we have chosen.
- What happens when you supply an empty vector to the `send_messages`?
- Is there a limit on how many messages can be sent in total, or in a
single batch?
- What happens when you try to send an invalid payload through
`send_messages` (empty payload, payload size > 64,000,000 bytes)
- When `send_messages` is called multiple times, is the order of the
messages preserved?
- You have written a test where one custom ID is set and sent to the
server. What happens when you set the same ID for multiple messages?
- Lastly, since we are working with bytes, I would like to you to try
sending a variety of payloads, and check whether they succeed or not. (null
bytes, utf-8 strings, emojis, etc. - consider using property based testing)
- `poll_messages` :
- What happens when you call poll_messages before login and connect?
- What happens when you give invalid topic, stream, and / or consumer
identifiers to the function?
- What happens when you give wrong topic, stream, and / or consumer
identifiers to the function?
- What happens when you give a wrong consumer_kind to the function?
- What happens when you give a wrong polling strategy kind and / or
value to the function? (also test the relevant limits)
- What happens when you give an invalid or non-existent partition ID to
the function?
- What happens when you supply count = 0 to the poll_messages?
- Is there a limit on how many messages can be requested in a single
poll?
- When the requested count is smaller than the number of available
messages, are only that many messages returned?
- When the requested count is larger than the number of available
messages, are only the available messages returned?
- Verify that polling without specifying a partition behaves as expected.
- Verify the behavior of each polling strategy: offset, first, last,
next, and timestamp.
- For offset, what happens when the offset is valid, too large, or
points exactly to the end?
- For timestamp, what happens when the timestamp is before the first
message, between messages, and after the last message?
- What happens when auto_commit = false and poll_messages is called
repeatedly with next?
- What happens when auto_commit = true and poll_messages is called
repeatedly with next?
- Are offsets tracked independently for different consumer IDs?
- Are offsets tracked independently for consumer and consumer_group?
- When multiple consumers in the same consumer group poll, is the
behavior what we expect?
- When poll_messages is called multiple times after multiple
send_messages calls, is the order of the returned messages preserved?
- Verify that the returned offsets are monotonic and correct across
multiple polls.
- Verify that the returned payloads are preserved exactly for a variety
of byte payloads. (null bytes, utf-8 strings, emojis, arbitrary bytes, etc. -
consider doing property-based testing)
- Verify that custom message IDs, if present, are returned correctly for
multiple messages, not just one.
- What happens when you poll from an empty partition, and then poll
again after new messages are sent?
- What happens when you poll after the stream or topic has been deleted?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]