PromQL support: Simplify evaluation test#93350
PromQL support: Simplify evaluation test#93350nikitamikhaylov merged 2 commits intoClickHouse:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the PromQL evaluation test file to improve code organization and maintainability. The main changes include moving test data setup into the fixture, consolidating test logic into reusable helper functions, and splitting a single large test into multiple focused test functions.
Key changes:
- Test data setup moved from individual tests into the
start_clusterfixture - Two new helper functions
do_query_testanddo_range_query_testreplace repetitive assertion code - The large
test_firstfunction split into three focused tests:test_range_selectors,test_instant_selectors, andtest_function_over_time
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/integration/test_prometheus_protocols/test_evaluation.py | Refactored test structure by introducing helper functions and splitting tests into smaller, more focused test cases |
| src/Storages/TimeSeries/PrometheusHTTPProtocolAPI.cpp | Simplified query execution by removing redundant context setup and settings configuration |
|
Workflow [PR], commit [a1d942a] Summary: ❌
|
clickhouse-gh
bot
added
the
pr-not-for-changelog
| if (!sql_query) | ||
| throw Exception(ErrorCodes::BAD_ARGUMENTS, "Failed to convert PromQL to SQL"); | ||
|
|
||
| auto query_context = getContext(); |
There was a problem hiding this comment.
I changed this code because at this point the query context is already completely initialized in PrometheusRequestHandler::QueryAPIImpl::makeContext() and trying to set query_id again here is not only unnecessary but also can trigger this assertion in executeQuery().
| False, | ||
| ) | ||
|
|
||
| queries = [ # array of tuples (query, timestamp, http_api_result, result of prometheusQuery, flag is ClickHouse HTTP API result is same as Prometheus one) |
There was a problem hiding this comment.
Here I changed the test approach. Instead of the big list containing all promql queries, now each test calls function do_query_test() once per each promql query. The new approach is easier for debugging because it makes easier to see points of failure.
0964d59
robot-ch-test-poll4
added
the
pr-synced-to-cloud
JTCunning
added
the
comp-promql
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
PromQL support: Simplify evaluation test
test_prometheus_protocols/test_evaluation.pyPart of #89356