Skip to content

Commit 09d61af

Browse files
test(kernel): exercise use_kernel=True through the real wheel; no silent skips
Ensure every CI job that's meant to cover the kernel actually drives the use_kernel=True path through the REAL databricks-sql-kernel wheel, and fails loudly if it can't (rather than silently skipping / passing on the Thrift path). Problem this fixes: - The kernel unit tests inject a fake databricks_sql_kernel into sys.modules. In a shared `pytest tests/unit tests/e2e` session (the coverage job, which installs --all-extras so the real wheel IS present) that fake shadowed the real wheel, so the kernel e2e tests silently skipped — the coverage job looked like it exercised the kernel but didn't. Changes: - tests/e2e/test_kernel_backend.py + test_kernel_tls.py: replace the silent `__file__`-based skip with a three-state guard keyed on importlib.metadata (the on-disk dist DB, which a sys.modules stub can't fake): skip only when the wheel is genuinely absent; FAIL LOUDLY when it's installed-but-shadowed. The `conn` fixture now also asserts conn.session.backend is KernelDatabricksClient, so a use_kernel=True connection that fell back to Thrift fails the test. - tests/unit/test_session.py: add TestUseKernelRoutesThroughRealWheel (marked `realkernel`) — a no-network proof that sql.connect(use_kernel=True) instantiates the REAL KernelDatabricksClient (mocks only open_session; does not fake the wheel). Skips if the wheel is absent; fails if it's shadowed. - pyproject.toml: register the `realkernel` marker. Tests so marked need an unpolluted sys.modules and must run in a separate pytest invocation from the fake-injecting unit tests. - tests/unit/test_kernel_client.py: document that its session-global fake mandates the separate-invocation rule for real-wheel tests. - code-quality-checks.yml: the Unit Tests + Kernel matrix now asserts the real wheel, runs `tests/unit -m "not realkernel"`, then runs the real-wheel routing test as its own invocation (`pytest tests/unit/test_session.py -m realkernel`). All three unit matrices gained `-m "not realkernel"`. - code-coverage.yml: --ignore the kernel e2e files and add `-m "not realkernel"` so the shared --all-extras session doesn't trip the new loud guards; the real live kernel e2e stays in kernel-e2e.yml (isolated session, real wheel, live warehouse). Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
1 parent e106ccb commit 09d61af

7 files changed

Lines changed: 204 additions & 39 deletions

File tree

.github/workflows/code-coverage.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,21 @@ jobs:
6363
install-args: "--all-extras"
6464
- name: Run all tests with coverage
6565
continue-on-error: false
66+
# This job installs --all-extras, so the REAL databricks-sql-kernel
67+
# wheel is present. The unit suite fakes databricks_sql_kernel in
68+
# sys.modules, which would shadow the real wheel in this shared
69+
# session — so the kernel-backed suites that need the real wheel
70+
# are excluded here and covered by the dedicated kernel-e2e.yml
71+
# (isolated session, real wheel, live warehouse):
72+
# - --ignore the kernel e2e files (they assert the real wheel and
73+
# now FAIL LOUDLY rather than silently skip if shadowed), and
74+
# - -m "not realkernel" deselects the no-network real-wheel
75+
# routing test for the same reason.
6676
run: |
6777
poetry run pytest tests/unit tests/e2e \
78+
--ignore=tests/e2e/test_kernel_backend.py \
79+
--ignore=tests/e2e/test_kernel_tls.py \
80+
-m "not realkernel" \
6881
-n 4 \
6982
--dist=loadgroup \
7083
--cov=src \

.github/workflows/code-quality-checks.yml

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ jobs:
4848
echo "=== Dependency Version: ${{ matrix.dependency-version }} ==="
4949
poetry run pip list
5050
- name: Run tests
51-
run: poetry run python -m pytest tests/unit
51+
run: poetry run python -m pytest tests/unit -m "not realkernel"
5252

5353
run-unit-tests-with-arrow:
5454
runs-on:
@@ -96,7 +96,7 @@ jobs:
9696
echo "=== Dependency Version: ${{ matrix.dependency-version }} with PyArrow ==="
9797
poetry run pip list
9898
- name: Run tests
99-
run: poetry run python -m pytest tests/unit
99+
run: poetry run python -m pytest tests/unit -m "not realkernel"
100100

101101
run-unit-tests-with-kernel:
102102
runs-on:
@@ -129,17 +129,22 @@ jobs:
129129
run: |
130130
echo "=== with databricks-sql-kernel ==="
131131
poetry run pip list
132-
- name: Verify the kernel extra is installed and importable
132+
- name: Assert the real kernel wheel is installed (not a stub)
133133
run: |
134-
poetry run python -c "import databricks_sql_kernel as k; assert k.__file__, 'kernel wheel missing __file__'; print('kernel ok:', k.__file__)"
135-
- name: Verify use_kernel backend wiring loads against the real wheel
136-
run: |
137-
# Import the kernel-backed client path against the *real* wheel
138-
# (the unit tests use a fake module, so this is the only check
139-
# that the published extra actually loads end to end).
140-
poetry run python -c "from databricks.sql.backend.kernel.client import KernelDatabricksClient; from databricks.sql.backend.kernel.result_set import KernelResultSet; print('use_kernel backend import ok')"
141-
- name: Run tests
142-
run: poetry run python -m pytest tests/unit
134+
poetry run python -c "import databricks_sql_kernel as k; assert k.__file__, 'kernel wheel missing __file__ — not the real wheel'; print('real kernel wheel:', k.__file__)"
135+
- name: Unit tests (kernel wheel present, realkernel deselected)
136+
# The bulk of tests/unit fakes databricks_sql_kernel in
137+
# sys.modules, so the real-wheel routing test is deselected here
138+
# and run on its own below (a shared session would shadow the
139+
# real wheel — both real-wheel tests fail loudly if that happens).
140+
run: poetry run python -m pytest tests/unit -m "not realkernel"
141+
- name: Drive use_kernel=True through the REAL wheel (routing)
142+
# Separate invocation, explicit file path: never collects the
143+
# fake-module test file, so sys.modules stays unpolluted. This is
144+
# the no-network proof that sql.connect(use_kernel=True) actually
145+
# instantiates the real KernelDatabricksClient (not a stub, not a
146+
# Thrift fallback). Fails loudly if the real wheel is shadowed.
147+
run: poetry run python -m pytest tests/unit/test_session.py -m realkernel -v
143148

144149
check-linting:
145150
runs-on:

pyproject.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ exclude = '/(\.eggs|\.git|\.hg|\.mypy_cache|\.nox|\.tox|\.venv|\.svn|_build|buck
100100
[tool.pytest.ini_options]
101101
markers = [
102102
"reviewed: Test case has been reviewed by Databricks",
103-
"serial: Tests that must run serially (not parallelized)"
103+
"serial: Tests that must run serially (not parallelized)",
104+
"realkernel: Requires the real databricks-sql-kernel wheel and an unpolluted sys.modules (no fake kernel stub); must run in a separate pytest invocation from tests that fake databricks_sql_kernel (deselect with -m 'not realkernel', run alone with -m realkernel).",
104105
]
105106
minversion = "6.0"
106107
log_cli = "false"

tests/e2e/test_kernel_backend.py

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
from __future__ import annotations
2323

24+
import sys
2425
from uuid import uuid4
2526

2627
import pytest
@@ -34,24 +35,59 @@
3435
ServerOperationError,
3536
)
3637

37-
# Skip the whole module unless the kernel wheel is genuinely installed.
38-
# ``pytest.importorskip`` alone isn't enough: the kernel unit tests inject a
39-
# fake ``databricks_sql_kernel`` ModuleType into ``sys.modules`` so the
40-
# connector's import-time ``import databricks_sql_kernel`` succeeds without
41-
# the Rust extension. In the same pytest session that fake module is still
42-
# in ``sys.modules`` when this e2e file is collected, and importorskip
43-
# happily returns it. A real wheel exposes ``__file__`` (the compiled
44-
# extension on disk); the fake ModuleType does not.
45-
_kernel_mod = pytest.importorskip(
46-
"databricks_sql_kernel",
47-
reason="use_kernel=True requires the databricks-sql-kernel package",
48-
)
49-
if not getattr(_kernel_mod, "__file__", None):
38+
# These tests must run against the REAL databricks-sql-kernel wheel and
39+
# must NOT silently pass when it's absent or shadowed. We distinguish
40+
# three states explicitly so a misconfigured CI job can't look green:
41+
#
42+
# 1. Wheel genuinely not installed -> legitimate skip.
43+
# 2. Wheel installed in the env but ``sys.modules`` currently holds a
44+
# stub (the kernel UNIT tests inject a fake ``databricks_sql_kernel``
45+
# ModuleType; in a shared ``pytest tests/unit tests/e2e`` session
46+
# that fake can still be resident when this file is collected) ->
47+
# FAIL loudly. Silently skipping here is what made the coverage job
48+
# look like it exercised the kernel when it didn't.
49+
# 3. Wheel installed and importable for real -> run.
50+
#
51+
# "Installed in the env" is decided via importlib.metadata (the dist
52+
# database on disk), which a ``sys.modules`` stub can't fake. The
53+
# ``__file__`` check then tells a real compiled extension from a stub
54+
# ModuleType.
55+
import importlib.metadata as _ilm
56+
57+
try:
58+
_ilm.version("databricks-sql-kernel")
59+
_kernel_installed = True
60+
except _ilm.PackageNotFoundError:
61+
_kernel_installed = False
62+
63+
_kernel_mod = sys.modules.get("databricks_sql_kernel")
64+
if _kernel_mod is None:
65+
try:
66+
import databricks_sql_kernel as _kernel_mod # type: ignore[import-not-found]
67+
except ImportError:
68+
_kernel_mod = None
69+
70+
_kernel_is_real = _kernel_mod is not None and getattr(_kernel_mod, "__file__", None)
71+
72+
if not _kernel_installed:
73+
# State 1: nothing to test against.
5074
pytest.skip(
51-
"databricks_sql_kernel is a test stub (no __file__); "
52-
"install the real wheel to run kernel e2e tests",
75+
"databricks-sql-kernel is not installed; "
76+
"install the real wheel (pip install 'databricks-sql-connector[kernel]') "
77+
"to run kernel e2e tests",
5378
allow_module_level=True,
5479
)
80+
elif not _kernel_is_real:
81+
# State 2: the wheel IS installed but a stub is shadowing it. Do NOT
82+
# skip — that would hide the fact that the kernel path never ran.
83+
raise RuntimeError(
84+
"databricks-sql-kernel is installed in this environment but "
85+
"sys.modules holds a stub (no __file__) — the kernel e2e tests "
86+
"would not actually exercise the real wheel. This usually means a "
87+
"unit test's fake databricks_sql_kernel module is shadowing the "
88+
"real one in a shared pytest session. Run the kernel e2e tests in "
89+
"isolation (separate pytest invocation) so the real wheel loads."
90+
)
5591

5692

5793
@pytest.fixture(scope="module")
@@ -80,9 +116,21 @@ def kernel_conn_params(connection_details):
80116
@pytest.fixture
81117
def conn(kernel_conn_params):
82118
"""One-shot connection per test (the simple_test pattern the
83-
existing e2e suite uses for cursor-level tests)."""
119+
existing e2e suite uses for cursor-level tests).
120+
121+
Asserts the connection actually routed through the kernel backend —
122+
if ``use_kernel=True`` silently fell back to Thrift (e.g. a wiring
123+
regression), these tests must fail rather than pass against the
124+
wrong backend.
125+
"""
126+
from databricks.sql.backend.kernel.client import KernelDatabricksClient
127+
84128
c = sql.connect(**kernel_conn_params)
85129
try:
130+
assert isinstance(c.session.backend, KernelDatabricksClient), (
131+
"use_kernel=True did not route through KernelDatabricksClient; "
132+
f"got {type(c.session.backend).__name__}"
133+
)
86134
yield c
87135
finally:
88136
c.close()

tests/e2e/test_kernel_tls.py

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,25 +30,46 @@
3030
from __future__ import annotations
3131

3232
import os
33+
import sys
3334

3435
import pytest
3536

3637
import databricks.sql as sql
3738
from databricks.sql.exc import Error as DatabricksSqlError
3839

39-
# Same real-wheel guard as test_kernel_backend.py: a fake
40-
# ``databricks_sql_kernel`` ModuleType injected by the unit tests has no
41-
# ``__file__``; only a compiled wheel does.
42-
_kernel_mod = pytest.importorskip(
43-
"databricks_sql_kernel",
44-
reason="use_kernel=True requires the databricks-sql-kernel package",
45-
)
46-
if not getattr(_kernel_mod, "__file__", None):
40+
# Same real-wheel guard as test_kernel_backend.py — see the detailed
41+
# rationale there. Skip only when the wheel is genuinely not installed;
42+
# FAIL LOUDLY if it's installed but shadowed by a stub (so a misconfigured
43+
# shared pytest session can't silently pass as covering the kernel).
44+
import importlib.metadata as _ilm
45+
46+
try:
47+
_ilm.version("databricks-sql-kernel")
48+
_kernel_installed = True
49+
except _ilm.PackageNotFoundError:
50+
_kernel_installed = False
51+
52+
_kernel_mod = sys.modules.get("databricks_sql_kernel")
53+
if _kernel_mod is None:
54+
try:
55+
import databricks_sql_kernel as _kernel_mod # type: ignore[import-not-found]
56+
except ImportError:
57+
_kernel_mod = None
58+
_kernel_is_real = _kernel_mod is not None and getattr(_kernel_mod, "__file__", None)
59+
60+
if not _kernel_installed:
4761
pytest.skip(
48-
"databricks_sql_kernel is a test stub (no __file__); "
49-
"install the real wheel to run kernel TLS e2e tests",
62+
"databricks-sql-kernel is not installed; install the real wheel "
63+
"to run kernel TLS e2e tests",
5064
allow_module_level=True,
5165
)
66+
elif not _kernel_is_real:
67+
raise RuntimeError(
68+
"databricks-sql-kernel is installed but sys.modules holds a stub "
69+
"(no __file__) — the kernel TLS e2e tests would not exercise the "
70+
"real wheel. Run them in isolation (separate pytest invocation) so "
71+
"a unit-test fake module doesn't shadow the real one."
72+
)
5273

5374
_MITM_CA = os.getenv("MITMPROXY_CA_CERT")
5475
if not _MITM_CA:

tests/unit/test_kernel_client.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,21 @@ def __init__(
6161
self.error_details_json = error_details_json
6262

6363

64+
# These unit tests exercise the connector's error-mapping / wiring logic
65+
# and need a *controllable* fake ``KernelError`` (to simulate arbitrary
66+
# kernel error codes), so they install a fake ``databricks_sql_kernel``
67+
# into ``sys.modules`` unconditionally.
68+
#
69+
# IMPORTANT: this fake is session-global and shadows a real wheel if one
70+
# is installed. Tests that need the REAL wheel (the use_kernel routing
71+
# test in test_session.py, and the e2e suite in
72+
# tests/e2e/test_kernel_backend.py) MUST be run in a SEPARATE pytest
73+
# invocation from this file — never `pytest tests/unit tests/e2e` in one
74+
# session when the real wheel is installed. Both of those real-wheel
75+
# tests detect the shadowing (real wheel present but sys.modules holds a
76+
# stub) and FAIL LOUDLY rather than silently skipping, so a CI job that
77+
# accidentally mixes them will go red instead of falsely green. The
78+
# kernel CI matrix runs the real-wheel tests as their own step.
6479
_fake_kernel_module = types.ModuleType("databricks_sql_kernel")
6580
_fake_kernel_module.KernelError = _FakeKernelError # type: ignore[attr-defined]
6681
_fake_kernel_module.Session = MagicMock() # type: ignore[attr-defined]

tests/unit/test_session.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,3 +525,65 @@ def test_user_agent_entry_reaches_kernel_client_http_headers(self):
525525
assert "my-partner-app" in ua, f"UA was {ua!r}"
526526
finally:
527527
conn.close()
528+
529+
530+
@pytest.mark.realkernel
531+
class TestUseKernelRoutesThroughRealWheel:
532+
"""No-network proof that ``sql.connect(use_kernel=True)`` actually
533+
routes through the REAL databricks-sql-kernel wheel — not a stub and
534+
not a fallback to Thrift.
535+
536+
This is the unit-side complement to the live e2e suite: it does not
537+
need a warehouse (only the network boundary ``open_session`` is
538+
mocked), but unlike the other kernel unit tests it does NOT fake the
539+
wheel — the real ``KernelDatabricksClient`` is instantiated and its
540+
``_kernel_session`` is built from the real ``databricks_sql_kernel``
541+
``Session``. Skips only when the real wheel is genuinely absent
542+
(e.g. the no-kernel CI tier); it must never silently pass when the
543+
wheel is present.
544+
"""
545+
546+
def _real_kernel_or_skip(self):
547+
import importlib.metadata as ilm
548+
549+
try:
550+
ilm.version("databricks-sql-kernel")
551+
except ilm.PackageNotFoundError:
552+
pytest.skip("databricks-sql-kernel wheel not installed")
553+
mod = __import__("databricks_sql_kernel")
554+
if not getattr(mod, "__file__", None):
555+
pytest.fail(
556+
"databricks-sql-kernel is installed but sys.modules holds a "
557+
"stub (no __file__) — a unit-test fake is shadowing the real "
558+
"wheel; this routing test would not exercise the real kernel."
559+
)
560+
561+
def test_connect_use_kernel_instantiates_real_kernel_backend(self):
562+
self._real_kernel_or_skip()
563+
564+
from databricks.sql.backend.kernel.client import KernelDatabricksClient
565+
566+
# Mock only the network boundary: the real KernelDatabricksClient
567+
# is constructed (building a real databricks_sql_kernel Session),
568+
# but open_session() doesn't hit the wire.
569+
with patch.object(
570+
KernelDatabricksClient,
571+
"open_session",
572+
return_value=SessionId(BackendType.SEA, "sess-id", None),
573+
):
574+
conn = databricks.sql.connect(
575+
server_hostname="foo.cloud.databricks.com",
576+
http_path="/sql/1.0/warehouses/abc",
577+
use_kernel=True,
578+
access_token="dapi-xyz",
579+
enable_telemetry=False,
580+
)
581+
try:
582+
# The active backend is the REAL kernel client class.
583+
assert isinstance(conn.session.backend, KernelDatabricksClient), (
584+
"use_kernel=True did not route through the real "
585+
f"KernelDatabricksClient; got "
586+
f"{type(conn.session.backend).__name__}"
587+
)
588+
finally:
589+
conn.close()

0 commit comments

Comments
 (0)