Merge pull request #5693 from ian-r-rose/fix-test-races

[WIP] Fix test races

tests/karma.conf.js

@@ -31,8 +31,8 @@ module.exports = function(config) {
       noInfo: true,
       stats: 'errors-only'
     },
-    browserNoActivityTimeout: 31000, // 31 seconds - upped from 10 seconds
-    browserDisconnectTimeout: 31000, // 31 seconds - upped from 2 seconds
+    browserNoActivityTimeout: 61000, // 61 seconds - upped from 10 seconds
+    browserDisconnectTimeout: 61000, // 61 seconds - upped from 2 seconds
     browserDisconnectTolerance: 2,
     port: 9876,
     colors: true,

tests/test-apputils/src/clientsession.spec.ts

@@ -302,22 +302,26 @@ describe('@jupyterlab/apputils', () => {
     });
 
     describe('#restart()', () => {
-      it('should restart if the user accepts the dialog', async () => {
-        let called = false;
-
-        await session.initialize();
-        session.statusChanged.connect((sender, args) => {
-          if (args === 'restarting') {
-            called = true;
-          }
-        });
-
-        const restart = session.restart();
-
-        await acceptDialog();
-        expect(await restart).to.equal(true);
-        expect(called).to.equal(true);
-      });
+      it(
+        'should restart if the user accepts the dialog',
+        async () => {
+          let called = false;
+
+          await session.initialize();
+          session.statusChanged.connect((sender, args) => {
+            if (args === 'restarting') {
+              called = true;
+            }
+          });
+
+          const restart = session.restart();
+
+          await acceptDialog();
+          expect(await restart).to.equal(true);
+          expect(called).to.equal(true);
+        },
+        30000
+      );
 
       it('should not restart if the user rejects the dialog', async () => {
         let called = false;
@@ -368,40 +372,48 @@ describe('@jupyterlab/apputils', () => {
       });
     });
 
-    describe('.restartKernel()', () => {
-      it('should restart if the user accepts the dialog', async () => {
-        let called = false;
-
-        session.statusChanged.connect((sender, args) => {
-          if (args === 'restarting') {
-            called = true;
-          }
-        });
-        await session.initialize();
-
-        const restart = ClientSession.restartKernel(session.kernel);
-
-        await acceptDialog();
-        await restart;
-        expect(called).to.equal(true);
-      });
-
-      it('should not restart if the user rejects the dialog', async () => {
-        let called = false;
-
-        await session.initialize();
-        session.statusChanged.connect((sender, args) => {
-          if (args === 'restarting') {
-            called = true;
-          }
-        });
-
-        const restart = ClientSession.restartKernel(session.kernel);
-
-        await dismissDialog();
-        await restart;
-        expect(called).to.equal(false);
-      });
+    describe('#restartKernel()', () => {
+      it(
+        'should restart if the user accepts the dialog',
+        async () => {
+          let called = false;
+
+          session.statusChanged.connect((sender, args) => {
+            if (args === 'restarting') {
+              called = true;
+            }
+          });
+          await session.initialize();
+
+          const restart = ClientSession.restartKernel(session.kernel);
+
+          await acceptDialog();
+          await restart;
+          expect(called).to.equal(true);
+        },
+        30000
+      ); // Allow for slower CI
+
+      it(
+        'should not restart if the user rejects the dialog',
+        async () => {
+          let called = false;
+
+          await session.initialize();
+          session.statusChanged.connect((sender, args) => {
+            if (args === 'restarting') {
+              called = true;
+            }
+          });
+
+          const restart = ClientSession.restartKernel(session.kernel);
+
+          await dismissDialog();
+          await restart;
+          expect(called).to.equal(false);
+        },
+        30000
+      ); // Allow for slower CI
     });
 
     describe('.getDefaultKernel()', () => {

tests/test-cells/src/widget.spec.ts

@@ -502,40 +502,6 @@ describe('cells/widget', () => {
         expect(widget.node.classList.contains(RENDERED_CLASS)).to.equal(false);
         widget.dispose();
       });
-
-      it('should ignore being set to the same value', async () => {
-        const widget = new LogMarkdownCell({
-          model,
-          rendermime,
-          contentFactory
-        });
-        widget.rendered = false;
-        Widget.attach(widget, document.body);
-
-        // Count how many update requests were processed.
-        await framePromise();
-        const original = widget.methods.filter(method => {
-          return method === 'onUpdateRequest';
-        }).length;
-
-        widget.rendered = false;
-        widget.rendered = false;
-        widget.rendered = false;
-        widget.rendered = false;
-
-        // Count how many update requests were processed
-        await framePromise();
-        const delta =
-          widget.methods.filter(method => {
-            return method === 'onUpdateRequest';
-          }).length - original;
-
-        // Make sure every single `rendered` toggle did not trigger an update.
-        expect(delta)
-          .to.be.gte(0)
-          .and.lte(1);
-        widget.dispose();
-      });
     });
 
     describe('#dispose()', () => {

tests/test-console/src/foreign.spec.ts

@@ -3,7 +3,7 @@
 
 import { expect } from 'chai';
 
-import { UUID } from '@phosphor/coreutils';
+import { PromiseDelegate, UUID } from '@phosphor/coreutils';
 
 import { KernelMessage, Session } from '@jupyterlab/services';
 
@@ -191,17 +191,21 @@ describe('@jupyterlab/console', () => {
     describe('#onIOPubMessage()', () => {
       it('should be called when messages come through', async () => {
         const code = 'print("onIOPubMessage:disabled")';
+        const promise = new PromiseDelegate<void>();
         handler.enabled = false;
         let called = false;
         handler.received.connect(() => {
           called = true;
+          promise.resolve(void 0);
         });
         await foreign.kernel.requestExecute({ code, stop_on_error: true }).done;
+        await promise;
         expect(called).to.equal(true);
       });
 
       it('should inject relevant cells into the parent', async () => {
         const code = 'print("#onIOPubMessage:enabled")';
+        const promise = new PromiseDelegate<void>();
         handler.enabled = true;
         const parent = handler.parent as TestParent;
         expect(parent.widgets.length).to.equal(0);
@@ -209,28 +213,29 @@ describe('@jupyterlab/console', () => {
         handler.injected.connect(() => {
           expect(parent.widgets.length).to.be.greaterThan(0);
           called = true;
+          promise.resolve(void 0);
         });
         await foreign.kernel.requestExecute({ code, stop_on_error: true }).done;
+        await promise;
         expect(called).to.equal(true);
       });
 
       it('should not reject relevant iopub messages', async () => {
         const code = 'print("#onIOPubMessage:relevant")';
+        const promise = new PromiseDelegate<void>();
         let called = false;
         handler.enabled = true;
-        let errored = false;
         handler.rejected.connect(() => {
-          errored = true;
+          promise.reject('rejected relevant iopub message');
         });
         handler.injected.connect((sender, msg) => {
           if (KernelMessage.isStreamMsg(msg)) {
             called = true;
+            promise.resolve(void 0);
           }
         });
         await foreign.kernel.requestExecute({ code, stop_on_error: true }).done;
-        if (errored) {
-          throw new Error('rejected relevant iopub message');
-        }
+        await promise;
         expect(called).to.equal(true);
       });
     });

tests/test-docregistry/src/context.spec.ts

@@ -195,7 +195,6 @@ describe('docregistry/context', () => {
         await context.initialize(true);
         await context.ready;
         expect(context.model.cells.canUndo).to.equal(false);
-        await dismissDialog();
       });
 
       it('should initialize the model when the file is reverted for the first time', async () => {
@@ -295,7 +294,6 @@ describe('docregistry/context', () => {
         const model = await manager.contents.get(context.path, opts);
 
         expect(model.content).to.equal('foo');
-        await dismissDialog();
       });
 
       it('should should preserve LF line endings upon save', async () => {
@@ -364,19 +362,19 @@ describe('docregistry/context', () => {
           const dialog = document.body.getElementsByClassName('jp-Dialog')[0];
           const input = dialog.getElementsByTagName('input')[0];
           input.value = newPath;
-          await acceptDialog();
-          await acceptDialog();
+          await acceptDialog(); // Accept rename dialog
+          await acceptDialog(); // Accept conflict dialog
         };
-        const promise = func();
         await manager.contents.save(newPath, {
           type: factory.contentType,
           format: factory.fileFormat,
           content: 'foo'
         });
         await context.initialize(true);
+        const promise = func();
         await context.saveAs();
-        expect(context.path).to.equal(newPath);
         await promise;
+        expect(context.path).to.equal(newPath);
       });
 
       it('should keep the file if overwrite is aborted', async () => {
@@ -387,20 +385,19 @@ describe('docregistry/context', () => {
           const dialog = document.body.getElementsByClassName('jp-Dialog')[0];
           const input = dialog.getElementsByTagName('input')[0];
           input.value = newPath;
-          await acceptDialog();
-          await dismissDialog();
+          await acceptDialog(); // Accept rename dialog
+          await dismissDialog(); // Reject conflict dialog
         };
-        const promise = func();
         await manager.contents.save(newPath, {
           type: factory.contentType,
           format: factory.fileFormat,
           content: 'foo'
         });
         await context.initialize(true);
+        const promise = func();
         await context.saveAs();
-        expect(context.path).to.equal(oldPath);
         await promise;
-        await dismissDialog();
+        expect(context.path).to.equal(oldPath);
       });
 
       it('should just save if the file name does not change', async () => {
@@ -448,7 +445,7 @@ describe('docregistry/context', () => {
       it('should delete the given checkpoint', async () => {
         await context.initialize(true);
         const model = await context.createCheckpoint();
-        context.deleteCheckpoint(model.id);
+        await context.deleteCheckpoint(model.id);
         const models = await context.listCheckpoints();
         expect(models.length).to.equal(0);
       });

tests/test-notebook/src/actions.spec.ts

@@ -552,7 +552,7 @@ describe('@jupyterlab/notebook', () => {
         expect(result).to.equal(false);
         expect(child.rendered).to.equal(true);
         await ipySession.kernel.restart();
-      }).timeout(30000); // Allow for slower CI
+      }).timeout(60000); // Allow for slower CI
     });
 
     describe('#runAndAdvance()', () => {
@@ -566,7 +566,7 @@ describe('@jupyterlab/notebook', () => {
         expect(result).to.equal(true);
         expect(cell.model.outputs.length).to.be.above(0);
         expect(next.rendered).to.equal(true);
-      });
+      }).timeout(30000); // Allow for slower CI
 
       it('should be a no-op if there is no model', async () => {
         widget.model = null;
@@ -638,7 +638,7 @@ describe('@jupyterlab/notebook', () => {
         expect(cell.rendered).to.equal(true);
         expect(widget.activeCellIndex).to.equal(2);
         await ipySession.kernel.restart();
-      }).timeout(30000); // Allow for slower CI
+      }).timeout(60000); // Allow for slower CI
     });
 
     describe('#runAndInsert()', () => {
@@ -712,7 +712,7 @@ describe('@jupyterlab/notebook', () => {
         expect(cell.rendered).to.equal(true);
         expect(widget.activeCellIndex).to.equal(2);
         await ipySession.kernel.restart();
-      }).timeout(30000); // Allow for slower CI
+      }).timeout(60000); // Allow for slower CI
     });
 
     describe('#runAll()', () => {
@@ -777,7 +777,7 @@ describe('@jupyterlab/notebook', () => {
         expect(result).to.equal(false);
         expect(cell.rendered).to.equal(true);
         await ipySession.kernel.restart();
-      }).timeout(30000); // Allow for slower CI
+      }).timeout(60000); // Allow for slower CI
     });
 
     describe('#selectAbove(`)', () => {

tests/test-notebook/src/widget.spec.ts

@@ -23,7 +23,7 @@ import { INotebookModel, NotebookModel } from '@jupyterlab/notebook';
 
 import { Notebook, StaticNotebook } from '@jupyterlab/notebook';
 
-import { sleep, NBTestUtils } from '@jupyterlab/testutils';
+import { NBTestUtils, framePromise } from '@jupyterlab/testutils';
 
 const contentFactory = NBTestUtils.createNotebookFactory();
 const editorConfig = NBTestUtils.defaultEditorConfig;
@@ -278,7 +278,7 @@ describe('@jupyter/notebook', () => {
         it('should handle changes to the model cell list', async () => {
           widget = createWidget();
           widget.model.cells.clear();
-          await sleep();
+          await framePromise();
           expect(widget.widgets.length).to.equal(1);
         });
 
@@ -626,7 +626,7 @@ describe('@jupyter/notebook', () => {
       it('should post an update request', async () => {
         const widget = createActiveWidget();
         widget.mode = 'edit';
-        await sleep();
+        await framePromise();
         expect(widget.methods).to.contain('onUpdateRequest');
       });
 
@@ -634,7 +634,7 @@ describe('@jupyter/notebook', () => {
         const widget = createActiveWidget();
         widget.model.fromJSON(NBTestUtils.DEFAULT_CONTENT);
         Widget.attach(widget, document.body);
-        await sleep();
+        await framePromise();
         widget.extendContiguousSelectionTo(widget.widgets.length - 1);
         const selectedRange = Array.from(Array(widget.widgets.length).keys());
         expect(selected(widget)).to.deep.equal(selectedRange);
@@ -708,7 +708,7 @@ describe('@jupyter/notebook', () => {
       it('should post an update request', async () => {
         const widget = createActiveWidget();
         widget.model.fromJSON(NBTestUtils.DEFAULT_CONTENT);
-        await sleep();
+        await framePromise();
         expect(widget.methods).to.contain('onUpdateRequest');
         widget.activeCellIndex = 1;
       });
@@ -1081,7 +1081,7 @@ describe('@jupyter/notebook', () => {
         widget = createActiveWidget();
         widget.model.fromJSON(NBTestUtils.DEFAULT_CONTENT);
         Widget.attach(widget, document.body);
-        await sleep();
+        await framePromise();
       });
 
       afterEach(() => {
@@ -1261,7 +1261,7 @@ describe('@jupyter/notebook', () => {
         widget.model.fromJSON(NBTestUtils.DEFAULT_CONTENT);
         Widget.attach(widget, document.body);
         const child = widget.widgets[0];
-        await sleep();
+        await framePromise();
         expect(widget.methods).to.contain('onAfterAttach');
         simulate(widget.node, 'mousedown');
         expect(widget.events).to.contain('mousedown');
@@ -1276,9 +1276,9 @@ describe('@jupyter/notebook', () => {
         const widget = createActiveWidget();
         widget.model.fromJSON(NBTestUtils.DEFAULT_CONTENT);
         Widget.attach(widget, document.body);
-        await sleep();
+        await framePromise();
         expect(widget.methods).to.contain('onAfterAttach');
-        await sleep();
+        await framePromise();
         expect(widget.methods).to.contain('onUpdateRequest');
         widget.dispose();
       });
@@ -1290,7 +1290,7 @@ describe('@jupyter/notebook', () => {
         widget.model.fromJSON(NBTestUtils.DEFAULT_CONTENT);
         Widget.attach(widget, document.body);
         const child = widget.widgets[0];
-        await sleep();
+        await framePromise();
         Widget.detach(widget);
         expect(widget.methods).to.contain('onBeforeDetach');
         widget.events = [];
@@ -1310,7 +1310,7 @@ describe('@jupyter/notebook', () => {
         Widget.attach(widget, document.body);
         MessageLoop.sendMessage(widget, Widget.Msg.ActivateRequest);
         expect(widget.methods).to.contain('onActivateRequest');
-        await sleep();
+        await framePromise();
         expect(document.activeElement).to.equal(widget.node);
         widget.dispose();
       });
@@ -1323,7 +1323,7 @@ describe('@jupyter/notebook', () => {
         widget = createActiveWidget();
         widget.model.fromJSON(NBTestUtils.DEFAULT_CONTENT);
         Widget.attach(widget, document.body);
-        await sleep();
+        await framePromise();
       });
 
       afterEach(() => {
@@ -1337,7 +1337,7 @@ describe('@jupyter/notebook', () => {
 
       it('should apply the edit class if in edit mode', async () => {
         widget.mode = 'edit';
-        await sleep();
+        await framePromise();
         expect(widget.hasClass('jp-mod-editMode')).to.equal(true);
       });
 
@@ -1348,7 +1348,7 @@ describe('@jupyter/notebook', () => {
 
       it('should set the selected class on the selected widgets', async () => {
         widget.select(widget.widgets[1]);
-        await sleep();
+        await framePromise();
         for (let i = 0; i < 2; i++) {
           const cell = widget.widgets[i];
           expect(cell.hasClass('jp-mod-selected')).to.equal(true);
@@ -1358,7 +1358,7 @@ describe('@jupyter/notebook', () => {
       it('should add the multi select class if there is more than one widget', async () => {
         widget.select(widget.widgets[1]);
         expect(widget.hasClass('jp-mod-multSelected')).to.equal(false);
-        await sleep();
+        await framePromise();
         expect(widget.hasClass('jp-mod-multSelected')).to.equal(false);
       });
     });
@@ -1368,7 +1368,7 @@ describe('@jupyter/notebook', () => {
         const widget = createActiveWidget();
         widget.model.fromJSON(NBTestUtils.DEFAULT_CONTENT);
         expect(widget.methods).to.contain('onCellInserted');
-        await sleep();
+        await framePromise();
         expect(widget.methods).to.contain('onUpdateRequest');
       });
 
@@ -1442,7 +1442,7 @@ describe('@jupyter/notebook', () => {
         const cell = widget.model.cells.get(0);
         widget.model.cells.removeValue(cell);
         expect(widget.methods).to.contain('onCellRemoved');
-        await sleep();
+        await framePromise();
         expect(widget.methods).to.contain('onUpdateRequest');
       });
 

tests/test-outputarea/src/widget.spec.ts

@@ -51,6 +51,10 @@ describe('outputarea/widget', () => {
   let widget: LogOutputArea;
   let model: OutputAreaModel;
 
+  beforeAll(async () => {
+    jest.setTimeout(60000);
+  });
+
   beforeEach(() => {
     model = new OutputAreaModel({
       values: NBTestUtils.DEFAULT_OUTPUTS,

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

@@ -24,6 +24,7 @@ describe('Kernel.IKernel', () => {
   let specs: Kernel.ISpecModels;
 
   beforeAll(async () => {
+    jest.setTimeout(60000);
     specs = await Kernel.getSpecs();
   });