Skip to content

Conversation

@chernser
Copy link
Contributor

@chernser chernser commented Nov 12, 2025

Summary

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG
  • For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

@chernser chernser changed the title implemented working env base [repo] Integration testing Nov 24, 2025
@mshustov mshustov requested a review from Copilot December 4, 2025 17:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a comprehensive integration testing framework for the Kafka ClickHouse connector. The changes refactor existing integration tests into a more maintainable structure with shared base classes and helpers, update configuration files to support both local and cloud testing environments, and add utilities for generating test datasets across multiple serialization formats.

  • Refactored integration tests into a base class structure for better code reuse
  • Added DatasetGenerator utility for creating test data in multiple formats (Avro, Protobuf, JSON)
  • Updated Confluent Platform version from 7.5.0 to 7.7.0

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
data01.yaml New test fixture defining database schema and message field mappings
DatasetGenerator.java New utility class for generating test datasets in multiple serialization formats
keeper_map_config.xml New ClickHouse configuration for integration tests
kafka_compose.yaml New Docker Compose configuration for local Kafka/ClickHouse environment
clickhouse_sink_with_jdbc_prop.json Updated to use template variables for exactlyOnce setting
clickhouse_sink_schemaless.json Updated to use template variables for username and exactlyOnce settings
clickhouse_sink_no_proxy_with_jdbc_prop.json New connector configuration without proxy settings
clickhouse_sink_no_proxy_schemaless.json Updated database and SSL settings to use defaults
clickhouse_sink_no_proxy.json Updated database and SSL settings to use defaults
clickhouse_sink.json Updated to use template variables for username and exactlyOnce settings
IntegrationTestBase.java New base class providing common setup/teardown and helper methods for integration tests
ConfluentPlatform.java Updated Confluent version to 7.7.0, added constants, improved startup configuration
ExactlyOnceTest.java Disabled test class pending restoration
ClickHouseSinkTest.java New test class using refactored base class structure
ClickHouseSinkConnectorIntegrationTest.java Removed old integration test implementation
ClickHouseCloudTest.java Removed cloud-specific test implementation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +198 to +201
record.put(field.name(), offset % 2 == 0);
case FLOAT:
case DOUBLE:
record.put(field.name(), offset * 10.123456);
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing break statements in switch cases. The BOOLEAN, FLOAT, DOUBLE, and STRING cases will fall through to subsequent cases, causing incorrect behavior where STRING value overwrites all previous field assignments.

Suggested change
record.put(field.name(), offset % 2 == 0);
case FLOAT:
case DOUBLE:
record.put(field.name(), offset * 10.123456);
record.put(field.name(), offset % 2 == 0);
break;
case FLOAT:
record.put(field.name(), offset * 10.123456);
break;
case DOUBLE:
record.put(field.name(), offset * 10.123456);
break;

Copilot uses AI. Check for mistakes.
protected int generateSchemalessData(String topicName, int numberOfPartitions, int numberOfRecords) throws Exception {
Map<String, Object> config = new HashMap<>(DATA_GEN_BASE_CONFIG);
config.put("value.converter", ConfluentPlatform.KAFKA_JSON_CONVERTER);
DATA_GEN_BASE_CONFIG.put("value.converter.schemas.enable", "false");
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line modifies the shared static DATA_GEN_BASE_CONFIG map, which will affect all subsequent test executions. This should modify the local config map instead to avoid test interference.

Suggested change
DATA_GEN_BASE_CONFIG.put("value.converter.schemas.enable", "false");
config.put("value.converter.schemas.enable", "false");

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants