From a2168efcdafa424dca45c2bd1e6322a4776cfcdd Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 7 Apr 2016 16:47:17 +0100 Subject: [PATCH 1/3] Correctly restore MatrixClientPeg after each test case --- test/components/structures/MatrixChat-test.js | 15 ++++++++++----- test/components/structures/MessagePanel-test.js | 7 +++++++ test/test-utils.js | 15 +++++++++++++-- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/test/components/structures/MatrixChat-test.js b/test/components/structures/MatrixChat-test.js index 7d96b3a77a..68bc85dd27 100644 --- a/test/components/structures/MatrixChat-test.js +++ b/test/components/structures/MatrixChat-test.js @@ -3,16 +3,21 @@ var TestUtils = require('react-addons-test-utils'); var expect = require('expect'); var sdk = require('matrix-react-sdk'); +var MatrixChat = sdk.getComponent('structures.MatrixChat'); +var peg = require('../../../src/MatrixClientPeg'); var test_utils = require('../../test-utils'); -var peg = require('../../../src/MatrixClientPeg.js'); var q = require('q'); describe('MatrixChat', function () { - var MatrixChat; - before(function() { - test_utils.stubClient(); - MatrixChat = sdk.getComponent('structures.MatrixChat'); + var sandbox; + + beforeEach(function() { + sandbox = test_utils.stubClient(); + }); + + afterEach(function() { + sandbox.restore(); }); it('gives a login panel by default', function () { diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js index 9a4996b5da..134cc4c34b 100644 --- a/test/components/structures/MessagePanel-test.js +++ b/test/components/structures/MessagePanel-test.js @@ -27,12 +27,19 @@ var test_utils = require('test-utils'); var mockclock = require('mock-clock'); describe('MessagePanel', function () { + var sandbox; var clock = mockclock.clock(); var realSetTimeout = window.setTimeout; var events = mkEvents(); + beforeEach(function() { + test_utils.beforeEach(this); + sandbox = test_utils.stubClient(sandbox); + }); + afterEach(function () { clock.uninstall(); + sandbox.restore(); }); function mkEvents() { diff --git a/test/test-utils.js b/test/test-utils.js index ed14306fbe..956c3c15b0 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -21,12 +21,23 @@ module.exports.beforeEach = function(context) { /** * Stub out the MatrixClient, and configure the MatrixClientPeg object to * return it when get() is called. + * + * @returns {sinon.Sandbox}; remember to call sandbox.restore afterwards. */ module.exports.stubClient = function() { - var pegstub = sinon.stub(peg); + var sandbox = sinon.sandbox.create(); + + // 'sandbox.restore()' doesn't work correctly on inherited methods, + // so we do this for each method + var methods = ['get', 'unset', 'replaceUsingUrls', + 'replaceUsingAccessToken']; + for (var i = 0; i < methods.length; i++) { + sandbox.stub(peg, methods[i]); + } var matrixClientStub = sinon.createStubInstance(jssdk.MatrixClient); - pegstub.get.returns(matrixClientStub); + peg.get.returns(matrixClientStub); + return sandbox; } From 7a821ce9d143d7193aee449001c1d536ccd3bb39 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 7 Apr 2016 17:49:39 +0100 Subject: [PATCH 2/3] Make it possible to only run one test file each time --- karma.conf.js | 24 ++++++++++++------- test/{tests.js => all-tests.js} | 6 +---- ...test-component-index.js => skinned-sdk.js} | 16 +++++++++---- 3 files changed, 28 insertions(+), 18 deletions(-) rename test/{tests.js => all-tests.js} (53%) rename test/{test-component-index.js => skinned-sdk.js} (74%) diff --git a/karma.conf.js b/karma.conf.js index eed6d580fa..0abbddb71b 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -11,11 +11,18 @@ var fs = require('fs'); * If you run karma in multi-run mode (with `npm run test-multi`), it will watch * the tests for changes, and webpack will rebuild using a cache. This is much quicker * than a clean rebuild. - * - * TODO: - * - can we run one test at a time? */ +// the name of the test file. By default, a special file which runs all tests. +// +// TODO: this could be a pattern, and karma would run each file, with a +// separate webpack bundle for each file. But then we get a separate instance +// of the sdk, and each of the dependencies, for each test file, and everything +// gets very confused. Can we persuade webpack to put all of the dependencies +// in a 'common' bundle? +// +var testFile = process.env.KARMA_TEST_FILE || 'test/all-tests.js'; + process.env.PHANTOMJS_BIN = 'node_modules/.bin/phantomjs'; function fileExists(name) { @@ -33,6 +40,7 @@ if (!fileExists(gsCss)) { gsCss = 'node_modules/react-gemini-scrollbar/'+gsCss; } + module.exports = function (config) { config.set({ // frameworks to use @@ -41,15 +49,15 @@ module.exports = function (config) { // list of files / patterns to load in the browser files: [ - 'test/tests.js', + testFile, gsCss, ], // list of files to exclude // // This doesn't work. It turns out that it's webpack which does the - // watching of the /test directory (possibly karma only watches - // tests.js itself). Webpack watches the directory so that it can spot + // watching of the /test directory (karma only watches `testFile` + // itself). Webpack watches the directory so that it can spot // new tests, which is fair enough; unfortunately it triggers a rebuild // every time a lockfile is created in that directory, and there // doesn't seem to be any way to tell webpack to ignore particular @@ -63,7 +71,7 @@ module.exports = function (config) { // available preprocessors: // https://npmjs.org/browse/keyword/karma-preprocessor preprocessors: { - 'test/tests.js': ['webpack', 'sourcemap'] + 'test/**/*.js': ['webpack', 'sourcemap'] }, // test results reporter to use @@ -139,7 +147,7 @@ module.exports = function (config) { }, resolve: { alias: { - 'matrix-react-sdk': path.resolve('src/index.js'), + 'matrix-react-sdk': path.resolve('test/skinned-sdk.js'), 'sinon': 'sinon/pkg/sinon.js', }, root: [ diff --git a/test/tests.js b/test/all-tests.js similarity index 53% rename from test/tests.js rename to test/all-tests.js index 3eec9ae8ee..6e604f84ad 100644 --- a/test/tests.js +++ b/test/all-tests.js @@ -1,11 +1,7 @@ -// tests.js +// all-tests.js // // Our master test file: uses the webpack require API to find our test files // and run them -// this is a handly place to make sure the sdk has been skinned -var sdk = require("matrix-react-sdk"); -sdk.loadSkin(require('./test-component-index')); - var context = require.context('.', true, /-test\.jsx?$/); context.keys().forEach(context); diff --git a/test/test-component-index.js b/test/skinned-sdk.js similarity index 74% rename from test/test-component-index.js rename to test/skinned-sdk.js index 0ab5f693dd..5d3526b797 100644 --- a/test/test-component-index.js +++ b/test/skinned-sdk.js @@ -1,12 +1,16 @@ /* - * test-component-index.js + * skinned-sdk.js * - * Stub out a bunch of the components which we expect the application to - * provide + * Skins the react-sdk with a few stub components which we expect the + * application to provide */ -var components = require('../src/component-index.js').components; + +var sdk = require("../src/index"); + +var skin = require('../src/component-index.js'); var stubComponent = require('./components/stub-component.js'); +var components = skin.components; components['structures.LeftPanel'] = stubComponent(); components['structures.RightPanel'] = stubComponent(); components['structures.RoomDirectory'] = stubComponent(); @@ -18,4 +22,6 @@ components['views.messages.DateSeparator'] = stubComponent({displayName: 'DateSe components['views.messages.MessageTimestamp'] = stubComponent({displayName: 'MessageTimestamp'}); components['views.messages.SenderProfile'] = stubComponent({displayName: 'SenderProfile'}); -module.exports.components = components; +sdk.loadSkin(skin); + +module.exports = sdk; From 7e6ea192fd41f35b4d7164ee9e05fecd69059980 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 8 Apr 2016 14:50:04 +0100 Subject: [PATCH 3/3] Build our own stub MatrixClient for the tests It turns out that a bunch of things rely on MatrixClient methods to return promises rather than undefined. Rather than having to undo half the work done by sinon.createStubInstance, just build our own object with as many methods as we need stubbed out. --- test/test-utils.js | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/test/test-utils.js b/test/test-utils.js index 956c3c15b0..a077722678 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -1,9 +1,11 @@ "use strict"; +var sinon = require('sinon'); +var q = require('q'); + var peg = require('../src/MatrixClientPeg.js'); var jssdk = require('matrix-js-sdk'); var MatrixEvent = jssdk.MatrixEvent; -var sinon = require('sinon'); /** * Perform common actions before each test case, e.g. printing the test case @@ -27,6 +29,21 @@ module.exports.beforeEach = function(context) { module.exports.stubClient = function() { var sandbox = sinon.sandbox.create(); + var client = { + getHomeserverUrl: sinon.stub(), + getIdentityServerUrl: sinon.stub(), + + getPushActionsForEvent: sinon.stub(), + getRoom: sinon.stub(), + loginFlows: sinon.stub(), + on: sinon.stub(), + + paginateEventTimeline: sinon.stub().returns(q()), + sendReadReceipt: sinon.stub().returns(q()), + }; + + // create the peg + // 'sandbox.restore()' doesn't work correctly on inherited methods, // so we do this for each method var methods = ['get', 'unset', 'replaceUsingUrls', @@ -34,9 +51,7 @@ module.exports.stubClient = function() { for (var i = 0; i < methods.length; i++) { sandbox.stub(peg, methods[i]); } - - var matrixClientStub = sinon.createStubInstance(jssdk.MatrixClient); - peg.get.returns(matrixClientStub); + peg.get.returns(client); return sandbox; }