test(e2e): make E2E suite robust to non-US / keypair / branched-storage projects - #391
Merged
Conversation
…ge projects Surfaced by running the E2E suite in CI against a GCP / Snowflake-keypair project (the new nightly e2e.yml from #385). All findings are test/environment robustness, NOT product bugs -- no product code changed. - ConfigSecretEncryption (x2): read `created["data"]["id"]`, not `created["id"]`. `config new --push` returns the standard `{status, data}` envelope; the test forgot to unwrap `["data"]` (the sibling `_test_config_new_push` already does). Manifested as `KeyError: 'id'`. - swap-tables rejection test: assert `"requires a branch"` instead of the old `"dev branch"` wording. The wording was corrected in #373 (swap works on any branch, incl. production); the CLI test was updated there but this E2E assertion was missed. - FullE2E workspace password: keypair-auth workspaces (Snowflake person-keypair login) have no password, so the API returns HTTP 400 "not supported for login type ..." -- skip that step cleanly instead of failing the whole run. - metastore_scope_available preflight: also treat an unreachable metastore host (connection error) as "scope unavailable" and skip, so a misconfigured stack URL yields one clean skip instead of a wall of errors across every dependent test.
Comment on lines
+7407
to
+7408
| # match the stable part of the message, not the old "dev branch" phrasing. | ||
| assert "requires a branch" in payload["error"]["message"] |
There was a problem hiding this comment.
🚩 Assertion wording change assumes PR #373 is merged
Line 7408 changes the assertion from "dev branch" in payload["error"]["message"] to "requires a branch" in payload["error"]["message"]. The comment explains the wording was corrected in PR #373 (swap works on any branch, including production). If this PR is merged before #373, or if #373's wording doesn't contain exactly "requires a branch", this E2E test would fail. Since the PR is presumably based on a branch that includes #373's changes, this is fine — but worth verifying during review that the merge-base includes the wording change.
Was this helpful? React with 👍 or 👎 to provide feedback.
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.
Why
The nightly
e2e.ymlfrom #385 ran the E2E suite against a GCP / Snowflake-keypair project for the first time — a different environment than the US / password stack the suite was originally written against. After fixing a doubledE2E_URLsecret (operator config, which had caused an unrelated metastore-URL cascade), 6 real failures remained. All are test / environment robustness, not product bugs — no product code is touched here.This PR fixes the 4 test-side ones. The other 2 (
NativeTypes/BranchMaterialize,CloneTable) were purely environmental (branched storage was disabled on the project) and pass once it is enabled.What (tests only)
ConfigSecretEncryption×2 — readcreated["data"]["id"], notcreated["id"].config new --pushreturns the standard{status, data}envelope; the test forgot to unwrap["data"](the sibling_test_config_new_pushalready does it). Manifested asKeyError: 'id'.swap-tablesrejection test — assert"requires a branch"instead of the old"dev branch"wording. The message was corrected in fix(storage): correct stale "dev branch only" swap-tables wording #373 (swap works on any branch, incl. production); the CLI test was updated there but this E2E assertion was missed.FullE2Eworkspace password — keypair-auth workspaces (Snowflakeperson-keypairlogin) have no password, so the API returns HTTP 400 "not supported for login type …". Skip that one step cleanly instead of failing the whole run.metastore_scope_availablepreflight — also treat an unreachable metastore host (connection error) as "scope unavailable" and skip, so a misconfigured stack URL yields one clean skip instead of a wall of errors across every dependent test.Verification
ruff+tyclean; full suite collects (3976 tests).e2e.yml(chore(ci): silent-drift gate, ty/changelog/error-code gates, py3.13 matrix, nightly e2e #385); this PR is what makes that run green on non-US / keypair / branched-storage projects.Out of scope
NativeTypes/CloneTablewere branched-storage-off (environment), not code.