Skip to content

Commit 3184056

Browse files
committed
Fix ClickHouse loader tests and add Makefile target
- Add `make test-clickhouse` target - Fix testcontainer readiness check (remove broken wait_for_logs, rely on built-in HTTP health check) - Import container classes independently so missing driver packages don't block other containers - Fix ClickHouse test config credentials to match container defaults - Remove calls to nonexistent _record_connection_opened/_closed methods - Set supports_overwrite=False since ClickHouse only supports APPEND mode - Fix base test_batch_loading to respect supports_overwrite config
1 parent e178196 commit 3184056

File tree

5 files changed

+36
-34
lines changed

5 files changed

+36
-34
lines changed

Makefile

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ test-snowflake:
4040
@echo "❄️ Running Snowflake tests..."
4141
$(PYTHON) pytest tests/ -m "snowflake" -v --log-cli-level=ERROR
4242

43+
test-clickhouse:
44+
@echo "🏠 Running ClickHouse tests..."
45+
$(PYTHON) pytest tests/ -m "clickhouse" -v --log-cli-level=ERROR
46+
4347
test-lmdb:
4448
@echo "⚡ Running LMDB tests..."
4549
$(PYTHON) pytest tests/ -m "lmdb" -v --log-cli-level=ERROR
@@ -131,6 +135,7 @@ help:
131135
@echo " make test-all - Run all tests with coverage"
132136
@echo " make test-postgresql - Run PostgreSQL tests"
133137
@echo " make test-redis - Run Redis tests"
138+
@echo " make test-clickhouse - Run ClickHouse tests"
134139
@echo " make test-snowflake - Run Snowflake tests"
135140
@echo " make test-performance - Run performance tests"
136141
@echo " make lint - Lint code with ruff"

src/amp/loaders/implementations/clickhouse_loader.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ def connect(self) -> None:
114114
self.logger.info(f'Found {len(tables.result_rows)} tables in database')
115115

116116
self._is_connected = True
117-
self._record_connection_opened()
118117

119118
except ImportError as err:
120119
raise ImportError(
@@ -126,13 +125,10 @@ def connect(self) -> None:
126125

127126
def disconnect(self) -> None:
128127
"""Close ClickHouse connection"""
129-
was_connected = self._is_connected
130128
if self._client:
131129
self._client.close()
132130
self._client = None
133131
self._is_connected = False
134-
if was_connected:
135-
self._record_connection_closed()
136132
self.logger.info('Disconnected from ClickHouse')
137133

138134
def _load_batch_impl(self, batch: pa.RecordBatch, table_name: str, **kwargs) -> int:

tests/conftest.py

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,30 @@
2929
if colima_socket.exists():
3030
os.environ['DOCKER_HOST'] = f'unix://{colima_socket}'
3131

32-
# Import testcontainers conditionally
32+
# Import testcontainers conditionally (each container imported independently
33+
# so missing driver packages don't block other containers)
34+
TESTCONTAINERS_AVAILABLE = False
35+
PostgresContainer = None
36+
RedisContainer = None
37+
ClickHouseContainer = None
38+
3339
if USE_TESTCONTAINERS:
3440
try:
3541
from testcontainers.postgres import PostgresContainer
42+
except ImportError:
43+
pass
44+
try:
3645
from testcontainers.redis import RedisContainer
46+
except ImportError:
47+
pass
48+
try:
3749
from testcontainers.clickhouse import ClickHouseContainer
38-
39-
TESTCONTAINERS_AVAILABLE = True
4050
except ImportError:
41-
TESTCONTAINERS_AVAILABLE = False
51+
pass
52+
53+
TESTCONTAINERS_AVAILABLE = any(c is not None for c in [PostgresContainer, RedisContainer, ClickHouseContainer])
54+
if not TESTCONTAINERS_AVAILABLE:
4255
logging.warning('Testcontainers not available. Falling back to manual configuration.')
43-
else:
44-
TESTCONTAINERS_AVAILABLE = False
4556

4657

4758
# Shared configuration fixtures
@@ -137,8 +148,8 @@ def test_config():
137148
@pytest.fixture(scope='session')
138149
def postgres_container():
139150
"""PostgreSQL container for integration tests"""
140-
if not TESTCONTAINERS_AVAILABLE:
141-
pytest.skip('Testcontainers not available')
151+
if PostgresContainer is None:
152+
pytest.skip('Testcontainers for PostgreSQL not available')
142153

143154
import time
144155

@@ -161,8 +172,8 @@ def postgres_container():
161172
@pytest.fixture(scope='session')
162173
def redis_container():
163174
"""Redis container for integration tests"""
164-
if not TESTCONTAINERS_AVAILABLE:
165-
pytest.skip('Testcontainers not available')
175+
if RedisContainer is None:
176+
pytest.skip('Testcontainers for Redis not available')
166177

167178
from testcontainers.core.waiting_utils import wait_for_logs
168179

@@ -180,22 +191,12 @@ def redis_container():
180191
@pytest.fixture(scope='session')
181192
def clickhouse_container():
182193
"""ClickHouse container for integration tests"""
183-
if not TESTCONTAINERS_AVAILABLE:
184-
pytest.skip('Testcontainers not available')
185-
186-
import time
187-
188-
from testcontainers.core.waiting_utils import wait_for_logs
194+
if ClickHouseContainer is None:
195+
pytest.skip('Testcontainers for ClickHouse not available')
189196

190197
container = ClickHouseContainer(image='clickhouse/clickhouse-server:24-alpine')
191198
container.start()
192199

193-
# Wait for ClickHouse to be ready
194-
wait_for_logs(container, 'Ready for connections', timeout=60)
195-
196-
# Additional wait for HTTP interface to be ready
197-
time.sleep(2)
198-
199200
yield container
200201

201202
container.stop()
@@ -204,7 +205,7 @@ def clickhouse_container():
204205
@pytest.fixture(scope='session')
205206
def postgresql_test_config(request):
206207
"""PostgreSQL configuration from testcontainer or environment"""
207-
if TESTCONTAINERS_AVAILABLE and USE_TESTCONTAINERS:
208+
if PostgresContainer is not None:
208209
# Get the postgres_container fixture
209210
postgres_container = request.getfixturevalue('postgres_container')
210211
return {
@@ -224,7 +225,7 @@ def postgresql_test_config(request):
224225
@pytest.fixture(scope='session')
225226
def redis_test_config(request):
226227
"""Redis configuration from testcontainer or environment"""
227-
if TESTCONTAINERS_AVAILABLE and USE_TESTCONTAINERS:
228+
if RedisContainer is not None:
228229
# Get the redis_container fixture
229230
redis_container = request.getfixturevalue('redis_container')
230231
return {
@@ -244,15 +245,15 @@ def redis_test_config(request):
244245
@pytest.fixture(scope='session')
245246
def clickhouse_test_config(request):
246247
"""ClickHouse configuration from testcontainer or environment"""
247-
if TESTCONTAINERS_AVAILABLE and USE_TESTCONTAINERS:
248+
if ClickHouseContainer is not None:
248249
# Get the clickhouse_container fixture
249250
clickhouse_container = request.getfixturevalue('clickhouse_container')
250251
return {
251252
'host': clickhouse_container.get_container_host_ip(),
252253
'port': clickhouse_container.get_exposed_port(8123),
253-
'database': 'default',
254-
'username': 'default',
255-
'password': '',
254+
'database': clickhouse_container.dbname,
255+
'username': clickhouse_container.username,
256+
'password': clickhouse_container.password,
256257
'batch_size': 100000,
257258
}
258259
else:

tests/integration/loaders/backends/test_clickhouse.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class ClickHouseTestConfig(LoaderTestConfig):
2525
loader_class = ClickHouseLoader
2626
config_fixture_name = 'clickhouse_test_config'
2727

28-
supports_overwrite = True
28+
supports_overwrite = False
2929
supports_streaming = True
3030
supports_multi_network = True
3131
supports_null_values = True

tests/integration/loaders/test_base_loader.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ def test_batch_loading(self, loader, medium_test_table, test_table_name, cleanup
5757
batches = medium_test_table.to_batches(max_chunksize=250)
5858

5959
for i, batch in enumerate(batches):
60-
mode = LoadMode.OVERWRITE if i == 0 else LoadMode.APPEND
60+
mode = LoadMode.OVERWRITE if i == 0 and self.config.supports_overwrite else LoadMode.APPEND
6161
result = loader.load_batch(batch, test_table_name, mode=mode)
6262

6363
assert result.success == True

0 commit comments

Comments
 (0)