test(tools): add unit tests for ClickHouseQueryActivity and ClickHous…#1025
Conversation
…eSystemHealth tools Covers contract, is_available, extract_params, run happy path, and run error path for both tools. No live ClickHouse instance required. Closes Tracer-Cloud#995
Greptile SummaryAdds unit tests for Confidence Score: 5/5Safe to merge — all findings are P2 style suggestions that do not affect test correctness or production behaviour. Both test files are structurally correct, follow the established project pattern, patch at the right module level, and provide solid coverage of the branching logic. The two P2 findings (missing assert_not_called in the include_table_stats=False case, and a missing edge-case test for table-stats failure) are quality improvements, not correctness defects, so they do not reduce the score below 5. tests/tools/test_clickhouse_system_health_tool.py — the include_table_stats=False happy-path test could be hardened with a get_table_stats assert_not_called check. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Test Suite] --> B[TestClickHouseQueryActivityToolContract\nBaseToolContract mixin]
A --> C[TestClickHouseSystemHealthToolContract\nBaseToolContract mixin]
A --> D[Standalone test functions\nQueryActivity: 7 tests]
A --> E[Standalone test functions\nSystemHealth: 10 tests]
B --> B1[test_metadata_has_valid_name]
B --> B2[test_metadata_has_valid_description]
B --> B3[test_metadata_has_input_schema]
B --> B4[test_metadata_has_valid_source]
B --> B5[test_is_available_returns_bool]
B --> B6[test_extract_params_returns_dict]
D --> D1[is_available: 3 cases]
D --> D2[extract_params: maps fields / defaults]
D --> D3[run happy path]
D --> D4[run error path]
D3 --> P1[patch get_query_activity\nat tool module level]
D4 --> P1
E --> E1[is_available: 3 cases]
E --> E2[extract_params: maps fields / defaults]
E --> E3[run with table_stats=True\nhealth available]
E --> E4[run with table_stats=False]
E --> E5[run skips table_stats\nwhen health unavailable]
E --> E6[run error path]
E3 --> P2[patch get_system_health\n+ get_table_stats]
E5 --> P3[patch both\nassert_not_called on table_stats]
E4 --> P4[patch get_system_health only\nno assert_not_called ⚠️]
Reviews (1): Last reviewed commit: "test(tools): add unit tests for ClickHou..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@Devesh36 @rrajan94 @yashksaini-coder kindly review |
|
thank you @Davidson3556 looks good |
2 participants
Closes #995
Describe the changes you have made in this PR -
Added unit tests for
ClickHouseQueryActivityToolandClickHouseSystemHealthTool, which previously had no test coverage. Created two new test files:tests/tools/test_clickhouse_query_activity_tool.py— 13 teststests/tools/test_clickhouse_system_health_tool.py— 15 testsEach file covers contract validation,
is_available,extract_params, run happy path, and run error path. No live ClickHouse instance needed — integration calls are patched.Demo/Screenshot for feature changes and bug fixes -
Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
Both tools are function-based
@tooldecorated functions that delegate toapp/integrations/clickhouse.py. The tests follow the same pattern used in similar tool tests in this repo (e.g.test_cloudwatch_logs_tool.py) — accessing__opensre_registered_tool__for contract checks and patching the integration functions at the tool module level so no real database connection is needed. The system health test also covers theinclude_table_statsbranching logic and verifiesget_table_statsis never called when health returns unavailable.Checklist before requesting a review