test(query-engine): add differential PromQL testing infra (#590, #594) - #630
Draft
milindsrivastava1997 wants to merge 10 commits into
Draft
test(query-engine): add differential PromQL testing infra (#590, #594)#630milindsrivastava1997 wants to merge 10 commits into
milindsrivastava1997 wants to merge 10 commits into
Conversation
Adds promql-compliance/seeder, a standalone Go module that builds a prompb.WriteRequest for a fixed, hand-authored dataset (counters, a gauge, and a label-churn series designed to exercise instant-vs-range divergence), snappy-compresses it, and pushes the same bytes via remote write to both a reference Prometheus and ASAPQuery's own remote-write ingest endpoint. This is the data-seeding half of the differential PromQL compliance harness proposed in #594; the comparison-harness half is a separate workstream. Co-Authored-By: Claude Sonnet 5 <[email protected]>
…oracle (#590) Adds the test #590 calls out as missing: for a stable key set, one range(start,end,step) query's per-timestamp series must exactly equal N separate instant(t) queries (one per step), not just checked at a single timestamp. Covers {Tumbling, Sliding} x {single-population Sum, dual-population Count} x >=4 steps. All four pass against current code. Co-Authored-By: Claude Sonnet 5 <[email protected]>
…ce-tester Vendor prometheus/compliance/promql (commit 67b8327, Apache 2.0) into promql-compliance/harness/ and patch Comparer.Compare to also run and diff an instant query (PromAPI.Query) alongside the existing range query, since upstream only ever exercised QueryRange. Range and instant outcomes are tracked and reported independently (Result.RangeSuccess/InstantSuccess), so "PASS: range, FAIL: instant" is a representable outcome instead of one aggregate pass/fail -- this is the exact bug class cataloged in #589. Adds config.yaml with placeholder reference/test target URLs and four seed regression test cases ported from the query patterns described in #589, #583, and #584 (top-k over range, per-step key snapshot churn, and two sliding-window rate queries). Adds comparer_test.go with fake in-process PromAPI doubles covering both diverging directions plus error/should-fail handling, since no unit tests existed upstream. Part of #594. Co-Authored-By: Claude Sonnet 5 <[email protected]>
…590) validate_range_query_params had no dedicated tests. Adds direct unit tests for its three error branches (start>=end, step==0, step not a multiple of the tumbling window) plus the happy path, in an inline test module next to the function (it's private and validate_range_query_params's error string is discarded before reaching any public caller, so the exact-string assertions can't be made from crate::tests). Also adds end-to-end coverage via handle_range_query_promql confirming each bad-param case is actually rejected in practice, including the start == end boundary specifically, and a new test that runs the same keys-but-no-value orphan-group scenario through both the instant and range entry points and diffs their skip/error behavior explicitly, so a future regression that splits their behavior fails here instead of only in one of the two existing per-path tests.
Adds simulate_sliding_window_keyed, a pure-function sliding-window oracle extended to keyed/grouped data (a sibling to the existing simulate_sliding_window/simulate_sliding_window_with_alignment, kept separate since those two are exercised by several existing index-slicing call sites that don't have a notion of a key). Drives it against a small deterministic sweep of window/slide configs and per-key presence patterns (appearing, disappearing, oscillating, gap mid-range) through a real SimpleEngine via create_engine_multi_timestamp_with_window + handle_range_query_promql. The sweep surfaced a real bug: execute_range_query_pipeline computes each Sliding-window step's window_start as current_time.saturating_sub(lookback_ms), so every output step before window_size_ms worth of history exists aliases onto the store's start=0 window instead of correctly having no sample. Captured as a minimal, #[ignore]'d regression (sliding_window_range_query_start_before_window_size_ms_returns_wrong_value) rather than patched, per #590's ground rules; the main property sweep starts each scenario at its own window_size_ms to avoid that known-buggy region while still exercising the key-expansion/merge behavior it's meant to check. Co-Authored-By: Claude Sonnet 5 <[email protected]>
#590) # Conflicts: # asap-query-engine/src/tests/mod.rs
milindsrivastava1997
marked this pull request as draft
August 26, 2026 12:28
milindsrivastava1997
changed the title
Contributor
Author
HandoffBuilt via 5 parallel agents against genuine gaps in existing coverage (a lot of instant/range regression territory was already tested — this fills what wasn't). Done, in this PR
Found along the way
TBD / not in this PR
Review pointers
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Parallel test-infrastructure work for #590 and #594, run as 5 scoped agents against genuinely uncovered gaps (the existing suite already covers a lot of instant/range regression territory — this fills what was left).
#590 — Rust test suite for instant/range PromQL paths (
asap-query-engine)range_multistep_instant_equivalence_tests.rs— the generic oracle the issue called out as missing: a multi-step range query vs. N independent instant queries, one per step, across Tumbling/Sliding × single/dual-population. All pass (stable-key-set happy path).range_query_validation_tests.rs+ inline tests insimple_engine/mod.rs— boundary coverage forvalidate_range_query_params(start>=end, step==0, step-not-multiple-of-window, start==end), plus an instant/range skip-behavior parity check for orphan groups.sliding_window_keyed_oracle_tests.rs+ newsimulate_sliding_window_keyedreference oracle — a property sweep (21 scenarios) over keyed sliding-window merges.Bug found: the sliding-window sweep surfaced a real bug in
execute_range_query_pipeline— for a step withcurrent_time < window_size_ms,window_start = current_time.saturating_sub(window_size_ms)clamps to0, aliasing onto the same store bucket as the legitimate step atcurrent_time == window_size_ms. The too-early step gets a phantom duplicate of the later step's value instead of correctly having no sample. Pinned as a minimal,#[ignore]d regression test (not fixed here — out of scope for this PR, tracked separately).#594 — Go differential e2e harness (
promql-compliance/)seeder/— remote-write dataset seeder (prompb + snappy) pushing a fixed, hand-authored dataset to twoquery_urls, so both a reference Prometheus and ASAPQuery ingest through the identical code path. Includes a deliberately gapped series to probe instant-vs-range divergence.harness/— forkedprometheus/compliance/promql, with a surgical patch toComparer.Compareso it also runs and diffs instant queries (previously range-only), plusconfig.yamlseeded with 4 regression cases tagged to Range query key expansion uses one snapshot instead of per-step keys #583/Range queries drop self-keyed accumulator expansion (top-k) for single-population metrics #584/Catalog: instant-query vs range-query behavioral inconsistencies #589.No live Prometheus/ASAPQuery run yet — this PR is code-only for the Go harness, per plan. CI wiring is deliberately deferred.
Test plan
cargo fmt,cargo check,cargo clippy,cargo testall pass (full pre-commit suite, run on the merged result)go build ./...andgo vet ./...pass for bothpromql-compliance/seederandpromql-compliance/harness🤖 Generated with Claude Code