Skip to content

Tests: Snowflake and Databricks error / lifecycle / edge paths via wiremock - #1218

Closed
rollroyces wants to merge 1 commit into
deeplethe:devfrom
rollroyces:fix/snowflake-databricks-boundary-tests-v2
Closed

rollroyces wants to merge 1 commit into
deeplethe:devfrom
rollroyces:fix/snowflake-databricks-boundary-tests-v2

Conversation

@rollroyces

Copy link
Copy Markdown
Contributor

Summary

Adds nine wiremock tests across the two HTTP-family query engines
for error and lifecycle paths that current dev does not already cover.

crates/utopia-server/src/query_engine/snowflake.rs:

  • auth_failure_body_is_surfaced — 401 / 403 / 503 share the same
    Snowflake {status}: {message} surfacing path; one body template
    runs three of them so a future unification or refactor of the
    error-mapping gets caught immediately.
  • non_json_error_body_falls_back_to_plain_text — 5xx with a
    non-JSON body (e.g. Internal error: warehouse suspended) still
    reaches the caller as a printable message; this pins the
    serde_json::from_str::<StatementResponse> failure branch in
    run.
  • business_error_code_in_a_2xx_body_is_bubbled_up — a successful
    2xx response carrying code: 000605 ("Statement was canceled by
    user") with no meta triggers the code != "090001" branch in
    run and surfaces the message; this is the "looks OK but isn't"
    case that would otherwise pass through as if nothing failed.

crates/utopia-server/src/query_engine/databricks.rs:

  • an_unauthorized_http_status_is_surfaced_as_a_connection_error —
    401 surfaces through reqwest::error_for_status(). Pinned so
    callers can recognise auth failures by status code, not message.
  • a_5xx_response_is_surfaced_with_status_code — 503 same shape.
  • a_canceled_statement_carries_a_distinct_message — state: CANCELED
    is a separate lifecycle state from FAILED, with error_code: CANCELED;
    the message must reach callers distinct from "the query failed".
  • pending_state_without_statement_id_is_surfaced — state: PENDING
    without statement_id would loop forever pulling non-existent handles.
    The existing ok_or_else("Databricks returned no statement_id")
    bail needs this as a regression anchor in case anyone refactors the
    id.required() check.
  • a_succeeded_statement_with_no_rows_returns_empty — SUCCEEDED
    with an empty data_array returns rows.is_empty(); pins the
    "DDL / 0-row query / empty schema" path through run.

Why v2

The previous attempt (#1207) was based on a fork c9760a1 that had
diverged from upstream/dev (now 25fa9f4, rc9 + ~20 commits).
crates/utopia-server/src/query_engine/snowflake.rs and
.../databricks.rs have both been substantively rewritten in upstream
since — Snowflake gained result-partition reading (#1062), Databricks
gained follow-chunk-links (#1063) and information-schema fallback
(#241, #702). The wiremock body shapes used by these tests were
chosen to drive the early-return paths in run (non-success status
code, missing handle, FAILED/CANCELED state) and so the test bodies
do not need to change with the new code. They are simply re-applied
to the current tree.

What this PR is not

  • No new engine. Snowflake and Databricks engine bodies are untouched.
  • No behavior change to existing tests. The existing
    a_synchronous_answer_is_typed_by_row_type,
    a_202_is_polled_and_its_partitions_keep_the_first_rows_types,
    an_error_body_is_surfaced,
    a_schema_read_follows_partitions_until_exhausted,
    multiple_partitions_require_a_statement_handle,
    failed_or_incomplete_partitions_do_not_return_partial_rows
    on snowflake, and the corresponding six on databricks, still pass
    through unchanged paths.
  • No CLI / API / UI change.

Verification

  • cargo check -p utopia-server --bin utopia-server did not finish
    in the local sandbox (dev-deps cold rebuild chain on the external
    FUSE mount). The new tests are pure wiremock — no DB, no real
    cluster. Recommend running on a workstation with a normal
    filesystem before merge.

Open question for the maintainer

If the test suite already runs against test_db::url() for the
wiremock tests as well, that's a bonus; if not, direction on where
these tests should be relocated would help future PRs of the same
shape. Looking at crates/utopia-server/src/query_engine/snowflake.rs::tests
and .../databricks.rs::tests they are already inline #[cfg(test)]
and match the surrounding convention, so I'm leaving them there.

…remock

Signed-off-by: Royce Lam <roycelam@umich.edu>
WaylandYang added a commit that referenced this pull request Oct 14, 2026
#1221)

* Tests: Snowflake and Databricks error / lifecycle / edge paths via wiremock

Signed-off-by: Royce Lam <roycelam@umich.edu>

* Format the Snowflake and Databricks error-path tests

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Wayland Yang <wayland0916@gmail.com>

---------

Signed-off-by: Royce Lam <roycelam@umich.edu>
Signed-off-by: Wayland Yang <wayland0916@gmail.com>
Co-authored-by: Royce Lam <roycelam@umich.edu>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@WaylandYang

Copy link
Copy Markdown
Contributor

感谢重新整理,这次只有测试本身,很清楚。

已经通过 #1221 合入 dev(c278cb8):你的提交原样保留、署名不变,我们只补了一次 cargo fmt(lint 只差这两处换行)。你的分支没有开"允许维护者修改",所以没法直接推到这里,这一条就先关闭了。

再次感谢。

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants