test(e2e): give the shared cluster warm-up a longer, single deadline - #2141
Draft
rugpanov wants to merge 7 commits into
Draft
test(e2e): give the shared cluster warm-up a longer, single deadline#2141rugpanov wants to merge 7 commits into
rugpanov wants to merge 7 commits into
Conversation
startCluster runs in onPrepare, before any spec or wdio session — so
specFileRetries can't recover a failure here; if it throws, the whole shard
aborts. The old budget was a fixed 100 attempts x 10s (~17 min) of polling
while the shared test cluster sat PENDING.
Cloud-side node placement can take much longer than that: the shared cluster
has been observed reaching RUNNING only ~1h after an UNEXPECTED_LAUNCH_FAILURE
("Timeout while placing nodes") that Databricks retried internally. Shards
polling PENDING gave up at ~17 min and failed en masse, even though the change
under test was unrelated (a pre-test infra timeout, not an assertion failure).
Rework startCluster:
- Poll against a single 60-minute deadline instead of a fixed attempt count.
The e2e job has no timeout-minutes (GitHub's 6h default), so 60 min is safe.
- Drop the SDK .wait() on the start path so the deadline is the only budget
(the waiter carried its own, shorter, hidden timeout).
- Tolerate a concurrent start() on the shared cluster (a sibling shard may have
already issued it) and keep polling instead of erroring out.
- Log state_message so the cloud-side reason for a slow/failed launch is
visible directly in CI.
Co-authored-by: Isaac
rugpanov
had a problem deploying
to
test-trigger-is
August 21, 2026 13:47 — with 
GitHub Actions
Error
GitHub Actions
Error
Contributor
Author
|
🤖 Integration tests ❌ 6 of 41 test jobs failed for |
…loop Replaces the previous commit's hand-rolled deadline/polling loop with a reuse of the production Cluster.start(), addressing review feedback (a blanket catch that turned permanent start failures into hour-long retries; a hand-rolled loop duplicating logic the repo already has). Cluster.start() already polls a stopped cluster to RUNNING and fails fast on TERMINATED/ERROR (surfacing termination_reason) via the SDK retry() idiom. Its only limitation here was the timeout: - Add an optional `timeout` param to Cluster.start(), defaulting to the SDK DEFAULT_MAX_TIMEOUT so existing callers are unchanged — mirroring ExecutionContext/Command which already expose the same knob. Covered by a new Cluster.test.ts case. - The e2e harness builds the cluster via Cluster.fromClusterId and passes a 60 min timeout (the shared cluster's node placement has been seen taking ~1h; the e2e job has no timeout-minutes, so GitHub's 6h default bounds it). Co-authored-by: Isaac
rugpanov
requested a deployment
to
test-trigger-is
August 21, 2026 16:32 — with
GitHub Actions
Queued
GitHub Actions
Queued
Contributor
Author
|
🤖 Integration tests triggered for |
Round-2 review of the previous approach (reusing the production Cluster.start()) surfaced that fitting the shared-cluster e2e scenario would need production changes: a caller timeout threaded through the TERMINATING wait, tolerance for the concurrent-start race across ~40 shards, and per-poll state_message logging. Changing Cluster.start() (used by 10+ callers incl. the UI) for a test-only need is too wide a blast radius. Instead keep production untouched and give the e2e path its own helper (src/test/startCluster.ts), duplicating the SDK retry() idiom: - One 60-minute deadline over the whole start (the shared cluster's cloud node placement has been seen taking ~1h; the e2e job has no timeout-minutes). - Fails fast: a terminal state after start is surfaced as ClusterStartError with the cloud reason, not retried to the deadline. - Tolerates the shared-cluster start race: a sibling shard's start() is swallowed and the poll (fatal-on-terminal) still catches real failures. - Logs state_message each poll for CI visibility. Reverts the Cluster.start()/Cluster.test.ts changes from the previous commit; covered by src/test/startCluster.test.ts. Co-authored-by: Isaac
Contributor
Author
|
🤖 Integration tests ❌ 38 of 41 test jobs failed for |
rugpanov
requested a deployment
to
test-trigger-is
August 21, 2026 16:49 — with
GitHub Actions
Queued
GitHub Actions
Queued
Round-3 review fixes for the cluster warm-up util: - Narrow the catch: only tolerate the concurrent-start race (on re-check the cluster is already coming up); rethrow genuine start failures (auth, permissions, bad request) instead of masking them behind a later terminal-state error. - Fall back to "unknown reason" in the failure message when neither state_message nor termination_reason is set (avoids a literal "undefined"). - Add tests for the TERMINATING wait path and for a propagated non-race start error; fix the test's import order. Co-authored-by: Isaac
rugpanov
requested a deployment
to
test-trigger-is
August 21, 2026 17:09 — with
GitHub Actions
Queued
GitHub Actions
Queued
Contributor
Author
|
🤖 Integration tests triggered for |
…line Round-4 review fixes for the cluster warm-up util: - Treat UNKNOWN as terminal in the start poll (flagged by Codex and Claude): once the start is issued, UNKNOWN is a failed launch, so fail fast instead of retrying to the timeout. Consistent with the pre-start / race-recheck sets. - Share one deadline across the TERMINATING wait and the RUNNING poll: each retry() previously received the full timeout, so a slow shutdown could nearly double the caller's bound. Compute the deadline once and pass the remaining time to each phase. - Add a test for UNKNOWN-after-start. Co-authored-by: Isaac
rugpanov
requested a deployment
to
test-trigger-is
August 21, 2026 17:20 — with
GitHub Actions
Queued
GitHub Actions
Queued
Contributor
Author
|
🤖 Integration tests triggered for |
Make the race test's success explicit (verifyStarted) rather than relying only on the awaited promise not throwing. Round-5 review nit. Co-authored-by: Isaac
rugpanov
requested a deployment
to
test-trigger-is
August 21, 2026 17:25 — with
GitHub Actions
Queued
GitHub Actions
Queued
Contributor
Author
|
🤖 Integration tests triggered for |
state_message is a string and termination_reason is an object; stringify only the object so a plain message reads cleanly (no wrapping quotes). Round-6 review nit. Co-authored-by: Isaac
Contributor
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
rugpanov
requested a deployment
to
test-trigger-is
August 21, 2026 17:31 — with
GitHub Actions
Queued
GitHub Actions
Queued
Contributor
Author
|
🤖 Integration tests triggered for |
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.
Problem
startClusterruns in wdio'sonPrepare— before any spec or even any wdio session — sospecFileRetriescannot recover a failure here; if it throws, the whole shard aborts with no retry.Its budget was a fixed 100 attempts × 10s (~17 min) of polling while the shared test cluster sat
PENDING. Cloud-side node placement can take much longer: the shared cluster has been observed reachingRUNNINGonly ~1h after anUNEXPECTED_LAUNCH_FAILURE("Timeout while placing nodes") that the platform then retried internally. Shards pollingPENDINGgave up at ~17 min and failed en masse — even though the change under test was unrelated (a pre-test infra timeout, not an assertion failure). Because ~40 shards share one cluster, a single slow launch reddens many shards at once.Change
Rework
startCluster:timeout-minutes(GitHub's 6h default), so 60 min is comfortably within it..wait()on the start path so the deadline is the only budget (the waiter carried its own, shorter, hidden timeout).start()on the shared cluster — a sibling shard may have already issued it — and keep polling instead of erroring out.state_messageso the cloud-side reason for a slow/failed launch is visible directly in CI.Testing
state_messagefield cross-checked against existing SDK usage in this repo.tsc/eslint/prettierwere not run (fresh checkout without the private registry); CItest:lintand e2e-typecheck are the gate.Opened as draft pending review.