test: add unit tests for ClickHouse QueryActivity and SystemHealth tools

[kespineira]

Fixes #995

Describe the changes you have made in this PR -

Added comprehensive unit tests for both ClickHouse function-based tools (get_clickhouse_query_activity, get_clickhouse_system_health) and extended integration tests for ClickHouse helper functions.

Files changed:

• tests/tools/test_clickhouse_query_activity_tool.py — new file (7 tests)
• tests/tools/test_clickhouse_system_health_tool.py — new file (10 tests)
• tests/integrations/test_clickhouse.py — extended with 23 new tests

Test coverage includes:

• Contract tests (tool metadata, schema, surface availability)
• is_available tests — verifies clickhouse_is_available returns correct boolean based on config presence
• extract_params tests — verifies parameter extraction and validation with missing/invalid fields
• run tests — happy path with mocked integration helpers returning sample data, and error paths with mocked exceptions
• Tool-specific branching — get_clickhouse_system_health returns table_stats when query succeeds

Integration helper tests (23 new):

• clickhouse_is_available — configured vs unconfigured, missing fields
• clickhouse_extract_params — valid params, missing host, missing password
• get_query_activity — happy path, error propagation, unconfigured
• get_system_health — happy path, error propagation, unconfigured
• get_table_stats — happy path, error propagation, unconfigured

Total: 63 ClickHouse tests (30 tool tests + 33 integration tests)

Demo/Screenshot for feature changes and bug fixes -

Tests pass locally:

$ make test-cov
...
63 passed

$ make lint
All checks passed.

$ make typecheck
Success: no issues found in 391 source files.

---

Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

• Yes, I used AI assistance (continue below)

If you used AI assistance:

• I have reviewed every single line of the AI-generated code
• I can explain the purpose and logic of each function/component I added
• I have tested edge cases and understand how the code handles them
• I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:

The ClickHouse tools use the lightweight @tool decorator pattern (function-based), not the class-based BaseTool pattern. This means:

1. Tool metadata is stored on the __opensre_registered_tool__ attribute set by the @tool decorator, not on class attributes like name/description.

2. is_available for both tools delegates to clickhouse_is_available from app/integrations/clickhouse.py, which checks if a ClickHouse config entry exists in the agent state. I mock this at the tool module level where it's imported.

3. extract_params delegates to clickhouse_extract_params, which validates that required fields (host, password) are present. I test both valid and invalid parameter scenarios.

4. run calls integration helpers (get_query_activity / get_system_health) internally. I mock these at the tool module import path (e.g., app.tools.ClickHouseQueryActivityTool.get_query_activity), NOT at the source module app.integrations.clickhouse, because the tools import them directly.

5. Key gotcha: ClickHouseConfig is a dataclass with instance fields, not class attributes. So patch.object(ClickHouseConfig, "max_results", ...) won't work — instead I pass max_results as a parameter or mock the config creation.

I considered using class-based BaseTool tests as the pattern, but the function-based pattern from test_postgresql_server_status_tool.py was a better match since both ClickHouse tools use @tool decorators.

---

Checklist before requesting a review

• I have added proper PR title and linked to the issue
• I have performed a self-review of my code
• I can explain the purpose of every function, class, and logic block I added
• I understand why my changes work and have tested them thoroughly
• I have considered potential edge cases and how my code handles them
• If it is a core feature, I have added thorough tests
• My code follows the project's style guidelines and conventions

---

Note: Please check Allow edits from maintainers if you would like us to assist in the PR.

[greptile-apps]

Greptile Summary

Adds 40 new unit tests across three files covering both ClickHouse tool wrappers (get_clickhouse_query_activity, get_clickhouse_system_health) and their underlying integration helpers (get_query_activity, get_system_health, get_table_stats, clickhouse_is_available, clickhouse_extract_params). The mocking strategy, assertion logic, and BaseToolContract usage all align correctly with the implementation.

Confidence Score: 5/5

Safe to merge — tests are correct and comprehensive with no logic errors.

All findings are P2 (minor style/clarity issue around a dead mock attribute). No incorrect assertions, missing error path coverage, or mismatched implementation/test logic was found.

No files require special attention.

Important Files Changed

Filename	Overview
tests/tools/test_clickhouse_query_activity_tool.py	New file — 7 tests covering tool contract (BaseToolContract mixin), metadata, is_available, extract_params, run happy/error/custom-limit paths; all match the implementation correctly.
tests/tools/test_clickhouse_system_health_tool.py	New file — 10 tests covering contract, metadata, availability, param extraction, table-stats toggle, and error paths; mocking strategy is correct for both get_system_health and get_table_stats.
tests/integrations/test_clickhouse.py	Extended with 4 new test classes (23 tests) for clickhouse_is_available, clickhouse_extract_params, get_query_activity, get_system_health, and get_table_stats; covers happy, error, edge-case and default-value paths.

Sequence Diagram

sequenceDiagram
    participant Test
    participant Tool as get_clickhouse_query_activity /<br/>get_clickhouse_system_health
    participant Integration as app.integrations.clickhouse
    participant Client as ClickHouse Client

    Test->>Tool: call(host, port, database, ...)
    Tool->>Integration: ClickHouseConfig(...)
    Tool->>Integration: get_query_activity(config, limit) / get_system_health(config)
    Integration->>Integration: config.is_configured?
    alt not configured
        Integration-->>Tool: {available: false, error: Not configured.}
    else configured
        Integration->>Client: _get_client(config)
        Integration->>Client: client.query(SQL)
        alt success
            Client-->>Integration: QueryResult
            Integration->>Integration: parse rows / truncate queries
            Integration-->>Tool: {available: true, queries/metrics/...}
        else exception
            Client-->>Integration: raise Exception
        end
        Integration->>Client: client.close() [finally]
        Integration-->>Tool: {available: false, error: str(err)}
    end
    Tool-->>Test: result dict

Loading

_{Reviews (1): Last reviewed commit: "test: add unit tests for ClickHouse Quer..." | Re-trigger Greptile}