Fix stale, hanging, and unrun ggsql-jupyter protocol tests - #557
Draft
samclark2015 wants to merge 1 commit into
Draft
samclark2015 wants to merge 1 commit into
samclark2015 wants to merge 1 commit into
Conversation
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 (#556), unrelated to this suite, which will make the job flaky until that's fixed separately. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This branch has not been deployed
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.
Summary
Fixes posit-dev/positron#15512: the Python protocol tests under
ggsql-jupyter/tests/had been stale and unrun (not wired into CI) since ~January 2026. This PR fixes the suite and wires it into CI.test_compliance.py: fixed a staleimplementationstring mismatch, a pager assertion that could never pass, and a kernelspec install that could clobber a developer's realggsqlkernelspec (now isolated to a scratchJUPYTER_DATA_DIRunder aggsql-testname). Also fixed two independent hangs —test_shutdownwas killing the class-shared kernel beforetest_status_messagesran (alphabetical test ordering), andtest_kernel_infowaited forever on a reply it explicitly filters out viaget_non_kernel_info_reply— plus a third bug wheretest_shutdownread its reply from the wrong ZMQ channel (control, not shell). Also collapsed the per-testcargo buildinsetUpdown to once per module.test_integration.py: replaced a blindsleep(3)with a real kernel-readiness check (14/14 tests pass; runtime dropped from ~211s to ~36s). Fixed a bug this exposed in howjupyter_client'sis_alive()was being used, given this test launches the kernel process manually rather than throughKernelManager.start_kernel().requirements.txt: droppedjupyterlab/ipykernel, unused by either test file.jupyter-protocol-testsjob to.github/workflows/build.yaml. It runs withcontinue-on-error: truefor now — see below.ggsql-jupyter/tests/README.mdandggsql-jupyter/CLAUDE.mdto match.A separate, pre-existing bug found along the way
While verifying these fixes, kernel startup intermittently panicked (crashed the whole kernel process) from a bug in the vendored
zeromqcrate'sRepSocket, used forggsql-jupyter's heartbeat socket — unrelated to anything in this PR. Filed as posit-dev/ggsql#556. It hit ~3 of 4 local runs, so the new CI job hascontinue-on-error: trueuntil #556 is fixed, to avoid a flaky-red job on every PR.Test plan
test_integration.py: 14/14 passing locallytest_compliance.py: passing locally when kernel startup doesn't hit ggsql-jupyter's heartbeat socket can panic the whole kernel (zeromq RepSocket, not yet implemented) #556's panic; confirmed the realggsqlkernelspec is never touched across multiple runsggsql-jupyter/src/kernel.rsthatshutdown_replyis sent on the control channel, matching thetest_shutdownfixjupyter-protocol-testsCI job goes green (or at least runs, givencontinue-on-error) on this PR🤖 Generated with Claude Code