Test fixes for MST metadata expectations and User-Agent env leak - #788
Merged
vikrantpuppala merged 2 commits intoMay 18, 2026
Merged
Test fixes for MST metadata expectations and User-Agent env leak#788vikrantpuppala merged 2 commits into
vikrantpuppala merged 2 commits into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
vikrantpuppala
had a problem deploying
to
azure-prod
May 15, 2026 09:19 — with 
GitHub Actions
Failure
Clear all KNOWN_AGENTS env vars in test_useragent_header before connecting, mirroring the pattern in test_agent_detection.py. Without this, running the test inside an AI-agent shell (e.g. Claude Code, which sets CLAUDECODE=1) causes session.py to append " agent/<product>" to the User-Agent and the strict `in` assertion fails. CI runners don't set agent env vars so this passes there, but the test is environment- sensitive locally. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
vikrantpuppala
force-pushed
the
fix/useragent-test-env-leak
branch
from
b4ad201 to
4089f3f
Compare
May 15, 2026 09:30
vikrantpuppala
had a problem deploying
to
azure-prod
May 15, 2026 09:30 — with
GitHub Actions
Failure
… MST
The TestMstMetadata tests were written speculatively expecting the
Thrift Get{Catalogs,Schemas,Tables,Columns} RPCs to succeed inside an
active multi-statement transaction. The server's MST guard now rejects
these RPCs with TRANSACTION_NOT_SUPPORTED ("... is not supported within
a multi-statement transaction."), matching the intentional design where
MST exposes a small allowlist of SQL forms.
Flip the four `*_in_mst` tests into `*_blocked` tests asserting the
expected DatabaseError. The two `*_non_transactional_after_concurrent_*`
freshness tests are deleted outright — their entire premise ("Thrift
RPCs bypass MST and see concurrent DDL") is no longer valid, so there
is nothing meaningful left to assert.
Note: the rejection happens at the MST guard before reaching the txn,
so the active transaction remains usable after a blocked Get* (unlike
SQL forms in TestMstBlockedSql, where the failure aborts the txn).
The shared helper documents this distinction.
Verified against dogfood: 4 passed in 33.67s.
Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
msrathore-db
approved these changes
May 18, 2026
msrathore-db
left a comment
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
3 tasks
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
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.
Summary
Two independent test-only fixes bundled together because both were uncovered while investigating CI red on test-with-coverage.
1. Flip TestMstMetadata to assert blocked behavior
The four tests/e2e/test_transactions.py::TestMstMetadata::test_cursor_*_in_mst tests, added in #775, were written speculatively expecting Thrift Get{Catalogs,Schemas,Tables,Columns} RPCs to succeed inside an active multi-statement transaction. The server's MST guard is intentionally designed to reject these ("... is not supported within a multi-statement transaction."), matching the small allowlist pattern documented in TestMstBlockedSql.
One subtle difference vs TestMstBlockedSql's SQL-form blocks: the Get* rejection happens at the MST guard before reaching the transaction, so the active txn remains usable after a blocked Get* (whereas SQL-form blocks abort the txn). The shared helper documents this.
2. Fix test_useragent_header environment-sensitivity
tests/unit/test_session.py::test_useragent_header strictly asserts ("User-Agent", "<name>/<version>") in http_headers. When run inside any AI-agent shell that databricks.sql.common.agent.KNOWN_AGENTS knows about (e.g. Claude Code sets CLAUDECODE=1), session.py:68-70 appends agent/<product> to the header and the in check fails. CI runners don't set agent env vars, so this passed there — it's an environment-sensitivity bug rather than a code regression.
The fix mirrors the pattern in tests/unit/test_agent_detection.py:42-49: monkeypatch.delenv every known agent env var before the assertion. Iterating KNOWN_AGENTS keeps the cleared list automatically in sync if new agents are added to src/databricks/sql/common/agent.py.
Test plan
This pull request and its description were written by Isaac.