Fix comms and add qtconsole downstream test (#1056)

Fixes #1059

.github/workflows/downstream.yml

@@ -84,6 +84,41 @@ jobs:
           pip install -e ".[test]"
           python test_ipykernel.py
 
+  qtconsole:
+    runs-on: ubuntu-latest
+    timeout-minutes: 20
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v3
+
+      - name: Setup Python
+        uses: actions/setup-python@v4
+        with:
+          python-version: "3.9"
+          architecture: "x64"
+
+      - name: Install System Packages
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y --no-install-recommends '^libxcb.*-dev' libx11-xcb-dev libglu1-mesa-dev libxrender-dev libxi-dev libxkbcommon-dev libxkbcommon-x11-dev
+      - name: Install qtconsole dependencies
+        shell: bash -l {0}
+        run: |
+          cd ${GITHUB_WORKSPACE}/..
+          git clone https://github.com/jupyter/qtconsole.git
+          cd qtconsole
+          ${pythonLocation}/bin/python -m pip install -e ".[test]"
+          ${pythonLocation}/bin/python -m pip install pyqt5
+      - name: Install Jupyter-Client changes
+        shell: bash -l {0}
+        run: ${pythonLocation}/bin/python -m pip install -e .
+
+      - name: Test qtconsole
+        shell: bash -l {0}
+        run: |
+          cd ${GITHUB_WORKSPACE}/../qtconsole
+          xvfb-run --auto-servernum ${pythonLocation}/bin/python -m pytest -x -vv -s --full-trace --color=yes qtconsole
+
   downstream_check: # This job does nothing and is only used for the branch protection
     if: always()
     needs:
@@ -92,6 +127,7 @@ jobs:
       - jupyter_client
       - ipyparallel
       - jupyter_kernel_test
+      - qtconsole
     runs-on: ubuntu-latest
     steps:
       - name: Decide whether the needed jobs succeeded or failed

ipykernel/comm/comm.py

@@ -42,7 +42,7 @@ def publish_msg(self, msg_type, data=None, metadata=None, buffers=None, **keys):
 
 
 # but for backwards compatibility, we need to inherit from LoggingConfigurable
-class Comm(traitlets.config.LoggingConfigurable, BaseComm):
+class Comm(BaseComm, traitlets.config.LoggingConfigurable):
     """Class for communicating between a Frontend and a Kernel"""
 
     kernel = Instance("ipykernel.kernelbase.Kernel", allow_none=True)  # type:ignore[assignment]
@@ -68,12 +68,16 @@ def _default_kernel(self):
     def _default_comm_id(self):
         return uuid.uuid4().hex
 
-    def __init__(self, *args, **kwargs):
-        # Comm takes positional arguments, LoggingConfigurable does not, so we explicitly forward arguments
+    def __init__(self, target_name='', data=None, metadata=None, buffers=None, **kwargs):
+        # Handle differing arguments between base classes.
+        kernel = kwargs.pop('kernel', None)
+        if target_name:
+            kwargs['target_name'] = target_name
+        BaseComm.__init__(
+            self, data=data, metadata=metadata, buffers=buffers, **kwargs
+        )  # type:ignore[call-arg]
+        kwargs['kernel'] = kernel
         traitlets.config.LoggingConfigurable.__init__(self, **kwargs)
-        # drop arguments not in BaseComm
-        kwargs.pop("kernel", None)
-        BaseComm.__init__(self, *args, **kwargs)
 
 
 __all__ = ["Comm"]

ipykernel/comm/manager.py

@@ -9,13 +9,13 @@
 import traitlets.config
 
 
-class CommManager(traitlets.config.LoggingConfigurable, comm.base_comm.CommManager):
+class CommManager(comm.base_comm.CommManager, traitlets.config.LoggingConfigurable):
 
     kernel = traitlets.Instance("ipykernel.kernelbase.Kernel")
     comms = traitlets.Dict()
     targets = traitlets.Dict()
 
     def __init__(self, **kwargs):
         # CommManager doesn't take arguments, so we explicitly forward arguments
-        traitlets.config.LoggingConfigurable.__init__(self, **kwargs)
         comm.base_comm.CommManager.__init__(self)
+        traitlets.config.LoggingConfigurable.__init__(self, **kwargs)

ipykernel/tests/test_comm.py

@@ -1,3 +1,5 @@
+import unittest.mock
+
 from ipykernel.comm import Comm, CommManager
 from ipykernel.ipkernel import IPythonKernel
 
@@ -47,10 +49,12 @@ def on_msg(msg):
     manager.register_target("fizz", fizz)
 
     kernel.comm_manager = manager
-    comm = Comm()
-    comm.on_msg(on_msg)
-    comm.on_close(on_close)
-    manager.register_comm(comm)
+    with unittest.mock.patch.object(Comm, "publish_msg") as publish_msg:
+        comm = Comm()
+        comm.on_msg(on_msg)
+        comm.on_close(on_close)
+        manager.register_comm(comm)
+        assert publish_msg.call_count == 1
 
     assert manager.get_comm(comm.comm_id) == comm
     assert manager.get_comm('foo') is None