From 3a25d9c37c55bea922f2ef85a348ee7194d3995c Mon Sep 17 00:00:00 2001 From: Sam Clark Date: Mon, 21 Sep 2026 15:59:00 -0500 Subject: [PATCH 1/2] Fix stale, hanging, and unrun ggsql-jupyter protocol tests test_compliance.py and test_integration.py under ggsql-jupyter/tests/ had gone unrun and untested since ~January 2026 (posit-dev/positron#15512): a stale implementation string, a pager assertion that could never pass, a kernelspec install that could clobber a developer's real "ggsql" kernel, and two independent hangs (test_shutdown killing the class-shared kernel before test_status_messages runs alphabetically, and test_kernel_info waiting forever on a reply it explicitly filters out) meant the suite never ran to completion. test_shutdown also read its reply from the wrong ZMQ channel. test_integration.py's kernel_manager fixture used a blind sleep(3) instead of a real readiness check. Fixes all of the above, isolates the kernelspec install to a scratch JUPYTER_DATA_DIR, drops unused test dependencies, and wires both files into a new CI job. That job runs with continue-on-error for now: kernel startup intermittently panics on a heartbeat-socket bug in the vendored zeromq crate (posit-dev/ggsql#556), unrelated to this suite, which will make the job flaky until that's fixed separately. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/build.yaml | 53 +++++++++++ ggsql-jupyter/CLAUDE.md | 2 +- ggsql-jupyter/tests/README.md | 6 +- ggsql-jupyter/tests/requirements.txt | 4 - ggsql-jupyter/tests/test_compliance.py | 113 ++++++++++++++++-------- ggsql-jupyter/tests/test_integration.py | 28 ++++-- 6 files changed, 154 insertions(+), 52 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 4b26b1c33..68c78aa31 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -147,3 +147,56 @@ jobs: - name: Build WASM package run: ./ggsql-wasm/build-wasm.sh --skip-opt + + jupyter-protocol-tests: + # The Python suite under ggsql-jupyter/tests/ that drives the kernel over + # a real ZMQ connection — the one thing the Rust unit tests above cannot + # reach (wire framing, HMAC signing, busy/idle ordering). See + # ggsql-jupyter/CLAUDE.md, "Testing". + runs-on: ubuntu-latest + # continue-on-error until posit-dev/ggsql#556 (a heartbeat-socket panic + # in the vendored zeromq crate, unrelated to this suite) is fixed: that + # bug crashes the kernel process in a large fraction of runs, which + # would otherwise make this job flaky-red rather than a real signal. + # Remove this once #556 lands. + continue-on-error: true + + steps: + - name: Check out repository + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: "22" + + - name: Install tree-sitter-cli + # parser.c is generated from grammar.js by the tree-sitter-ggsql + # build script and is not committed, so this is needed on every + # fresh checkout — see /CLAUDE.md, "Building". + run: npm install -g tree-sitter-cli + + - name: Install Rust + uses: dtolnay/rust-toolchain@stable + + - name: Caching + uses: Swatinem/rust-cache@v2 + with: + shared-key: ${{ runner.os }}-build + cache-on-failure: true + save-if: ${{ github.ref == 'refs/heads/main' }} + + - name: Build ggsql-jupyter + run: cargo build --bin ggsql-jupyter + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install Python test dependencies + run: pip install -r ggsql-jupyter/tests/requirements.txt + + - name: Run ggsql-jupyter protocol tests + working-directory: ggsql-jupyter/tests + run: pytest test_integration.py test_compliance.py -v diff --git a/ggsql-jupyter/CLAUDE.md b/ggsql-jupyter/CLAUDE.md index 7a099b221..c770aef65 100644 --- a/ggsql-jupyter/CLAUDE.md +++ b/ggsql-jupyter/CLAUDE.md @@ -215,7 +215,7 @@ pip install -r requirements.txt pytest ``` -`test_compliance.py` verifies handler coverage (`execute_request`, `kernel_info_request`, `is_complete_request`, `shutdown_request`); `test_integration.py` drives a real kernel via `jupyter_client`. +`test_compliance.py` verifies handler coverage (`execute_request`, `kernel_info_request`, `is_complete_request`, `shutdown_request`); `test_integration.py` drives a real kernel via `jupyter_client`. Both run in CI (`jupyter-protocol-tests` job in `/.github/workflows/build.yaml`). ## See also diff --git a/ggsql-jupyter/tests/README.md b/ggsql-jupyter/tests/README.md index 7729a6963..6990a68fb 100644 --- a/ggsql-jupyter/tests/README.md +++ b/ggsql-jupyter/tests/README.md @@ -70,6 +70,8 @@ Run official Jupyter kernel compliance tests: ```bash # From ggsql-jupyter/tests/ directory pytest test_compliance.py -v - -# Note: This will install the kernel spec temporarily ``` + +`test_compliance.py` installs its kernelspec under a scratch `JUPYTER_DATA_DIR` +it creates and tears down itself, using the name `ggsql-test` — running it +never touches a real `ggsql` kernelspec you may have installed. diff --git a/ggsql-jupyter/tests/requirements.txt b/ggsql-jupyter/tests/requirements.txt index b12d1037f..a1eb560d5 100644 --- a/ggsql-jupyter/tests/requirements.txt +++ b/ggsql-jupyter/tests/requirements.txt @@ -3,7 +3,3 @@ jupyter-client>=8.0.0 jupyter-kernel-test>=0.7.0 pytest>=7.0.0 pytest-asyncio>=0.21.0 - -# For manual testing and debugging -jupyterlab>=4.0.0 -ipykernel>=6.0.0 diff --git a/ggsql-jupyter/tests/test_compliance.py b/ggsql-jupyter/tests/test_compliance.py index e5e48b1bc..2cd50f515 100644 --- a/ggsql-jupyter/tests/test_compliance.py +++ b/ggsql-jupyter/tests/test_compliance.py @@ -5,17 +5,22 @@ messaging protocol correctly according to the specification. """ +import os import unittest import jupyter_kernel_test as jkt import subprocess from pathlib import Path +# Isolated from any real Jupyter install: setup_module points JUPYTER_DATA_DIR +# at a scratch directory before this name is ever installed or removed. +KERNEL_NAME = "ggsql-test" + class ggsqlKernelTests(jkt.KernelTests): """Compliance tests for ggsql-jupyter kernel.""" # Kernel name (will be overridden to use custom command) - kernel_name = "ggsql" + kernel_name = KERNEL_NAME # Language name language_name = "ggsql" @@ -26,11 +31,6 @@ class ggsqlKernelTests(jkt.KernelTests): # Code samples for testing code_hello_world = "SELECT 'Hello, World!' as greeting" - # Expected output pattern (for simple SELECT) - # Note: jupyter_kernel_test looks for this in text/plain output - # We may need to adjust based on actual output format - code_page_something = "SELECT 'something' as result" - # Override test_execute_stdout - SQL kernels don't produce stdout def test_execute_stdout(self): """SQL kernels produce execute_result, not stdout streams.""" @@ -38,35 +38,28 @@ def test_execute_stdout(self): # They produce execute_result messages instead pass - def setUp(self): - """Build kernel before tests.""" - # Build the kernel - repo_root = Path(__file__).parent.parent.parent - result = subprocess.run( - ["cargo", "build", "--bin", "ggsql-jupyter"], - cwd=repo_root / "ggsql-jupyter", - capture_output=True, - text=True, - ) - if result.returncode != 0: - self.fail(f"Failed to build kernel: {result.stderr}") - - super().setUp() - # Test that kernel_info_request works def test_kernel_info(self): - """Test kernel_info_request returns correct information.""" + """Test kernel_info_request returns correct information. + + `get_non_kernel_info_reply` (jkt's own `execute_helper` uses it to + skip past an unsolicited reply and get to the one actually being + waited on) would hang here forever: it explicitly discards + `kernel_info_reply` messages, but that is the only reply this + request ever produces. `get_shell_msg` with a bounded timeout is + what jkt's own base `test_kernel_info` uses for the same request. + """ self.flush_channels() msg_id = self.kc.kernel_info() - reply = self.get_non_kernel_info_reply() + reply = self.kc.get_shell_msg(timeout=jkt.TIMEOUT) self.assertEqual(reply["msg_type"], "kernel_info_reply") content = reply["content"] self.assertEqual(content["status"], "ok") self.assertEqual(content["protocol_version"], "5.3") - self.assertEqual(content["implementation"], "ggsql") + self.assertEqual(content["implementation"], "ggsql-jupyter") # Language info lang_info = content["language_info"] @@ -202,15 +195,32 @@ def test_execute_input(self): # Test shutdown def test_shutdown(self): - """Test that shutdown works.""" - self.flush_channels() + """Test that shutdown works. + + `setUpClass`/`tearDownClass` own one kernel shared by every test + method in this class, so shutting *that* one down here would leave + nothing for whatever test runs next (unittest orders methods + alphabetically, so this would otherwise run before + `test_status_messages`). Start a throwaway kernel instead. + + `kc.shutdown()` sends `shutdown_request` on the *control* channel + (see `KernelClient.shutdown`'s docstring), and the reply comes back + on the same channel — not shell, despite the original version of + this test waiting on `get_shell_msg` and timing out here every time. + """ + from jupyter_client.manager import start_new_kernel - msg_id = self.kc.shutdown() - reply = self.kc.get_shell_msg(timeout=5) + km, kc = start_new_kernel(kernel_name=self.kernel_name) + try: + msg_id = kc.shutdown() + reply = kc.get_control_msg(timeout=5) - self.assertEqual(reply["msg_type"], "shutdown_reply") - self.assertEqual(reply["content"]["status"], "ok") - self.assertIn("restart", reply["content"]) + self.assertEqual(reply["msg_type"], "shutdown_reply") + self.assertEqual(reply["content"]["status"], "ok") + self.assertIn("restart", reply["content"]) + finally: + kc.stop_channels() + km.shutdown_kernel() # Test persistent state def test_persistent_state(self): @@ -233,14 +243,29 @@ def test_persistent_state(self): self.assertEqual(reply3["content"]["status"], "ok") +# Restored in teardown_module. Set before install so the kernelspec below +# never touches a developer's real Jupyter data directory. +_original_jupyter_data_dir = None +_scratch_data_dir = None + + # Configure kernel for testing def setup_module(): - """Setup module by installing kernel spec.""" + """Build the kernel once and install it into an isolated kernelspec.""" import tempfile import json - import os - # Build kernel + global _original_jupyter_data_dir, _scratch_data_dir + + # Isolate JUPYTER_DATA_DIR before anything below can install or remove a + # kernelspec, so this suite can never clobber a developer's real "ggsql" + # kernel — nothing in the environment has to know to set this itself. + _original_jupyter_data_dir = os.environ.get("JUPYTER_DATA_DIR") + _scratch_data_dir = tempfile.mkdtemp(prefix="ggsql-jupyter-data-") + os.environ["JUPYTER_DATA_DIR"] = _scratch_data_dir + + # Build kernel (once for the whole module; individual tests no longer + # rebuild it in setUp). repo_root = Path(__file__).parent.parent.parent result = subprocess.run( ["cargo", "build", "--bin", "ggsql-jupyter"], @@ -259,7 +284,7 @@ def setup_module(): # Create kernel spec kernel_spec = { "argv": [str(binary_path), "-f", "{connection_file}"], - "display_name": "ggsql", + "display_name": KERNEL_NAME, "language": "ggsql", } @@ -268,7 +293,7 @@ def setup_module(): with open(spec_dir / "kernel.json", "w") as f: json.dump(kernel_spec, f) - # Install kernel spec + # Install kernel spec (into the scratch JUPYTER_DATA_DIR set above) result = subprocess.run( [ "jupyter", @@ -276,7 +301,7 @@ def setup_module(): "install", "--user", "--name", - "ggsql", + KERNEL_NAME, str(spec_dir), ], capture_output=True, @@ -289,12 +314,22 @@ def setup_module(): def teardown_module(): - """Cleanup kernel spec after tests.""" + """Cleanup kernel spec after tests and restore JUPYTER_DATA_DIR.""" subprocess.run( - ["jupyter", "kernelspec", "remove", "-f", "ggsql"], + ["jupyter", "kernelspec", "remove", "-f", KERNEL_NAME], capture_output=True, ) + if _original_jupyter_data_dir is None: + os.environ.pop("JUPYTER_DATA_DIR", None) + else: + os.environ["JUPYTER_DATA_DIR"] = _original_jupyter_data_dir + + if _scratch_data_dir is not None: + import shutil + + shutil.rmtree(_scratch_data_dir, ignore_errors=True) + if __name__ == "__main__": # Run setup diff --git a/ggsql-jupyter/tests/test_integration.py b/ggsql-jupyter/tests/test_integration.py index 4bee05069..1fc998b47 100644 --- a/ggsql-jupyter/tests/test_integration.py +++ b/ggsql-jupyter/tests/test_integration.py @@ -44,6 +44,13 @@ def _launch(kernel_binary, extra_args=()): try: # Use KernelManager to write connection file with proper ports km = KernelManager() + # We launch and kill the process ourselves below (km.start_kernel() + # is never called), so km's own liveness bookkeeping never gets set + # up: km.is_alive() would otherwise unconditionally report the + # kernel dead, since has_kernel is False. Telling it we don't own + # the kernel process makes wait_for_ready() rely purely on getting a + # real kernel_info_reply within its timeout, which is what we want. + km._owns_kernel = False km.write_connection_file() connection_file = km.connection_file @@ -57,15 +64,24 @@ def _launch(kernel_binary, extra_args=()): # Store the process in km for cleanup km._kernel_process = kernel_process - # Wait for kernel to be ready - time.sleep(3) - - # Check if process started successfully - if kernel_process.poll() is not None: - stdout, stderr = kernel_process.communicate() + # Wait for the kernel to actually be ready (heartbeat up) rather than + # guessing with a fixed sleep. Opened and closed here rather than + # left open, since callers that want a client build their own via + # km.client() (see the `client`/`console_client` fixtures below). + probe = km.client() + probe.start_channels() + try: + probe.wait_for_ready(timeout=10) + except RuntimeError: + probe.stop_channels() + stdout, stderr = (b"", b"") + if kernel_process.poll() is not None: + stdout, stderr = kernel_process.communicate() pytest.fail( f"Kernel failed to start:\nstdout: {stdout.decode()}\nstderr: {stderr.decode()}" ) + else: + probe.stop_channels() yield km From 2c77927917c34a407bdc74d9f357f1553501989f Mon Sep 17 00:00:00 2001 From: Sam Clark Date: Wed, 23 Sep 2026 12:22:31 -0500 Subject: [PATCH 2/2] Session-scope the integration kernel, activate real compliance tests, build the kernel once test_integration.py launched a fresh kernel per test; kernel_manager and console_kernel_manager are now session-scoped, and TestShutdown gets its own private kernel (via a new _launch_kernel context manager) instead of killing the shared one, since that's the one thing every other test in the file still needs. Its shutdown reply is also now read from the control channel, where it actually arrives, rather than shell. test_compliance.py inherited from jupyter_kernel_test.KernelTests without defining any of the sample attributes that drive its built-in tests, so most of them silently skipped. Checked kernel.rs's message dispatch to see what's real: complete_request, inspect_request, history_request and pager support don't exist, and display_data is never emitted, so those tests are now explicitly overridden with a comment explaining why. code_generate_error, code_execute_result, complete_code_samples and incomplete_code_samples are backed by real behavior, so they're now defined and those tests actually run. Both files built the kernel independently; that's now conftest.py's build_kernel_binary(), shared by both. build.yaml's protocol-test step sets GGSQL_SKIP_GENERATE=1 since the grammar was already generated by the build step just before it. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/build.yaml | 5 ++ ggsql-jupyter/tests/conftest.py | 38 +++++++++++ ggsql-jupyter/tests/test_compliance.py | 61 ++++++++++++----- ggsql-jupyter/tests/test_integration.py | 88 +++++++++++-------------- 4 files changed, 125 insertions(+), 67 deletions(-) create mode 100644 ggsql-jupyter/tests/conftest.py diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 68c78aa31..0e3b2ba13 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -199,4 +199,9 @@ jobs: - name: Run ggsql-jupyter protocol tests working-directory: ggsql-jupyter/tests + # The grammar was already generated by the "Build ggsql-jupyter" step + # above; the fixtures' own `cargo build` calls (see conftest.py) are + # just up-to-date checks and don't need tree-sitter-cli again. + env: + GGSQL_SKIP_GENERATE: "1" run: pytest test_integration.py test_compliance.py -v diff --git a/ggsql-jupyter/tests/conftest.py b/ggsql-jupyter/tests/conftest.py new file mode 100644 index 000000000..b1d4865f9 --- /dev/null +++ b/ggsql-jupyter/tests/conftest.py @@ -0,0 +1,38 @@ +"""Shared fixtures for the ggsql-jupyter protocol test suite. + +`test_integration.py` (pytest) and `test_compliance.py` (unittest-style +setup_module) both need the kernel binary, so the build lives here once +rather than duplicated in each file. CI's own build step already generates +the grammar and compiles the binary before pytest runs; calling this again +from a fixture is a cheap up-to-date check, not a rebuild. +""" + +import subprocess +from pathlib import Path + +import pytest + + +def build_kernel_binary() -> str: + """Build ggsql-jupyter and return the path to its binary.""" + repo_root = Path(__file__).parent.parent.parent + result = subprocess.run( + ["cargo", "build", "--bin", "ggsql-jupyter"], + cwd=repo_root / "ggsql-jupyter", + capture_output=True, + text=True, + ) + if result.returncode != 0: + raise RuntimeError(f"Failed to build kernel: {result.stderr}") + + binary_path = repo_root / "target" / "debug" / "ggsql-jupyter" + if not binary_path.exists(): + raise RuntimeError(f"Kernel binary not found at {binary_path}") + + return str(binary_path) + + +@pytest.fixture(scope="session") +def kernel_binary() -> str: + """Build and return path to ggsql-jupyter binary, once per session.""" + return build_kernel_binary() diff --git a/ggsql-jupyter/tests/test_compliance.py b/ggsql-jupyter/tests/test_compliance.py index 2cd50f515..982cb840f 100644 --- a/ggsql-jupyter/tests/test_compliance.py +++ b/ggsql-jupyter/tests/test_compliance.py @@ -11,6 +11,8 @@ import subprocess from pathlib import Path +from conftest import build_kernel_binary + # Isolated from any real Jupyter install: setup_module points JUPYTER_DATA_DIR # at a scratch directory before this name is ever installed or removed. KERNEL_NAME = "ggsql-test" @@ -31,6 +33,14 @@ class ggsqlKernelTests(jkt.KernelTests): # Code samples for testing code_hello_world = "SELECT 'Hello, World!' as greeting" + # These have a real implementation behind them, so defining the sample + # lets jupyter_kernel_test's own inherited test exercise it directly + # rather than duplicating the assertions in a test we wrote ourselves. + code_generate_error = "SELECT * FROM nonexistent_table" + code_execute_result = [{"code": "SELECT 123 as num", "mime": "text/plain"}] + complete_code_samples = ["SELECT 1"] + incomplete_code_samples = ["SELECT (1"] + # Override test_execute_stdout - SQL kernels don't produce stdout def test_execute_stdout(self): """SQL kernels produce execute_result, not stdout streams.""" @@ -38,6 +48,38 @@ def test_execute_stdout(self): # They produce execute_result messages instead pass + # Everything below has no backing implementation in the kernel: there is + # no complete_request, inspect_request or history_request handler (see + # kernel.rs's message dispatch), `payload` is hardcoded to `[]` so there + # is no pager support, and nothing is ever emitted as `display_data` — + # results always go out as `execute_result`. Defining the sample + # attributes that would make these inherited tests run would exercise + # protocol features this kernel doesn't have, so they're overridden here + # to record that as a deliberate choice rather than a silent SkipTest. + def test_execute_stderr(self): + """No stream messages of any kind are ever emitted.""" + pass + + def test_completion(self): + """No complete_request handler exists.""" + pass + + def test_pager(self): + """`payload` is hardcoded to `[]`; there is no pager support.""" + pass + + def test_display_data(self): + """Results always go out as execute_result, never display_data.""" + pass + + def test_history(self): + """No history_request handler exists.""" + pass + + def test_inspect(self): + """No inspect_request handler exists.""" + pass + # Test that kernel_info_request works def test_kernel_info(self): """Test kernel_info_request returns correct information. @@ -265,25 +307,12 @@ def setup_module(): os.environ["JUPYTER_DATA_DIR"] = _scratch_data_dir # Build kernel (once for the whole module; individual tests no longer - # rebuild it in setUp). - repo_root = Path(__file__).parent.parent.parent - result = subprocess.run( - ["cargo", "build", "--bin", "ggsql-jupyter"], - cwd=repo_root / "ggsql-jupyter", - capture_output=True, - text=True, - ) - if result.returncode != 0: - raise RuntimeError(f"Failed to build kernel: {result.stderr}") - - # Find binary - binary_path = repo_root / "target" / "debug" / "ggsql-jupyter" - if not binary_path.exists(): - raise RuntimeError(f"Kernel binary not found at {binary_path}") + # rebuild it in setUp). Shared with test_integration.py via conftest.py. + binary_path = build_kernel_binary() # Create kernel spec kernel_spec = { - "argv": [str(binary_path), "-f", "{connection_file}"], + "argv": [binary_path, "-f", "{connection_file}"], "display_name": KERNEL_NAME, "language": "ggsql", } diff --git a/ggsql-jupyter/tests/test_integration.py b/ggsql-jupyter/tests/test_integration.py index 1fc998b47..a409ec156 100644 --- a/ggsql-jupyter/tests/test_integration.py +++ b/ggsql-jupyter/tests/test_integration.py @@ -5,36 +5,16 @@ to verify correct behavior. """ +import contextlib import json import time import subprocess -import tempfile import os -from pathlib import Path import pytest from jupyter_client import KernelManager - -@pytest.fixture(scope="session") -def kernel_binary(): - """Build and return path to ggsql-jupyter binary.""" - # Build the kernel - repo_root = Path(__file__).parent.parent.parent - result = subprocess.run( - ["cargo", "build", "--bin", "ggsql-jupyter"], - cwd=repo_root / "ggsql-jupyter", - capture_output=True, - text=True, - ) - if result.returncode != 0: - pytest.fail(f"Failed to build kernel: {result.stderr}") - - # Find binary - binary_path = repo_root / "target" / "debug" / "ggsql-jupyter" - if not binary_path.exists(): - pytest.fail(f"Kernel binary not found at {binary_path}") - - return str(binary_path) +# kernel_binary is defined in conftest.py, shared with test_compliance.py so +# the kernel is built once rather than once per test file. def _launch(kernel_binary, extra_args=()): @@ -103,15 +83,26 @@ def _launch(kernel_binary, extra_args=()): pass -@pytest.fixture +# Also usable as `with _launch_kernel(binary) as km:` outside of pytest's +# fixture machinery — see TestShutdown.test_shutdown_request below, which +# needs a kernel of its own rather than the session-scoped one. +_launch_kernel = contextlib.contextmanager(_launch) + + +@pytest.fixture(scope="session") def kernel_manager(kernel_binary): - """Create and start a kernel manager.""" + """Start one kernel manager, shared by every test in this session. + + Session-scoped so the suite doesn't pay kernel startup cost per test. + Any test that needs to shut down or otherwise damage its kernel — see + TestShutdown — must launch a private one instead of using this fixture. + """ yield from _launch(kernel_binary) -@pytest.fixture +@pytest.fixture(scope="session") def console_kernel_manager(kernel_binary): - """A kernel that believes it is a Positron console session. + """A session-scoped kernel that believes it is a Positron console session. `--session-mode` is what the extension's `createKernelSpec` appends, and it is authoritative over the session-id heuristic — so this is the only way to @@ -545,37 +536,32 @@ def test_status_busy_idle(self, client): class TestShutdown: """Test shutdown_request/reply messages.""" - def test_shutdown_request(self, kernel_manager): - """Test that kernel responds to shutdown_request.""" - kc = kernel_manager.client() - kc.start_channels() - - try: - kc.wait_for_ready(timeout=10) + def test_shutdown_request(self, kernel_binary): + """Test that kernel responds to shutdown_request. - # Send shutdown request on control channel - msg_id = kc.shutdown() + Launches its own kernel rather than taking the session-scoped + `kernel_manager` fixture: this test's whole point is to shut its + kernel down, which would take every other test in this module down + with it if it were the shared one. - # Try to get reply with a longer timeout + The reply comes back on the *control* channel — `kc.shutdown()` + sends `shutdown_request` there, per `KernelClient.shutdown`'s + docstring — not shell. + """ + with _launch_kernel(kernel_binary) as km: + kc = km.client() + kc.start_channels() try: - reply = kc.get_shell_msg(timeout=10) + kc.wait_for_ready(timeout=10) + + msg_id = kc.shutdown() + reply = kc.get_control_msg(timeout=10) assert reply["msg_type"] == "shutdown_reply" assert reply["content"]["status"] == "ok" assert "restart" in reply["content"] - except: - # If we can't get the reply, at least verify the kernel process is terminating - # Wait a bit for shutdown to process - time.sleep(2) - kernel_process = kernel_manager._kernel_process - # Process should either be terminated or terminating - if kernel_process.poll() is None: - # Still running, send explicit shutdown - kernel_process.terminate() - kernel_process.wait(timeout=5) - - finally: - kc.stop_channels() + finally: + kc.stop_channels() class TestExecuteInput: