uros-b commented on code in PR #56526:
URL: https://github.com/apache/spark/pull/56526#discussion_r3430338692
##########
connector/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaMicroBatchSourceSuite.scala:
##########
@@ -1883,6 +1883,14 @@ abstract class KafkaMicroBatchV2SourceSuite extends
KafkaMicroBatchSourceSuiteBa
// test null latestAvailablePartitionOffsets
assert(KafkaMicroBatchStream.metrics(Optional.ofNullable(offset),
None).isEmpty)
}
+
+ test("SPARK-57438: metrics should not NPE when latest partition offsets are
unavailable" +
Review Comment:
I don't think that the test truly covers the change asked about in
https://github.com/apache/spark/pull/56526/changes#r3417070560. The added test
only exercises the companion `KafkaMicroBatchStream.metrics(..., Some(null))`,
i.e. the defense-in-depth helper.
Because the helper alone now neutralizes Some(null), reverting line 360
would not fail any test, so the root-cause call-site fix is effectively
unguarded against regression. A test driving the instance metrics() with
`latestPartitionOffsets == null` would probably close this a bit better.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
