Merge pull request #1061 from hypothesis/mockable-imports

Introduce a new approach to mocking modules in tests

package.json

@@ -20,6 +20,7 @@
     "aws-sdk": "^2.345.0",
     "babel-plugin-angularjs-annotate": "^0.10.0",
     "babel-plugin-istanbul": "^5.1.0",
+    "babel-plugin-mockable-imports": "^1.1.0",
     "babel-plugin-transform-async-to-promises": "^0.8.6",
     "babel-preset-env": "^1.7.0",
     "babelify": "^10.0.0",

src/karma.config.js

@@ -86,6 +86,7 @@ module.exports = function(config) {
             // code coverage instrumentation for Istanbul.
             extensions: ['.js', '.coffee'],
             plugins: [
+              'mockable-imports',
               [
                 'babel-plugin-istanbul',
                 {

src/sidebar/components/test/group-list-item-out-of-scope-test.js

 'use strict';
 
 const { mount } = require('enzyme');
-const preact = require('preact');
 const { createElement } = require('preact');
-const proxyquire = require('proxyquire');
 
 const { events } = require('../../services/analytics');
+const GroupListItemOutOfScope = require('../group-list-item-out-of-scope');
 
 describe('GroupListItemOutOfScope', () => {
   let fakeAnalytics;
   let fakeGroupListItemCommon;
-  let GroupListItemOutOfScope;
 
   const fakeGroup = {
     id: 'groupid',
@@ -28,27 +26,23 @@ describe('GroupListItemOutOfScope', () => {
       .first()
       .simulate('click');
 
-  before(() => {
+  beforeEach(() => {
+    fakeAnalytics = {
+      track: sinon.stub(),
+      events,
+    };
     fakeGroupListItemCommon = {
       orgName: sinon.stub(),
       trackViewGroupActivity: sinon.stub(),
     };
 
-    GroupListItemOutOfScope = proxyquire('../group-list-item-out-of-scope', {
-      // Use same instance of Preact module in tests and mocked module.
-      // See https://robertknight.me.uk/posts/browserify-dependency-mocking/
-      preact,
-
+    GroupListItemOutOfScope.$imports.$mock({
       '../util/group-list-item-common': fakeGroupListItemCommon,
-      '@noCallThru': true,
     });
   });
 
-  beforeEach(() => {
-    fakeAnalytics = {
-      track: sinon.stub(),
-      events,
-    };
+  afterEach(() => {
+    GroupListItemOutOfScope.$imports.$restore();
   });
 
   const createGroupListItemOutOfScope = fakeGroup => {

src/sidebar/components/test/group-list-item-test.js

@@ -2,7 +2,7 @@
 
 const { createElement } = require('preact');
 const { mount } = require('enzyme');
-const proxyquire = require('proxyquire');
+const GroupListItem = require('../group-list-item');
 
 const { events } = require('../../services/analytics');
 
@@ -10,19 +10,6 @@ describe('GroupListItem', () => {
   let fakeAnalytics;
   let fakeStore;
   let fakeGroupListItemCommon;
-  let GroupListItem;
-
-  before(() => {
-    fakeGroupListItemCommon = {
-      orgName: sinon.stub(),
-      trackViewGroupActivity: sinon.stub(),
-    };
-
-    GroupListItem = proxyquire('../group-list-item', {
-      '../util/group-list-item-common': fakeGroupListItemCommon,
-      '@noCallThru': true,
-    });
-  });
 
   beforeEach(() => {
     fakeStore = {
@@ -34,6 +21,19 @@ describe('GroupListItem', () => {
       track: sinon.stub(),
       events,
     };
+
+    fakeGroupListItemCommon = {
+      orgName: sinon.stub(),
+      trackViewGroupActivity: sinon.stub(),
+    };
+
+    GroupListItem.$imports.$mock({
+      '../util/group-list-item-common': fakeGroupListItemCommon,
+    });
+  });
+
+  afterEach(() => {
+    GroupListItem.$imports.$restore();
   });
 
   const createGroupListItem = fakeGroup => {

src/sidebar/components/test/hypothesis-app-test.js

 'use strict';
 
 const angular = require('angular');
-const proxyquire = require('proxyquire');
 
 const events = require('../../events');
 const bridgeEvents = require('../../../shared/bridge-events');
-const util = require('../../../shared/test/util');
+
+const hypothesisApp = require('../hypothesis-app');
 
 describe('sidebar.components.hypothesis-app', function() {
   let $componentController = null;
   let $scope = null;
   let $rootScope = null;
-  let fakeAnnotationMetadata = null;
   let fakeStore = null;
   let fakeAnalytics = null;
   let fakeAuth = null;
@@ -44,24 +43,17 @@ describe('sidebar.components.hypothesis-app', function() {
   });
 
   beforeEach(function() {
-    fakeAnnotationMetadata = {
-      location: function() {
-        return 0;
-      },
-    };
-
     fakeServiceConfig = sandbox.stub();
 
-    const component = proxyquire(
-      '../hypothesis-app',
-      util.noCallThru({
-        angular: angular,
-        '../annotation-metadata': fakeAnnotationMetadata,
-        '../service-config': fakeServiceConfig,
-      })
-    );
+    hypothesisApp.$imports.$mock({
+      '../service-config': fakeServiceConfig,
+    });
+
+    angular.module('h', []).component('hypothesisApp', hypothesisApp);
+  });
 
-    angular.module('h', []).component('hypothesisApp', component);
+  afterEach(() => {
+    hypothesisApp.$imports.$restore();
   });
 
   beforeEach(angular.mock.module('h'));

src/sidebar/components/test/login-control-test.js

 'use strict';
 
 const angular = require('angular');
-const proxyquire = require('proxyquire');
 
 const bridgeEvents = require('../../../shared/bridge-events');
 const util = require('../../directive/test/util');
 
+const loginControl = require('../login-control');
+
 function pageObject(element) {
   return {
     menuLinks: function() {
@@ -79,19 +80,15 @@ function thirdPartyUserPage() {
 
 describe('loginControl', function() {
   let fakeBridge;
-  const fakeServiceConfig = sinon.stub();
+  let fakeServiceConfig;
   let fakeWindow;
 
   before(function() {
-    angular.module('app', []).component(
-      'loginControl',
-      proxyquire('../login-control', {
-        '../service-config': fakeServiceConfig,
-      })
-    );
+    angular.module('app', []).component('loginControl', loginControl);
   });
 
   beforeEach(function() {
+    fakeServiceConfig = sinon.stub().returns(null);
     fakeBridge = { call: sinon.stub() };
     const fakeServiceUrl = sinon.stub().returns('someUrl');
     const fakeSettings = {
@@ -106,8 +103,13 @@ describe('loginControl', function() {
       $window: fakeWindow,
     });
 
-    fakeServiceConfig.reset();
-    fakeServiceConfig.returns(null);
+    loginControl.$imports.$mock({
+      '../service-config': fakeServiceConfig,
+    });
+  });
+
+  afterEach(() => {
+    loginControl.$imports.$restore();
   });
 
   describe('the user profile button', function() {

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

 'use strict';
 
 const angular = require('angular');
-const proxyquire = require('proxyquire');
 
 const util = require('../../directive/test/util');
-const noCallThru = require('../../../shared/test/util').noCallThru;
+const markdown = require('../markdown');
 
 describe('markdown', function() {
   function isHidden(element) {
@@ -40,46 +39,39 @@ describe('markdown', function() {
   }
 
   before(function() {
-    angular.module('app').component(
-      'markdown',
-      proxyquire(
-        '../markdown',
-        noCallThru({
-          angular: angular,
-          katex: {
-            renderToString: function(input) {
-              return 'math:' + input.replace(/$$/g, '');
-            },
-          },
-          'lodash.debounce': function(fn) {
-            // Make input change debouncing synchronous in tests
-            return function() {
-              fn();
-            };
-          },
-          '../render-markdown': noCallThru(function(markdown) {
-            return 'rendered:' + markdown;
-          }),
-
-          '../markdown-commands': {
-            convertSelectionToLink: mockFormattingCommand,
-            toggleBlockStyle: mockFormattingCommand,
-            toggleSpanStyle: mockFormattingCommand,
-            LinkType: require('../../markdown-commands').LinkType,
-          },
-          '../media-embedder': noCallThru({
-            replaceLinksWithEmbeds: function(element) {
-              // Tag the element as having been processed
-              element.dataset.replacedLinksWithEmbeds = 'yes';
-            },
-          }),
-        })
-      )
-    );
+    angular.module('app', []).component('markdown', markdown);
   });
 
   beforeEach(function() {
     angular.mock.module('app');
+
+    markdown.$imports.$mock({
+      'lodash.debounce': function(fn) {
+        // Make input change debouncing synchronous in tests
+        return function() {
+          fn();
+        };
+      },
+      '../render-markdown': markdown => {
+        return 'rendered:' + markdown;
+      },
+      '../markdown-commands': {
+        convertSelectionToLink: mockFormattingCommand,
+        toggleBlockStyle: mockFormattingCommand,
+        toggleSpanStyle: mockFormattingCommand,
+        LinkType: require('../../markdown-commands').LinkType,
+      },
+      '../media-embedder': {
+        replaceLinksWithEmbeds: function(element) {
+          // Tag the element as having been processed
+          element.dataset.replacedLinksWithEmbeds = 'yes';
+        },
+      },
+    });
+  });
+
+  afterEach(() => {
+    markdown.$imports.$restore();
   });
 
   describe('read only state', function() {

src/sidebar/components/test/top-bar-test.js

 'use strict';
 
 const angular = require('angular');
-const proxyquire = require('proxyquire');
 
+const topBar = require('../top-bar');
 const util = require('../../directive/test/util');
 
 describe('topBar', function() {
   const fakeSettings = {};
-  const fakeIsThirdPartyService = sinon.stub();
+  let fakeIsThirdPartyService;
 
   before(function() {
     angular
       .module('app', [])
-      .component(
-        'topBar',
-        proxyquire('../top-bar', {
-          '../util/is-third-party-service': fakeIsThirdPartyService,
-          '@noCallThru': true,
-        })
-      )
+      .component('topBar', topBar)
       .component('loginControl', {
         bindings: require('../login-control').bindings,
       })
@@ -35,8 +29,15 @@ describe('topBar', function() {
       settings: fakeSettings,
     });
 
-    fakeIsThirdPartyService.reset();
-    fakeIsThirdPartyService.returns(false);
+    fakeIsThirdPartyService = sinon.stub().returns(false);
+
+    topBar.$imports.$mock({
+      '../util/is-third-party-service': fakeIsThirdPartyService,
+    });
+  });
+
+  afterEach(() => {
+    topBar.$imports.$restore();
   });
 
   function applyUpdateBtn(el) {

src/sidebar/services/test/service-url-test.js

 'use strict';
 
-const proxyquire = require('proxyquire');
+const serviceUrlFactory = require('../service-url');
 
 /** Return a fake store object. */
 function fakeStore() {
@@ -20,7 +20,7 @@ function createServiceUrl(linksPromise) {
     .stub()
     .returns({ url: 'EXPANDED_URL', params: {} });
 
-  const serviceUrlFactory = proxyquire('../service-url', {
+  serviceUrlFactory.$imports.$mock({
     '../util/url-util': { replaceURLParams: replaceURLParams },
   });
 
@@ -45,6 +45,7 @@ describe('sidebar.service-url', function() {
 
   afterEach(function() {
     console.warn.restore();
+    serviceUrlFactory.$imports.$restore();
   });
 
   context('before the API response has been received', function() {

src/sidebar/test/raven-test.js

 'use strict';
 
-const proxyquire = require('proxyquire');
-const noCallThru = require('../../shared/test/util').noCallThru;
+const raven = require('../raven');
 
 function fakeExceptionData(scriptURL) {
   return {
@@ -28,7 +27,6 @@ describe('raven', function() {
   let fakeAngularTransformer;
   let fakeAngularPlugin;
   let fakeRavenJS;
-  let raven;
 
   beforeEach(function() {
     fakeRavenJS = {
@@ -52,13 +50,14 @@ describe('raven', function() {
       Raven.setDataCallback(fakeAngularTransformer);
     });
 
-    raven = proxyquire(
-      '../raven',
-      noCallThru({
-        'raven-js': fakeRavenJS,
-        'raven-js/plugins/angular': fakeAngularPlugin,
-      })
-    );
+    raven.$imports.$mock({
+      'raven-js': fakeRavenJS,
+      'raven-js/plugins/angular': fakeAngularPlugin,
+    });
+  });
+
+  afterEach(() => {
+    raven.$imports.$restore();
   });
 
   describe('.install()', function() {

src/sidebar/test/render-markdown-test.js

 'use strict';
 
-const proxyquire = require('proxyquire');
+const renderMarkdown = require('../render-markdown');
 
 describe('render-markdown', function() {
   let render;
-  let renderMarkdown;
 
   beforeEach(function() {
-    renderMarkdown = proxyquire('../render-markdown', {
+    renderMarkdown.$imports.$mock({
       katex: {
         renderToString: function(input, opts) {
           if (opts && opts.displayMode) {
@@ -23,6 +22,10 @@ describe('render-markdown', function() {
     };
   });
 
+  afterEach(() => {
+    renderMarkdown.$imports.$restore();
+  });
+
   describe('autolinking', function() {
     it('should autolink URLs', function() {
       assert.equal(

src/sidebar/test/virtual-thread-list-test.js

 'use strict';
 
-const proxyquire = require('proxyquire');
-
-const VirtualThreadList = proxyquire('../virtual-thread-list', {
-  'lodash.debounce': function(fn) {
-    // Make debounced functions execute immediately
-    return fn;
-  },
-});
 const util = require('../../shared/test/util');
+const VirtualThreadList = require('../virtual-thread-list');
+
 const unroll = util.unroll;
 
 describe('VirtualThreadList', function() {
@@ -45,6 +39,16 @@ describe('VirtualThreadList', function() {
     };
   }
 
+  beforeEach(() => {
+    VirtualThreadList.$imports.$mock({
+      'lodash.debounce': fn => fn,
+    });
+  });
+
+  afterEach(() => {
+    VirtualThreadList.$imports.$restore();
+  });
+
   beforeEach(function() {
     fakeScope = { $digest: sinon.stub() };
 

src/sidebar/util/test/fetch-config-test.js

 'use strict';
 
-const proxyquire = require('proxyquire');
-
 const {
   assertPromiseIsRejected,
 } = require('../../../shared/test/promise-util');
 
+const { fetchConfig, $imports } = require('../fetch-config');
+
 describe('sidebar.util.fetch-config', () => {
-  let fetchConfig;
   let fakeHostConfig;
   let fakeJsonRpc;
   let fakeWindow;
@@ -17,11 +16,10 @@ describe('sidebar.util.fetch-config', () => {
     fakeJsonRpc = {
       call: sinon.stub(),
     };
-    const patched = proxyquire('../fetch-config', {
+    $imports.$mock({
       '../host-config': fakeHostConfig,
       './postmessage-json-rpc': fakeJsonRpc,
     });
-    fetchConfig = patched.fetchConfig;
 
     // By default, embedder provides no custom config.
     fakeHostConfig.returns({});
@@ -39,6 +37,10 @@ describe('sidebar.util.fetch-config', () => {
     fakeWindow = { parent: fakeParent, top: fakeTopWindow };
   });
 
+  afterEach(() => {
+    $imports.$restore();
+  });
+
   describe('fetchConfig', () => {
     // By default, combine the settings rendered into the sidebar's HTML page
     // by h with the settings from `window.hypothesisConfig` in the parent

src/sidebar/util/test/is-third-party-service-test.js

 'use strict';
 
-const proxyquire = require('proxyquire');
+const isThirdPartyService = require('../is-third-party-service');
 
 describe('sidebar.util.isThirdPartyService', () => {
   let fakeServiceConfig;
   let fakeSettings;
-  let isThirdPartyService;
 
   beforeEach(() => {
     fakeServiceConfig = sinon.stub();
     fakeSettings = { authDomain: 'hypothes.is' };
 
-    isThirdPartyService = proxyquire('../is-third-party-service', {
+    isThirdPartyService.$imports.$mock({
       '../service-config': fakeServiceConfig,
       '@noCallThru': true,
     });
   });
 
+  afterEach(() => {
+    isThirdPartyService.$imports.$restore();
+  });
+
   it('returns false for first-party services', () => {
     fakeServiceConfig.returns({ authority: 'hypothes.is' });
 

yarn.lock

@@ -1549,6 +1549,11 @@ babel-plugin-istanbul@^5.1.0:
     istanbul-lib-instrument "^3.2.0"
     test-exclude "^5.2.2"
 
+babel-plugin-mockable-imports@^1.1.0:
+  version "1.2.0"
+  resolved "https://registry.yarnpkg.com/babel-plugin-mockable-imports/-/babel-plugin-mockable-imports-1.2.0.tgz#151db640cf6339a6a878a3b70e336eec4baa1895"
+  integrity sha512-KqKIFsPDQcXf2XZZazAxGa7xUgbGueLfCQb4mD7G16lvLYfz13SakC6IAcRaBueXwNn2O0K6VGlXZzPs0aLL3Q==
+
 babel-plugin-syntax-async-functions@^6.8.0:
   version "6.13.0"
   resolved "https://registry.yarnpkg.com/babel-plugin-syntax-async-functions/-/babel-plugin-syntax-async-functions-6.13.0.tgz#cad9cad1191b5ad634bf30ae0872391e0647be95"