I was not aware of `InternalFixedKeyRecordFactory`. As the name
indicates, it's considered an internal class, so not sure if we should
recommend to use it in test...
I understand why this class is required, and why it was put into a
public package; the way Java works, enforces this. Not sure
Looking at the ticket and the sample code, I think it would be possible to
continue using `InternalFixedKeyRecordFactory` as the avenue to create
`FixedKeyRecord`s in tests. As long as there was a
MockFixedKeyProcessorContext, I think we would be able to test
FixedKeyProcessors without a Topology.
Had a discussion on https://issues.apache.org/jira/browse/KAFKA-15242
and it was pointed out, that we also need to do something about
`FixedKeyRecord`. It does not have a public constructor (what is
correct; it should not have one). However, this makes testing
`FixedKeyProcessor` impossible
Please also update the KIP.
To get a wiki account created, please request it via a commet on this
ticket: https://issues.apache.org/jira/browse/INFRA-25451
After you have the account, please share your wiki id, and we can give
you write permission on the wiki.
-Matthias
On 5/3/24 6:30
Hi Matthias,
Sorry this fell out of my radar for a bit.
Revisiting the topic, I think you’re right and we accept the duplicated
nesting as an appropriate solution to not affect the larger public API.
I can update my PR with the change.
Regards,
Shashwat Pandey
On Wed, May 1, 2024 at 11:00 PM
Any updates on this KIP?
On 3/28/24 4:11 AM, Matthias J. Sax wrote:
It seems that `MockRecordMetadata` is a private class, and thus not part
of the public API. If there are any changes required, we don't need to
discuss on the KIP.
For `CapturedPunctuator` and `CapturedForward` it's a
It seems that `MockRecordMetadata` is a private class, and thus not part
of the public API. If there are any changes required, we don't need to
discuss on the KIP.
For `CapturedPunctuator` and `CapturedForward` it's a little bit more
tricky. My gut feeling is, that the classes might not need
Thanks for the feedback Matthias!
The reason I proposed the extension of MockProcessorContext was more to do
with the internals of the class (MockRecordMetadata, CapturedPunctuator and
CapturedForward).
However, I do see your point, I would then think to split
MockProcessorContext and
Thanks for the KIP Shashwat. Closing this testing gap is great! It did
come up a few time already...
One question: why do you propose to `extend MockProcessorContext`?
Given how the actual runtime context classes are setup, it seems that
the regular context and fixed-key-context are distinct,
