Copilot commented on code in PR #9585:
URL: https://github.com/apache/seatunnel/pull/9585#discussion_r2218135247
##########
seatunnel-connectors-v2/connector-clickhouse/src/test/java/org/apache/seatunnel/connectors/seatunnel/clickhouse/source/ClickhouseValueReaderTest.java:
##########
@@ -283,22 +290,55 @@ public void testSqlStrategyReadWithSortingKey() {
Assertions.assertFalse(reader.hasNext());
- Mockito.verify(mockProxy, Mockito.times(3)).batchFetchRecords(any(),
any());
+ Mockito.verify(mockProxy, Mockito.times(3))
+ .batchFetchRecords(any(), eq(sourceTable.getTablePath()),
any());
}
- private void initStreamValueReaderMock() throws ClickHouseException {
- // mock ClickHouseResponse
+ @Test
+ public void testBatchFetchRecordsAndTableId() throws Exception {
+ // mock proxy query response
+ ClickhouseProxy proxy = Mockito.spy(new ClickhouseProxy(node));
+ Field requestField =
ClickhouseProxy.class.getDeclaredField("clickhouseRequest");
Review Comment:
Using reflection to access private fields makes the test fragile and tightly
coupled to implementation details. Consider adding a package-private
constructor or setter method to ClickhouseProxy for testing purposes instead of
using reflection.
##########
seatunnel-connectors-v2/connector-clickhouse/src/test/java/org/apache/seatunnel/connectors/seatunnel/clickhouse/source/ClickhouseValueReaderTest.java:
##########
@@ -283,22 +290,55 @@ public void testSqlStrategyReadWithSortingKey() {
Assertions.assertFalse(reader.hasNext());
- Mockito.verify(mockProxy, Mockito.times(3)).batchFetchRecords(any(),
any());
+ Mockito.verify(mockProxy, Mockito.times(3))
+ .batchFetchRecords(any(), eq(sourceTable.getTablePath()),
any());
}
- private void initStreamValueReaderMock() throws ClickHouseException {
- // mock ClickHouseResponse
+ @Test
+ public void testBatchFetchRecordsAndTableId() throws Exception {
+ // mock proxy query response
+ ClickhouseProxy proxy = Mockito.spy(new ClickhouseProxy(node));
+ Field requestField =
ClickhouseProxy.class.getDeclaredField("clickhouseRequest");
+ requestField.setAccessible(true);
Review Comment:
Setting field accessibility bypasses encapsulation and can cause issues in
environments with security managers. This approach is brittle and should be
replaced with proper dependency injection or test-friendly constructors.
--
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]
