Refactor and modernize some tests

Author: acelaya

src/annotator/config/test/config-func-settings-from-test.js

@@ -3,7 +3,7 @@ import { configFuncSettingsFrom } from '../config-func-settings-from';
 describe('annotator.config.configFuncSettingsFrom', () => {
   const sandbox = sinon.createSandbox();
 
-  afterEach('reset the sandbox', () => {
+  afterEach(() => {
     sandbox.restore();
   });
 
@@ -16,7 +16,7 @@ describe('annotator.config.configFuncSettingsFrom', () => {
   });
 
   context("when window.hypothesisConfig() isn't a function", () => {
-    beforeEach('stub console.warn()', () => {
+    beforeEach(() => {
       sandbox.stub(console, 'warn');
     });
 

src/annotator/config/test/index-test.js

@@ -55,7 +55,7 @@ describe('annotator/config/index', () => {
   );
 
   context("when there's no application/annotator+html <link>", () => {
-    beforeEach('remove the application/annotator+html <link>', () => {
+    beforeEach(() => {
       Object.defineProperty(fakeSettingsFrom(), 'sidebarAppUrl', {
         get: sinon.stub().throws(new Error("there's no link")),
       });

src/annotator/config/test/settings-test.js

@@ -85,7 +85,7 @@ describe('annotator/config/settingsFrom', () => {
     context(
       'when the host page has a js-hypothesis-config with an annotations setting',
       () => {
-        beforeEach('add a js-hypothesis-config annotations setting', () => {
+        beforeEach(() => {
           fakeParseJsonConfig.returns({
             annotations: 'annotationsFromJSON',
           });
@@ -101,19 +101,16 @@ describe('annotator/config/settingsFrom', () => {
         context(
           "when there's also an `annotations` in the URL fragment",
           () => {
-            specify(
-              'js-hypothesis-config annotations override URL ones',
-              () => {
-                const window_ = fakeWindow(
-                  'http://localhost:3000#annotations:annotationsFromURL',
-                );
-
-                assert.equal(
-                  settingsFrom(window_).annotations,
-                  'annotationsFromJSON',
-                );
-              },
-            );
+            it('overrides URLs with js-hypothesis-config annotations', () => {
+              const window_ = fakeWindow(
+                'http://localhost:3000#annotations:annotationsFromURL',
+              );
+
+              assert.equal(
+                settingsFrom(window_).annotations,
+                'annotationsFromJSON',
+              );
+            });
           },
         );
       },
@@ -183,7 +180,7 @@ describe('annotator/config/settingsFrom', () => {
     context(
       'when the host page has a js-hypothesis-config with a query setting',
       () => {
-        beforeEach('add a js-hypothesis-config query setting', () => {
+        beforeEach(() => {
           fakeParseJsonConfig.returns({
             query: 'queryFromJSON',
           });
@@ -194,7 +191,7 @@ describe('annotator/config/settingsFrom', () => {
         });
 
         context("when there's also a query in the URL fragment", () => {
-          specify('js-hypothesis-config queries override URL ones', () => {
+          it('overrides URL query with js-hypothesis-config ones', () => {
             const window_ = fakeWindow(
               'http://localhost:3000#annotations:query:queryFromUrl',
             );
@@ -338,54 +335,54 @@ describe('annotator/config/settingsFrom', () => {
     [
       {
         when: 'the client is embedded in a web page',
-        specify: 'it returns setting values from window.hypothesisConfig()',
+        title: 'returns setting values from window.hypothesisConfig()',
         configFuncSettings: { foo: 'configFuncValue' },
         jsonSettings: { foo: 'ignored' }, // hypothesisConfig() overrides js-hypothesis-config
         expected: 'configFuncValue',
       },
       {
         when: 'the client is embedded in a web page',
-        specify:
-          'it ignores settings from js-hypothesis-config if `ignoreOtherConfiguration` is present',
+        title:
+          'ignores settings from js-hypothesis-config if `ignoreOtherConfiguration` is present',
         isBrowserExtension: false,
         configFuncSettings: { ignoreOtherConfiguration: '1' },
         jsonSettings: { foo: 'ignored' },
         expected: undefined,
       },
       {
         when: 'the client is embedded in a web page',
-        specify: 'it returns setting values from js-hypothesis-config objects',
+        title: 'returns setting values from js-hypothesis-config objects',
         configFuncSettings: {},
         jsonSettings: { foo: 'jsonValue' },
         expected: 'jsonValue',
       },
       {
         when: 'the client is embedded in a web page',
-        specify:
-          'hypothesisConfig() settings override js-hypothesis-config ones',
+        title:
+          'overrides js-hypothesis-config with hypothesisConfig() settings',
         configFuncSettings: { foo: 'configFuncValue' },
         jsonSettings: { foo: 'jsonValue' },
         expected: 'configFuncValue',
       },
       {
         when: 'the client is embedded in a web page',
-        specify:
-          'even a null from hypothesisConfig() overrides js-hypothesis-config',
+        title:
+          'overrides js-hypothesis-config even with null from hypothesisConfig()',
         configFuncSettings: { foo: null },
         jsonSettings: { foo: 'jsonValue' },
         expected: null,
       },
       {
         when: 'the client is embedded in a web page',
-        specify:
-          'even an undefined from hypothesisConfig() overrides js-hypothesis-config',
+        title:
+          'overrides js-hypothesis-config even with undefined from hypothesisConfig()',
         configFuncSettings: { foo: undefined },
         jsonSettings: { foo: 'jsonValue' },
         expected: undefined,
       },
     ].forEach(test => {
       context(test.when, () => {
-        specify(test.specify, () => {
+        it(test.title, () => {
           fakeConfigFuncSettingsFrom.returns(test.configFuncSettings);
           fakeParseJsonConfig.returns(test.jsonSettings);
           const settings = settingsFrom(fakeWindow());

src/boot/test/parse-json-config-test.js

@@ -29,7 +29,7 @@ describe('#parseJsonConfig', () => {
   });
 
   context("when there's JSON scripts with no top-level objects", () => {
-    beforeEach('add JSON scripts with no top-level objects', () => {
+    beforeEach(() => {
       appendJSHypothesisConfig(document, 'null');
       appendJSHypothesisConfig(document, '23');
       appendJSHypothesisConfig(document, 'true');
@@ -41,7 +41,7 @@ describe('#parseJsonConfig', () => {
   });
 
   context("when there's a JSON script with a top-level array", () => {
-    beforeEach('add a JSON script containing a top-level array', () => {
+    beforeEach(() => {
       appendJSHypothesisConfig(document, '["a", "b", "c"]');
     });
 
@@ -51,7 +51,7 @@ describe('#parseJsonConfig', () => {
   });
 
   context("when there's a JSON script with a top-level string", () => {
-    beforeEach('add a JSON script with a top-level string', () => {
+    beforeEach(() => {
       appendJSHypothesisConfig(document, '"hi"');
     });
 
@@ -61,11 +61,11 @@ describe('#parseJsonConfig', () => {
   });
 
   context("when there's a JSON script containing invalid JSON", () => {
-    beforeEach('stub console.warn()', () => {
+    beforeEach(() => {
       sandbox.stub(console, 'warn');
     });
 
-    beforeEach('add a JSON script containing invalid JSON', () => {
+    beforeEach(() => {
       appendJSHypothesisConfig(document, 'this is not valid json');
     });
 
@@ -87,7 +87,7 @@ describe('#parseJsonConfig', () => {
   });
 
   context("when there's a JSON script with an empty object", () => {
-    beforeEach('add a JSON script containing an empty object', () => {
+    beforeEach(() => {
       appendJSHypothesisConfig(document, '{}');
     });
 
@@ -97,7 +97,7 @@ describe('#parseJsonConfig', () => {
   });
 
   context("when there's a JSON script containing some settings", () => {
-    beforeEach('add a JSON script containing some settings', () => {
+    beforeEach(() => {
       appendJSHypothesisConfig(document, '{"foo": "FOO", "bar": "BAR"}');
     });
 
@@ -107,7 +107,7 @@ describe('#parseJsonConfig', () => {
   });
 
   context('when there are JSON scripts with different settings', () => {
-    beforeEach('add some JSON scripts with different settings', () => {
+    beforeEach(() => {
       appendJSHypothesisConfig(document, '{"foo": "FOO"}');
       appendJSHypothesisConfig(document, '{"bar": "BAR"}');
       appendJSHypothesisConfig(document, '{"gar": "GAR"}');
@@ -123,17 +123,14 @@ describe('#parseJsonConfig', () => {
   });
 
   context('when multiple JSON scripts contain the same setting', () => {
-    beforeEach('add some JSON scripts with different settings', () => {
+    beforeEach(() => {
       appendJSHypothesisConfig(document, '{"foo": "first"}');
       appendJSHypothesisConfig(document, '{"foo": "second"}');
       appendJSHypothesisConfig(document, '{"foo": "third"}');
     });
 
-    specify(
-      'settings from later in the page override ones from earlier',
-      () => {
-        assert.equal(parseJsonConfig(document).foo, 'third');
-      },
-    );
+    it('overrides settings from earlier in the page with later ones', () => {
+      assert.equal(parseJsonConfig(document).foo, 'third');
+    });
   });
 });

src/shared/messaging/test/port-rpc-test.js

@@ -1,3 +1,4 @@
+import { promiseWithResolvers } from '../../promise-with-resolvers';
 import { PortRPC, installPortCloseWorkaroundForSafari } from '../port-rpc';
 
 describe('PortRPC', () => {
@@ -79,11 +80,12 @@ describe('PortRPC', () => {
     sinon.restore();
   });
 
-  it('should call the method `plusOne` on rpc2', done => {
-    rpc1.call('plusOne', 1, 2, 3, value => {
-      assert.deepEqual(value, [2, 3, 4]);
-      done();
-    });
+  it('should call the method `plusOne` on rpc2', async () => {
+    const { promise, resolve } = promiseWithResolvers();
+    rpc1.call('plusOne', 1, 2, 3, resolve);
+
+    const value = await promise;
+    assert.deepEqual(value, [2, 3, 4]);
   });
 
   it('should not call the method `plusOne` if rpc1 is destroyed', () => {
@@ -102,15 +104,18 @@ describe('PortRPC', () => {
     assert.notCalled(plusOne);
   });
 
-  it('should call the method `concat` on rpc1', done => {
-    rpc2.call('concat', 'hello', ' ', 'world', value => {
-      assert.equal(value, 'hello world');
-    });
-
-    rpc2.call('concat', [1], [2], [3], value => {
-      assert.deepEqual(value, [1, 2, 3]);
-      done();
-    });
+  it('should call the method `concat` on rpc1', async () => {
+    const { promise: promiseOne, resolve: resolvePromiseOne } =
+      promiseWithResolvers();
+    rpc2.call('concat', 'hello', ' ', 'world', resolvePromiseOne);
+    const valueOne = await promiseOne;
+    assert.equal(valueOne, 'hello world');
+
+    const { promise: promiseTwo, resolve: resolvePromiseTwo } =
+      promiseWithResolvers();
+    rpc2.call('concat', [1], [2], [3], resolvePromiseTwo);
+    const valueTwo = await promiseTwo;
+    assert.deepEqual(valueTwo, [1, 2, 3]);
   });
 
   it('should call method on valid message', async () => {

src/shared/test/cfi-test.js

@@ -41,7 +41,9 @@ describe('sidebar/util/cfi', () => {
         expected: ['/2/4', '/2/4'],
       },
     ].forEach(({ range, expected }) => {
-      assert.deepEqual(splitCFIRange(range), expected);
+      it('splitCFIRange', () => {
+        assert.deepEqual(splitCFIRange(range), expected);
+      });
     });
   });
 

src/sidebar/components/test/HypothesisApp-test.js

@@ -343,7 +343,7 @@ describe('HypothesisApp', () => {
     });
 
     context('when a third-party service is in use', () => {
-      beforeEach('configure a third-party service to be in use', () => {
+      beforeEach(() => {
         fakeServiceConfig.returns({});
       });
 

src/sidebar/components/test/ModerationBanner-test.js

@@ -1,6 +1,7 @@
 import {
   checkAccessibility,
   mockImportedComponents,
+  waitFor,
 } from '@hypothesis/frontend-testing';
 import { mount } from '@hypothesis/frontend-testing';
 
@@ -121,7 +122,7 @@ describe('ModerationBanner', () => {
     assert.calledWith(fakeApi.annotation.hide, { id: 'ann-id' });
   });
 
-  it('reports an error if hiding the annotation fails', done => {
+  it('reports an error if hiding the annotation fails', async () => {
     const wrapper = createComponent({
       annotation: moderatedAnnotation({
         flagCount: 10,
@@ -130,10 +131,9 @@ describe('ModerationBanner', () => {
     fakeApi.annotation.hide.returns(Promise.reject(new Error('Network Error')));
     wrapper.find('button').simulate('click');
 
-    setTimeout(() => {
-      assert.calledWith(fakeToastMessenger.error, 'Failed to hide annotation');
-      done();
-    }, 0);
+    await waitFor(() =>
+      fakeToastMessenger.error.calledWith('Failed to hide annotation'),
+    );
   });
 
   it('unhides the annotation if "Unhide" is clicked', () => {
@@ -147,7 +147,7 @@ describe('ModerationBanner', () => {
     assert.calledWith(fakeApi.annotation.unhide, { id: 'ann-id' });
   });
 
-  it('reports an error if unhiding the annotation fails', done => {
+  it('reports an error if unhiding the annotation fails', async () => {
     const wrapper = createComponent({
       annotation: moderatedAnnotation({
         flagCount: 1,
@@ -158,13 +158,10 @@ describe('ModerationBanner', () => {
       Promise.reject(new Error('Network Error')),
     );
     wrapper.find('button').simulate('click');
-    setTimeout(() => {
-      assert.calledWith(
-        fakeToastMessenger.error,
-        'Failed to unhide annotation',
-      );
-      done();
-    }, 0);
+
+    await waitFor(() =>
+      fakeToastMessenger.error.calledWith('Failed to unhide annotation'),
+    );
   });
 
   it(

src/sidebar/services/test/auth-test.js

@@ -136,7 +136,7 @@ describe('AuthService', () => {
 
     context('when the access token request fails', () => {
       const expectedErr = new Error('Grant token exchange failed');
-      beforeEach('make access token requests fail', () => {
+      beforeEach(() => {
         fakeClient.exchangeGrantToken.rejects(expectedErr);
       });
 
@@ -291,7 +291,7 @@ describe('AuthService', () => {
     });
 
     context('when a refresh request fails', () => {
-      beforeEach('make refresh token requests fail', () => {
+      beforeEach(() => {
         fakeClient.refreshToken.returns(Promise.reject(new Error('failed')));
         fakeClient.exchangeGrantToken.returns(successfulTokenResponse);
       });