mjsax commented on a change in pull request #10810:
URL: https://github.com/apache/kafka/pull/10810#discussion_r647126680



##########
File path: 
streams/test-utils/src/main/java/org/apache/kafka/streams/processor/MockProcessorContext.java
##########
@@ -319,7 +321,7 @@ public void setRecordMetadata(final String topic,
         this.topic = topic;
         this.partition = partition;
         this.offset = offset;
-        this.headers = headers;
+        this.headers = Objects.requireNonNull(headers);

Review comment:
       Thinking about is more, it might be best to keep the code as is, and 
allow `null`.
     - adding the new check might break existing test code
     - converting `null` to empty header might also break tests
     - users don't have a good way to pass in headers, as `Headers` is an 
interface (and there is no other public API they could use to create an object)
     - it was not a problem so far, and long term we want to deprecate this 
"old" API anyway (so we might want to avoid a case of pre-mature optimization)
   
   Of course, there is still this small gap between actual runtime code and the 
mock -- in the runtime, we don't ever return `null` and thus if the mock might 
return `null`, users cannot test their production code without an actually 
unnecessary null-check... but maybe that is acceptable. 




-- 
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to