Add testing of handleComms option, fix errors.

packages/services/src/kernel/default.ts

@@ -1702,7 +1702,10 @@ namespace Private {
       // them in our clone, since the comm message protocol has implicit
       // assumptions that only one connection is handling comm messages.
       // See https://github.com/jupyter/jupyter_client/issues/263
-      const handleComms = !some(runningKernels, k => k.handleComms);
+      const handleComms = !some(
+        runningKernels,
+        k => k.id === model.id && k.handleComms
+      );
       const newKernel = kernel.clone({ handleComms });
       return newKernel;
     }

tests/test-services/src/kernel/comm.spec.ts

@@ -7,6 +7,8 @@ import { PromiseDelegate } from '@phosphor/coreutils';
 
 import { KernelMessage, Kernel } from '@jupyterlab/services';
 
+import { isFulfilled } from '@jupyterlab/testutils';
+
 import { init } from '../utils';
 
 // Initialize fetch override.
@@ -82,6 +84,15 @@ describe('jupyter.services - Comm', () => {
         const comm2 = kernel.connectToComm('test', '1234');
         expect(comm).to.equal(comm2);
       });
+
+      it('should throw an error when the kernel does not handle comms', async () => {
+        const kernel2 = await Kernel.startNew({
+          name: 'ipython',
+          handleComms: false
+        });
+        expect(kernel2.handleComms).to.be.false;
+        expect(() => kernel2.connectToComm('test', '1234')).to.throw();
+      });
     });
 
     describe('#registerCommTarget()', () => {
@@ -105,6 +116,37 @@ describe('jupyter.services - Comm', () => {
         kernel.removeCommTarget('test', hook);
         comm.dispose();
       });
+
+      it('should do nothing if the kernel does not handle comms', async () => {
+        const promise = new PromiseDelegate<
+          [Kernel.IComm, KernelMessage.ICommOpenMsg]
+        >();
+        const hook = (comm: Kernel.IComm, msg: KernelMessage.ICommOpenMsg) => {
+          promise.resolve([comm, msg]);
+        };
+        const kernel2 = await Kernel.startNew({
+          name: 'ipython',
+          handleComms: false
+        });
+        kernel2.registerCommTarget('test', hook);
+
+        // Request the comm creation.
+        await kernel2.requestExecute({ code: SEND }, true).done;
+
+        // The promise above should not be fulfilled, even after a short delay.
+        expect(await isFulfilled(promise.promise, 300)).to.be.false;
+
+        // The kernel comm has not been closed - we just completely ignored it.
+        let reply = await kernel2.requestExecute(
+          { code: `assert comm._closed is False` },
+          true
+        ).done;
+        // If the assert was false, we would get an 'error' status
+        expect(reply.content.status).to.equal('ok');
+
+        // Clean up
+        kernel2.removeCommTarget('test', hook);
+      });
     });
 
     describe('#commInfo()', () => {

tests/test-services/src/kernel/kernel.spec.ts

@@ -164,6 +164,22 @@ describe('kernel', () => {
       expect(kernel.id).to.equal(id);
       kernel.dispose();
     });
+
+    it('should turn off comm handling in the new connection if it was enabled in first kernel', () => {
+      expect(defaultKernel.handleComms).to.be.true;
+      const kernel = Kernel.connectTo(defaultKernel.model);
+      expect(kernel.handleComms).to.be.false;
+      kernel.dispose();
+    });
+
+    it('should turn on comm handling in the new connection if it was disabled in all other connections', async () => {
+      const kernel = await Kernel.startNew({ handleComms: false });
+      expect(kernel.handleComms).to.be.false;
+      const kernel2 = Kernel.connectTo(defaultKernel.model);
+      expect(kernel2.handleComms).to.be.true;
+      kernel.dispose();
+      kernel2.dispose();
+    });
   });
 
   describe('Kernel.shutdown()', () => {

testutils/src/index.ts

@@ -153,12 +153,22 @@ export function signalToPromise<T, U>(signal: ISignal<T, U>): Promise<[T, U]> {
 /**
  * Test to see if a promise is fulfilled.
  *
+ * @param delay - optional delay in milliseconds before checking
  * @returns true if the promise is fulfilled (either resolved or rejected), and
  * false if the promise is still pending.
  */
-export async function isFulfilled<T>(p: PromiseLike<T>): Promise<boolean> {
+export async function isFulfilled<T>(
+  p: PromiseLike<T>,
+  delay = 0
+): Promise<boolean> {
   let x = Object.create(null);
-  let result = await Promise.race([p, x]).catch(() => false);
+  let race: any;
+  if (delay > 0) {
+    race = sleep(delay, x);
+  } else {
+    race = x;
+  }
+  let result = await Promise.race([p, race]).catch(() => false);
   return result !== x;
 }