Skip to content

test(planner): assert the undeclared-gauge p99 outcome, not a candidate id - #801

Merged
zzylol merged 2 commits into
mainfrom
fix/main-red-795-tests
Oct 21, 2026
Merged

zzylol merged 2 commits into
mainfrom
fix/main-red-795-tests

Conversation

@zzylol

@zzylol zzylol commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Why

Main is red. an_undeclared_gauges_merged_ddsketch_panes_get_no_relative_bound_through_avg expects the planning error to name the merged-pane DDSketch candidate. The joint search now discards a query's sketch candidates before composition when they cannot meet its target. After that, the error lists only the candidates that reached Stage 3: P1 (RawDataNotRetained).

What

The test now checks the intended outcome rather than a candidate id:

  • planning fails with no valid candidate, so no merged-pane DDSketch candidate is accepted;
  • the rejection the report keeps is listed, P1 with RawDataNotRetained (the exact plan, which needs raw samples);
  • through the exhaustive search (Stage1ToStage3Search::exhaustive()), which keeps every candidate in Stage 3, every candidate that keeps a DDSketch pane chain is rejected with AccuracyTargetMissed and no relative bound. The test therefore fails if those candidates disappear or are rejected for another reason. Candidates are found by their pane chain, not by id.

Planner behavior does not change. In the declared case, the joint search selects the same merged-pane DDSketch plan that the exhaustive search (stage_pipeline) selects, at the same cost of 21.2225 per second. Only its candidate id differs: P3-m1 instead of P5-m1.

How

The test is in crates/planner/tests/runtime_install.rs. Its assertion on "P5-m1" is replaced with assertions on no valid candidate and on the P1 RawDataNotRetained rejection, plus an exhaustive-search run that checks each merged-pane DDSketch candidate's rejection cause and reason. The declared-gauge test's doc comment no longer names the old P5-m1 id.

Key code interfaces added / modified

None. Tests only.

Before this PR

cargo test --workspace --locked on main (aceedff) fails in an_undeclared_gauges_merged_ddsketch_panes_get_no_relative_bound_through_avg.

After this PR

Main's test suite passes. The declared-gauge test is unchanged: the merged-pane DDSketch plan is selected, has a relative bound ≤ 0.05, and matches the query-time reference and the exact answers.

Verification

  • cargo test --workspace --locked: 1815 passed, 0 failed (before the exhaustive-search assertion was added).
  • cargo test -p asap-planner --test runtime_install --locked: 18 passed, 0 failed (with it).
  • cargo clippy --workspace --all-targets --all-features --locked -- -D warnings: clean.
  • cargo fmt --all -- --check: clean.

…ot a candidate id

The joint search discards the sketch candidates of a query before
composition when they cannot meet its target, so the planning error
lists only the candidates that reached Stage 3. Without the
non-negative declaration that is the exact plan, rejected because raw
samples are not retained. The test now checks that planning fails with
no valid candidate and that this rejection is reported, instead of
expecting the merged-pane candidate's id in the error.
@zzylol zzylol added accuracy Accuracy requirements, guarantees, and propagation bug Something isn't working promql PromQL front-end lowering (L1→L2) labels Oct 21, 2026
…declared gauge

Through the exhaustive search, every Stage 2 candidate that keeps a
DDSketch pane chain is rejected with AccuracyTargetMissed and no
relative bound, so the test no longer passes if those candidates are
missing or rejected for another reason. The declared-gauge test's doc
no longer names the old P5-m1 id.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accuracy Accuracy requirements, guarantees, and propagation bug Something isn't working promql PromQL front-end lowering (L1→L2)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant