Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce a new approach to mocking modules in tests #1061

Merged
merged 7 commits into from
Apr 23, 2019
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/karma.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ module.exports = function(config) {
// code coverage instrumentation for Istanbul.
extensions: ['.js', '.coffee'],
plugins: [
'mockable-imports',
[
'babel-plugin-istanbul',
{
Expand Down
24 changes: 9 additions & 15 deletions src/sidebar/components/test/group-list-item-out-of-scope-test.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
'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',
Expand All @@ -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();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for cleaning up the mock objects between tests.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs note that Jest would do this automatically for us. This test runner looks pretty nice. Is that something you think we might move to in the future?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. As I understand it, Jest currently runs only under Node and not in a real browser. That is OK for many web apps where a fake browser environment like JSDOM is good enough for running tests. For the client I'm not sure if it will be adequate.

});

const createGroupListItemOutOfScope = fakeGroup => {
Expand Down
28 changes: 14 additions & 14 deletions src/sidebar/components/test/group-list-item-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,14 @@

const { createElement } = require('preact');
const { mount } = require('enzyme');
const proxyquire = require('proxyquire');
const GroupListItem = require('../group-list-item');

const { events } = require('../../services/analytics');

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 = {
Expand All @@ -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 => {
Expand Down
28 changes: 10 additions & 18 deletions src/sidebar/components/test/hypothesis-app-test.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
'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;
Expand Down Expand Up @@ -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'));
Expand Down
22 changes: 12 additions & 10 deletions src/sidebar/components/test/login-control-test.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
'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() {
Expand Down Expand Up @@ -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 = {
Expand All @@ -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() {
Expand Down
68 changes: 30 additions & 38 deletions src/sidebar/components/test/markdown-test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
'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) {
Expand Down Expand Up @@ -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() {
Expand Down
23 changes: 12 additions & 11 deletions src/sidebar/components/test/top-bar-test.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,18 @@
'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,
})
Expand All @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions src/sidebar/services/test/service-url-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const proxyquire = require('proxyquire');
const serviceUrlFactory = require('../service-url');

/** Return a fake store object. */
function fakeStore() {
Expand All @@ -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 },
});

Expand All @@ -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() {
Expand Down
Loading