Skip to content

chore(tesseract): run pre-aggregation integration tests against live CubeStore#11018

Open
waralexrom wants to merge 8 commits into
masterfrom
tesseract-cubestore-integration-tests
Open

chore(tesseract): run pre-aggregation integration tests against live CubeStore#11018
waralexrom wants to merge 8 commits into
masterfrom
tesseract-cubestore-integration-tests

Conversation

@waralexrom
Copy link
Copy Markdown
Member

Summary

Tesseract's pre-aggregation integration tests previously executed their rollup queries against Postgres, which doesn't match production — in the real system external pre-aggregations live in CubeStore and are served by the CubeStore query engine. This PR makes those tests run against a live cubestored, so the suite exercises the actual production path and catches CubeStore/Tesseract consistency regressions.

Changes

  • integration-cubestore feature + harness: spawns a shared local cubestored (free ports, temp data dir, per-test schema, mysql_async), builds each pre-aggregation in Postgres, uploads it to CubeStore as CSV via CREATE TABLE ... LOCATION (the same import pipeline production rollups use), then runs the outer query rendered in the CubeStore dialect.
  • Production-faithful routing: rollup/rollupJoin default to external (mirroring CUBEJS_EXTERNAL_DEFAULT=true); try_execute sends external-covered queries to CubeStore (skipped without the feature) and everything else to Postgres. All pre_aggregations/ tests migrated.
  • CubeStore dialect fidelity in the mock: mirror the native DriverTools methods (date_binDATE_BIN, time_stamp_castCAST as TIMESTAMP, date_time_castto_timestamp, convert_tzCONVERT_TZ) plus the CubeStoreQuery.sqlTemplates() overrides, so the external path renders real CubeStore SQL rather than Postgres SQL.
  • Pre-agg indexes: parsed from the YAML mock and emitted as INDEX / AGGREGATE INDEX + AGGREGATIONS on the CubeStore table.
  • Deterministic, query-driven row order: each test carries a total order: (no post-sort masking), so ORDER BY rendering stays under test.
  • Binary resolution: prefers a local release build (where CI drops the build-cubestore artifact), falls back to the downloaded @cubejs-backend/cubestore release; debug builds warn (they stack-overflow on deep multi-stage plans) and require CUBESTORED_ALLOW_DEBUG=1 to opt in.
  • CI: unit-core consumes the existing build-cubestore release artifact and runs the cube workspace with cubesqlplanner/integration-cubestore — the artifact is always the current source tree's CubeStore, so one job covers both Tesseract and CubeStore regressions.

Testing

cargo test --features integration-cubestore (release cubestored): pre_aggregations 47 passed, cubestore 1 passed; cargo test --lib (no feature) 1001 passed. Verified migrated snapshot values match the prior Postgres results (modulo CubeStore decimal/timestamp formatting and the now query-driven order).

🤖 Generated with Claude Code

… on live CubeStore

Add an integration-cubestore feature that runs Tesseract queries fully
covered by external pre-aggregations against a locally built cubestored:

- cubestore_service.rs spawns a shared cubestored (free ports, temp data
  dir) and gives each test an isolated schema over mysql_async
- TestContext::try_execute_cubestore builds pre-agg tables in Postgres,
  uploads them into CubeStore (CREATE TABLE + INSERT), then runs the outer
  query rendered in the CubeStore dialect
- MockBaseTools carries external driver tools; cubestore_templates mirrors
  CubeStoreQuery.sqlTemplates() so external-covered queries render CubeStore SQL
- pre-agg YAML indexes parsed into the mock and emitted as INDEX /
  AGGREGATE INDEX + AGGREGATIONS on the CubeStore table
Replace CREATE TABLE + INSERT with writing a local CSV and loading it
through CREATE TABLE ... LOCATION, the same import pipeline production
rollups use. Values are serialized to match the JS postgres type parsers
(timestamp -> YYYY-MM-DDTHH:mm:ss.SSS, bool true/false, NULL as empty
field), which removes the timestamptz trim workaround.

Also harden the schema-qualifying regex with a trailing token boundary to
avoid prefix-name collisions, and remove the cubestored temp data dir on
exit.
Make pre-aggregation integration tests follow production routing: rollup
and rollupJoin default to external (CUBEJS_EXTERNAL_DEFAULT=true) in the
YAML mock, and TestContext::try_execute runs external-covered queries on
CubeStore (skipped without the integration-cubestore feature) and
everything else on Postgres. All pre_aggregations tests migrated.

Mirror the CubeStore dialect's native DriverTools methods in the mock
(date_bin -> DATE_BIN, time_stamp_cast -> CAST as TIMESTAMP,
date_time_cast -> to_timestamp, convert_tz -> CONVERT_TZ); previously the
external mock carried CubeStore templates but Postgres method bodies, so
it ran Postgres SQL against CubeStore. Sort result rows for deterministic
snapshots.
…build

Resolve cubestored from the downloaded @cubejs-backend/cubestore release
binary by default (suits Tesseract development without a 10-minute
CubeStore build). Set CUBESTORED_LOCAL_BUILD=1 to use a local cargo build
from rust/cubestore/target (release preferred — debug stack-overflows on
deep multi-stage plans). CUBESTORED_BIN_PATH still overrides everything.
Resolve cubestored as: local release build (where CI drops the
build-cubestore artifact) -> downloaded @cubejs-backend/cubestore release.
A debug build is ignored by default (it stack-overflows on deep
multi-stage plans) but warns when present and unused; CUBESTORED_ALLOW_DEBUG=1
opts into it for fast local iteration on features that don't hit the
overflow. CUBESTORED_BIN_PATH still overrides everything.
unit-core now consumes the build-cubestore release artifact (dropped into
rust/cubestore/target/release, where the resolver looks first) and runs the
cube workspace with cubesqlplanner/integration-cubestore instead of
integration-postgres, so pre-aggregation tests execute on CubeStore. The
artifact is the current source tree's CubeStore, so the same job catches
both Tesseract and CubeStore regressions.
Remove the post-execution row sort in the CubeStore result formatter and
give every executing pre-agg test a total `order:` over its group-by keys,
so row order is exercised by the query (CubeStore's auto ORDER BY is only
partial) instead of being silently masked. NULLs now sort last (ASC), as
CubeStore orders them.

Also make AGGREGATIONS rendering for an aggregate index fail loudly
(unimplemented!) on unsupported measure types (e.g. countDistinctApprox/HLL)
instead of silently dropping them while still emitting AGGREGATE INDEX.
@github-actions github-actions Bot added the rust Pull requests that update Rust code label Jun 5, 2026
@waralexrom waralexrom changed the title test(tesseract): run pre-aggregation integration tests against live CubeStore chore(tesseract): run pre-aggregation integration tests against live CubeStore Jun 5, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Jun 5, 2026

Claude finished @waralexrom's task in 2m 10s —— View job


Claude PR Review — LGTM with minor observations (0 blockers, 0 high, 0 medium, 4 low/nits)
  • Load PR context
  • Check existing review threads (none — no dedup needed)
  • Read core changes (harness, dialect mock, CI)
  • Spot-check pre-agg test migrations & snapshots
  • Post review

Summary

Solid, carefully written change. The new harness puts the pre-aggregation suite onto the real production path (Postgres builds the rollup → CSV → CREATE TABLE … LOCATION → CubeStore serves the outer query in CubeStore dialect), which is exactly the regression class that was previously invisible. The dialect mirroring in MockDriverTools (date_bin, time_stamp_cast, date_time_cast, convert_tz) and the CubeStore sqlTemplates() override are faithful enough for the migrated snapshots; the YAML-side external default of true for rollup/rollupJoin correctly mirrors CUBEJS_EXTERNAL_DEFAULT=true. The unit-core rewire to consume the in-tree build-cubestore artifact (instead of a downloaded release) means one CI job now covers both Tesseract and CubeStore at the current SHA — nice.

Correctness & design — looks good

  • try_execute routing (test_context.rs:488–513): external requires non-empty usages AND all usages external, which matches the contract; falls through cleanly to try_execute_pg otherwise.
  • try_execute_cubestore asserts the two preconditions (pre_aggregations non-empty + all external) before doing any work — good failure mode.
  • Table-name re-qualification regex (test_context.rs:797–809): the trailing ([\s,)]|$) boundary correctly prevents a visitors prefix from rewriting visitors_daily, and the captured opening/closing quote groups keep symmetric quoting. Also: a second pass over the same table can't match itself because the pattern requires FROM|JOIN\s+, not schema..
  • cubestore_table_extras_sql: AGGREGATIONS gated on the presence of an aggregate index, matching the JS createTableIndexes flow. The "column name might be granularity-suffixed" fallback (columns.iter().find(|(n, _)| n.starts_with(&format!("{}_", alias)))) is the right adjustment for time dimensions.
  • register_atexit_cleanup (cubestore_service.rs:14–35) is best-effort and correct given CS_INSTANCE is a process-wide OnceCell (single PID/dir to remember). SIGKILL will leak as usual, which is acceptable for tests.

Nits / small follow-ups

  1. cubestored stderr/stdout dropped (cubestore_service.rs:139–140): silencing both streams makes a startup failure hard to diagnose in CI — you'd see "did not open MySQL port within 60s" with no upstream log. Consider tee'ing to a file under the data dir (which is already temp-scoped and cleaned up) so a failed run leaves a breadcrumb. Low-priority.
  2. format_interval drops a trailing token on odd-count input (mock_driver_tools.rs:88–96): while let (Some(num), Some(unit)) = (tokens.next(), tokens.next()) silently swallows a leftover token. Inputs you'll see are always <n> <unit> pairs, but a debug_assert! or panic! on the leftover would catch a malformed interval immediately instead of silently rendering a wrong window.
  3. CubeStore templates override list is short (mock_sql_templates_render.rs:575–593): only time_series_select and operators/is_not_distinct_from are overridden vs. the JS CubeStoreQuery.sqlTemplates(), which has more. The migrated snapshots pass, so the existing surface is covered — but worth a comment marker (// add overrides here as new tests exercise them) so future maintainers don't assume parity. Optional.
  4. unimplemented!() for HLL aggregations (test_context.rs:1020–1024): fine as a runway, but the panic message refers to "the CubeStore test harness yet" — when a test eventually needs countDistinctApprox, the failure surface (a panic deep in upload_pre_agg_tables_to_cubestore) is somewhat obscure. A todo! or explicit skip-with-reason would be slightly friendlier. Optional.

Testing/CI

  • Per-PR description: 47 pre_aggregations tests passing with feature, 1001 --lib tests passing without — coverage matches the migration scope.
  • CI: unit-core is now correctly gated on build-cubestore and uses its release artifact via the path the resolver already prefers. The added chmod +x step is the standard fix for download-artifact losing the executable bit.
  • Per-test row order is now query-driven (order: blocks in every migrated test) — confirmed in spot-checked snapshots; removes the brittle post-sort and keeps ORDER BY rendering under test.

Security

Nothing concerning. The MySQL connection is loopback-only on a free port, the CSV path is process+schema+table scoped under temp_dir(), and there's no user input flowing into the spawned process or generated SQL.

· branch [`tesseract-cubestore-integration-tests`](https://github.com/cube-js/cube/tree/tesseract-cubestore-integration-tests)

…ature

MockPreAggregationIndex and the indexes field/accessor are only read by the
integration-cubestore-gated CubeStore table builder; allow dead_code only
when that feature is off so clippy --all-targets is clean in every config.
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Jun 5, 2026

Claude finished @waralexrom's task in 1m 7s —— View job


Claude PR Review — LGTM with minor observations (0 blockers, 0 high, 0 medium, 4 low/nits)

Re-review after sync — findings unchanged
  • Loaded PR context (8 commits, 74 files)
  • Verified prior review's points still apply against current tree
  • Confirmed no new inline threads to dedupe
  • Posted re-review

Summary

The 8 commits in this push are the same set already covered by the prior review at 11:45 — re-verified against the current tree, all findings still hold. The harness puts the pre-aggregation suite onto the real production path (Postgres builds the rollup → CSV → CREATE TABLE … LOCATION → CubeStore serves the outer query in CubeStore dialect), and CI's unit-core now consumes the in-tree build-cubestore artifact so one job covers both Tesseract and CubeStore regressions at the current SHA.

Correctness & design — looks good

  • try_execute routing (test_context.rs:488–513): external requires non-empty usages AND all usages external; falls through cleanly to try_execute_pg otherwise.
  • try_execute_cubestore asserts both preconditions before doing any work — good failure mode.
  • Table-name re-qualification regex (test_context.rs:797–809): trailing ([\s,)]|$) boundary correctly prevents a visitors prefix from rewriting visitors_daily; symmetric quoting preserved. A second pass over the same table can't match itself because the pattern requires FROM|JOIN\s+, not schema..
  • cubestore_table_extras_sql: AGGREGATIONS gated on the presence of an aggregate index, matching the JS createTableIndexes flow. The granularity-suffix fallback (columns.iter().find(|(n, _)| n.starts_with(&format!("{}_", alias)))) is the right adjustment for time dimensions.
  • register_atexit_cleanup (cubestore_service.rs:14–35) is best-effort and correct given CS_INSTANCE is a process-wide OnceCell. SIGKILL will leak as usual — acceptable for tests.

Nits / small follow-ups (unchanged from prior review)

  1. cubestored stderr/stdout dropped (cubestore_service.rs:139–140): silencing both streams makes a startup failure hard to diagnose in CI — "did not open MySQL port within 60s" with no upstream log. Consider tee'ing to a file under the data dir (already temp-scoped and cleaned up) so a failed run leaves a breadcrumb. Low-priority.
  2. format_interval drops a trailing token on odd-count input (mock_driver_tools.rs:88–96): while let (Some(num), Some(unit)) = (tokens.next(), tokens.next()) silently swallows a leftover token. Inputs are always <n> <unit> pairs in practice, but a debug_assert! would catch a malformed interval immediately.
  3. CubeStore templates override list is short (mock_sql_templates_render.rs:575–593): only time_series_select and operators/is_not_distinct_from overridden vs. the JS CubeStoreQuery.sqlTemplates(). Migrated snapshots pass — but a // add overrides here as new tests exercise them marker would prevent maintainers from assuming parity. Optional.
  4. unimplemented!() for HLL aggregations (test_context.rs:1020–1024): the panic surface is somewhat obscure (deep in upload_pre_agg_tables_to_cubestore). A todo! with explicit message or a skip-with-reason would be friendlier when a test eventually needs countDistinctApprox. Optional.

Testing/CI

  • 47 pre_aggregations tests pass with the feature; 1001 --lib tests pass without — matches migration scope.
  • unit-core correctly gated on build-cubestore and uses its release artifact via the resolver's preferred path. The chmod +x step is the standard fix for download-artifact losing the executable bit.
  • Per-test row order is now query-driven (order: blocks in every migrated test) — removes the brittle post-sort and keeps ORDER BY rendering under test.

Security

Nothing concerning. MySQL connection is loopback-only on a free port; CSV path is process+schema+table scoped under temp_dir(); no user input flows into the spawned process or generated SQL.

· branch tesseract-cubestore-integration-tests ·

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.82%. Comparing base (a2c65aa) to head (2d38992).
⚠️ Report is 18 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11018      +/-   ##
==========================================
- Coverage   83.32%   78.82%   -4.50%     
==========================================
  Files         255      471     +216     
  Lines       77177    94442   +17265     
  Branches        0     3502    +3502     
==========================================
+ Hits        64304    74446   +10142     
- Misses      12873    19488    +6615     
- Partials        0      508     +508     
Flag Coverage Δ
cube-backend 58.57% <ø> (?)
cubesql 83.33% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant