From dc20f32ad7b0dbde599f05ea2ef8986e2711d0ea Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Wed, 2 May 2018 11:19:01 +0100 Subject: [PATCH 1/6] Move waitForUpdate to test-utils --- test/components/structures/GroupView-test.js | 21 +------------------- test/test-utils.js | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/test/components/structures/GroupView-test.js b/test/components/structures/GroupView-test.js index 71df26da46..76baafe1c8 100644 --- a/test/components/structures/GroupView-test.js +++ b/test/components/structures/GroupView-test.js @@ -26,32 +26,13 @@ import sdk from 'matrix-react-sdk'; import Matrix from 'matrix-js-sdk'; import * as TestUtils from 'test-utils'; +const { waitForUpdate } = TestUtils; const GroupView = sdk.getComponent('structures.GroupView'); const WrappedGroupView = TestUtils.wrapInMatrixClientContext(GroupView); const Spinner = sdk.getComponent('elements.Spinner'); -/** - * Call fn before calling componentDidUpdate on a react component instance, inst. - * @param {React.Component} inst an instance of a React component. - * @returns {Promise} promise that resolves when componentDidUpdate is called on - * given component instance. - */ -function waitForUpdate(inst) { - return new Promise((resolve, reject) => { - const cdu = inst.componentDidUpdate; - - inst.componentDidUpdate = (prevProps, prevState, snapshot) => { - resolve(); - - if (cdu) cdu(prevProps, prevState, snapshot); - - inst.componentDidUpdate = cdu; - }; - }); -} - describe('GroupView', function() { let root; let rootElement; diff --git a/test/test-utils.js b/test/test-utils.js index d2c685b371..e7d3412722 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -302,3 +302,23 @@ export function wrapInMatrixClientContext(WrappedComponent) { } return Wrapper; } + +/** + * Call fn before calling componentDidUpdate on a react component instance, inst. + * @param {React.Component} inst an instance of a React component. + * @returns {Promise} promise that resolves when componentDidUpdate is called on + * given component instance. + */ +export function waitForUpdate(inst) { + return new Promise((resolve, reject) => { + const cdu = inst.componentDidUpdate; + + inst.componentDidUpdate = (prevProps, prevState, snapshot) => { + resolve(); + + if (cdu) cdu(prevProps, prevState, snapshot); + + inst.componentDidUpdate = cdu; + }; + }); +} From 3e55a456019a5016b5e196ac5da7cd987af7a71b Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 3 May 2018 15:24:41 +0100 Subject: [PATCH 2/6] Mock getGroups on MatrixClient for RoomList _makeGroupInviteTiles --- test/test-utils.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test-utils.js b/test/test-utils.js index e7d3412722..fb21c62170 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -74,6 +74,7 @@ export function createTestClient() { getPushActionsForEvent: sinon.stub(), getRoom: sinon.stub().returns(mkStubRoom()), getRooms: sinon.stub().returns([]), + getGroups: sinon.stub().returns([]), loginFlows: sinon.stub(), on: sinon.stub(), removeListener: sinon.stub(), From 8fcb530e6f677869a5d06b7a5701b231f41174e3 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 3 May 2018 15:25:14 +0100 Subject: [PATCH 3/6] Install lolex for clock mocking in tests --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 1f979369e3..146cb86082 100644 --- a/package.json +++ b/package.json @@ -74,6 +74,7 @@ "isomorphic-fetch": "^2.2.1", "linkifyjs": "^2.1.3", "lodash": "^4.13.1", + "lolex": "^2.3.2", "matrix-js-sdk": "0.10.1", "optimist": "^0.6.1", "pako": "^1.0.5", From 80d251b6223077e03c6cbaf85674e4967e6f6730 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 3 May 2018 15:41:35 +0100 Subject: [PATCH 4/6] Add tests for optimistic updates of moving room tiles --- test/components/views/rooms/RoomList-test.js | 185 +++++++++++++++++++ 1 file changed, 185 insertions(+) create mode 100644 test/components/views/rooms/RoomList-test.js diff --git a/test/components/views/rooms/RoomList-test.js b/test/components/views/rooms/RoomList-test.js new file mode 100644 index 0000000000..bb92d59201 --- /dev/null +++ b/test/components/views/rooms/RoomList-test.js @@ -0,0 +1,185 @@ +import React from 'react'; +import ReactTestUtils from 'react-addons-test-utils'; +import ReactDOM from 'react-dom'; +import expect from 'expect'; +import lolex from 'lolex'; + +import * as TestUtils from 'test-utils'; + +import sdk from '../../../../src/index'; +import MatrixClientPeg from '../../../../src/MatrixClientPeg'; +import { DragDropContext } from 'react-beautiful-dnd'; + +import dis from '../../../../src/dispatcher'; +import DMRoomMap from '../../../../src/utils/DMRoomMap.js'; + +import { Room, RoomMember } from 'matrix-js-sdk'; + +describe('RoomList', () => { + let parentDiv = null; + let sandbox = null; + let client = null; + let root = null; + const myUserId = '@me:domain'; + let clock = null; + + beforeEach(function() { + TestUtils.beforeEach(this); + sandbox = TestUtils.stubClient(sandbox); + client = MatrixClientPeg.get(); + client.credentials = {userId: myUserId}; + + clock = lolex.install(); + + DMRoomMap.makeShared(); + + parentDiv = document.createElement('div'); + document.body.appendChild(parentDiv); + + const RoomList = sdk.getComponent('views.rooms.RoomList'); + const WrappedRoomList = TestUtils.wrapInMatrixClientContext(RoomList); + root = ReactDOM.render( + + + + , parentDiv); + ReactTestUtils.findRenderedComponentWithType(root, RoomList); + }); + + afterEach((done) => { + if (parentDiv) { + ReactDOM.unmountComponentAtNode(parentDiv); + parentDiv.remove(); + parentDiv = null; + } + sandbox.restore(); + + clock.uninstall(); + + done(); + }); + + describe('when no tags are selected', () => { + describe('does correct optimistic update when dragging from', () => { + const movingRoomId = '!someroomid'; + const movingRoom = new Room(movingRoomId); + + // Mock joined member + const myMember = new RoomMember(movingRoomId, myUserId); + myMember.membership = 'join'; + movingRoom.getMember = (userId) => ({ + [client.credentials.userId]: myMember, + }[userId]); + + function expectRoomInSubList(room, subListTest) { + const RoomSubList = sdk.getComponent('structures.RoomSubList'); + const RoomTile = sdk.getComponent('views.rooms.RoomTile'); + + const subLists = ReactTestUtils.scryRenderedComponentsWithType(root, RoomSubList); + const containingSubList = subLists.find(subListTest); + + let expectedRoomTile; + try { + expectedRoomTile = ReactTestUtils.findRenderedComponentWithType(containingSubList, RoomTile); + } catch (err) { + // truncate the error message because it's spammy + err.message = 'Error finding RoomTile: ' + err.message.split('componentType')[0] + '...'; + throw err; + } + + expect(expectedRoomTile).toExist(); + expect(expectedRoomTile.props.room).toBe(room); + } + + function expectCorrectMove(oldTag, newTag) { + const getTagSubListTest = (tag) => { + if (tag === undefined) return (s) => s.props.label.endsWith('Rooms'); + return (s) => s.props.tagName === tag; + }; + + // Default to finding the destination sublist with newTag + const destSubListTest = getTagSubListTest(newTag); + const srcSubListTest = getTagSubListTest(oldTag); + + // Mock the matrix client + client.getRooms = () => [movingRoom]; + + if (['m.favourite', 'm.lowpriority'].includes(oldTag)) movingRoom.tags = {[oldTag]: {}}; + if (oldTag === 'im.vector.fake.direct') { + // Mock inverse m.direct + DMRoomMap.shared().roomToUser = { + [movingRoom.roomId]: '@someotheruser:domain', + }; + } + + dis.dispatch({action: 'MatrixActions.sync', prevState: null, state: 'PREPARED', matrixClient: client}); + + clock.runAll(); + + expectRoomInSubList(movingRoom, srcSubListTest); + + dis.dispatch({action: 'RoomListActions.tagRoom.pending', request: { + oldTag, newTag, room: movingRoom, + }}); + + // Run all setTimeouts for dispatches and room list rate limiting + clock.runAll(); + + expectRoomInSubList(movingRoom, destSubListTest); + } + + it('rooms to people', () => { + expectCorrectMove(undefined, 'im.vector.fake.direct'); + }); + + it('rooms to favourites', () => { + expectCorrectMove(undefined, 'm.favourite'); + }); + + it('rooms to low priority', () => { + expectCorrectMove(undefined, 'm.lowpriority'); + }); + + // XXX: Known to fail - the view does not update immediately to reflect the change. + // Whe running the app live, it updates when some other event occurs (likely the + // m.direct arriving) that these tests do not fire. + xit('people to rooms', () => { + expectCorrectMove('im.vector.fake.direct', undefined); + }); + + it('people to favourites', () => { + expectCorrectMove('im.vector.fake.direct', 'm.favourite'); + }); + + it('people to lowpriority', () => { + expectCorrectMove('im.vector.fake.direct', 'm.lowpriority'); + }); + + it('low priority to rooms', () => { + expectCorrectMove('m.lowpriority', undefined); + }); + + it('low priority to people', () => { + expectCorrectMove('m.lowpriority', 'im.vector.fake.direct'); + }); + + it('low priority to low priority', () => { + expectCorrectMove('m.lowpriority', 'm.lowpriority'); + }); + + it('favourites to rooms', () => { + expectCorrectMove('m.favourite', undefined); + }); + + it('favourites to people', () => { + expectCorrectMove('m.favourite', 'im.vector.fake.direct'); + }); + + it('favourites to low priority', () => { + expectCorrectMove('m.favourite', 'm.lowpriority'); + }); + }); + }); +}); + + From e15b39092dd70b0ddb7ba3b3d007ec30d406c379 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 3 May 2018 18:04:01 +0100 Subject: [PATCH 5/6] Add tests for testing room tile updates when tags (groups in LLP) are selected --- test/components/views/rooms/RoomList-test.js | 244 +++++++++++++------ 1 file changed, 176 insertions(+), 68 deletions(-) diff --git a/test/components/views/rooms/RoomList-test.js b/test/components/views/rooms/RoomList-test.js index bb92d59201..f40a89777b 100644 --- a/test/components/views/rooms/RoomList-test.js +++ b/test/components/views/rooms/RoomList-test.js @@ -12,9 +12,22 @@ import { DragDropContext } from 'react-beautiful-dnd'; import dis from '../../../../src/dispatcher'; import DMRoomMap from '../../../../src/utils/DMRoomMap.js'; +import GroupStore from '../../../../src/stores/GroupStore.js'; import { Room, RoomMember } from 'matrix-js-sdk'; +function generateRoomId() { + return '!' + Math.random().toString().slice(2, 10) + ':domain'; +} + +function createRoom(opts) { + const room = new Room(generateRoomId()); + if (opts) { + Object.assign(room, opts); + } + return room; +} + describe('RoomList', () => { let parentDiv = null; let sandbox = null; @@ -23,6 +36,13 @@ describe('RoomList', () => { const myUserId = '@me:domain'; let clock = null; + const movingRoomId = '!someroomid'; + let movingRoom; + let otherRoom; + + let myMember; + let myOtherMember; + beforeEach(function() { TestUtils.beforeEach(this); sandbox = TestUtils.stubClient(sandbox); @@ -44,6 +64,40 @@ describe('RoomList', () => { , parentDiv); ReactTestUtils.findRenderedComponentWithType(root, RoomList); + + movingRoom = createRoom({name: 'Moving room'}); + expect(movingRoom.roomId).toNotBe(null); + + // Mock joined member + myMember = new RoomMember(movingRoomId, myUserId); + myMember.membership = 'join'; + movingRoom.getMember = (userId) => ({ + [client.credentials.userId]: myMember, + }[userId]); + + otherRoom = createRoom({name: 'Other room'}); + myOtherMember = new RoomMember(otherRoom.roomId, myUserId); + myOtherMember.membership = 'join'; + otherRoom.getMember = (userId) => ({ + [client.credentials.userId]: myOtherMember, + }[userId]); + + // Mock the matrix client + client.getRooms = () => [ + movingRoom, + otherRoom, + createRoom({tags: {'m.favourite': {order: 0.1}}, name: 'Some other room'}), + createRoom({tags: {'m.favourite': {order: 0.2}}, name: 'Some other room 2'}), + createRoom({tags: {'m.lowpriority': {}}, name: 'Some unimportant room'}), + createRoom({tags: {'custom.tag': {}}, name: 'Some room customly tagged'}), + ]; + + const roomMap = {}; + client.getRooms().forEach((r) => { + roomMap[r.roomId] = r; + }); + + client.getRoom = (roomId) => roomMap[roomId]; }); afterEach((done) => { @@ -59,75 +113,68 @@ describe('RoomList', () => { done(); }); - describe('when no tags are selected', () => { + function expectRoomInSubList(room, subListTest) { + const RoomSubList = sdk.getComponent('structures.RoomSubList'); + const RoomTile = sdk.getComponent('views.rooms.RoomTile'); + + const subLists = ReactTestUtils.scryRenderedComponentsWithType(root, RoomSubList); + const containingSubList = subLists.find(subListTest); + + let expectedRoomTile; + try { + const roomTiles = ReactTestUtils.scryRenderedComponentsWithType(containingSubList, RoomTile); + console.info({roomTiles: roomTiles.length}); + expectedRoomTile = roomTiles.find((tile) => tile.props.room === room); + } catch (err) { + // truncate the error message because it's spammy + err.message = 'Error finding RoomTile for ' + room.roomId + ' in ' + + subListTest + ': ' + + err.message.split('componentType')[0] + '...'; + throw err; + } + + expect(expectedRoomTile).toExist(); + expect(expectedRoomTile.props.room).toBe(room); + } + + function expectCorrectMove(oldTag, newTag) { + const getTagSubListTest = (tag) => { + if (tag === undefined) return (s) => s.props.label.endsWith('Rooms'); + return (s) => s.props.tagName === tag; + }; + + // Default to finding the destination sublist with newTag + const destSubListTest = getTagSubListTest(newTag); + const srcSubListTest = getTagSubListTest(oldTag); + + // Set up the room that will be moved such that it has the correct state for a room in + // the section for oldTag + if (['m.favourite', 'm.lowpriority'].includes(oldTag)) movingRoom.tags = {[oldTag]: {}}; + if (oldTag === 'im.vector.fake.direct') { + // Mock inverse m.direct + DMRoomMap.shared().roomToUser = { + [movingRoom.roomId]: '@someotheruser:domain', + }; + } + + dis.dispatch({action: 'MatrixActions.sync', prevState: null, state: 'PREPARED', matrixClient: client}); + + clock.runAll(); + + expectRoomInSubList(movingRoom, srcSubListTest); + + dis.dispatch({action: 'RoomListActions.tagRoom.pending', request: { + oldTag, newTag, room: movingRoom, + }}); + + // Run all setTimeouts for dispatches and room list rate limiting + clock.runAll(); + + expectRoomInSubList(movingRoom, destSubListTest); + } + + function itDoesCorrectOptimisticUpdatesForDraggedRoomTiles() { describe('does correct optimistic update when dragging from', () => { - const movingRoomId = '!someroomid'; - const movingRoom = new Room(movingRoomId); - - // Mock joined member - const myMember = new RoomMember(movingRoomId, myUserId); - myMember.membership = 'join'; - movingRoom.getMember = (userId) => ({ - [client.credentials.userId]: myMember, - }[userId]); - - function expectRoomInSubList(room, subListTest) { - const RoomSubList = sdk.getComponent('structures.RoomSubList'); - const RoomTile = sdk.getComponent('views.rooms.RoomTile'); - - const subLists = ReactTestUtils.scryRenderedComponentsWithType(root, RoomSubList); - const containingSubList = subLists.find(subListTest); - - let expectedRoomTile; - try { - expectedRoomTile = ReactTestUtils.findRenderedComponentWithType(containingSubList, RoomTile); - } catch (err) { - // truncate the error message because it's spammy - err.message = 'Error finding RoomTile: ' + err.message.split('componentType')[0] + '...'; - throw err; - } - - expect(expectedRoomTile).toExist(); - expect(expectedRoomTile.props.room).toBe(room); - } - - function expectCorrectMove(oldTag, newTag) { - const getTagSubListTest = (tag) => { - if (tag === undefined) return (s) => s.props.label.endsWith('Rooms'); - return (s) => s.props.tagName === tag; - }; - - // Default to finding the destination sublist with newTag - const destSubListTest = getTagSubListTest(newTag); - const srcSubListTest = getTagSubListTest(oldTag); - - // Mock the matrix client - client.getRooms = () => [movingRoom]; - - if (['m.favourite', 'm.lowpriority'].includes(oldTag)) movingRoom.tags = {[oldTag]: {}}; - if (oldTag === 'im.vector.fake.direct') { - // Mock inverse m.direct - DMRoomMap.shared().roomToUser = { - [movingRoom.roomId]: '@someotheruser:domain', - }; - } - - dis.dispatch({action: 'MatrixActions.sync', prevState: null, state: 'PREPARED', matrixClient: client}); - - clock.runAll(); - - expectRoomInSubList(movingRoom, srcSubListTest); - - dis.dispatch({action: 'RoomListActions.tagRoom.pending', request: { - oldTag, newTag, room: movingRoom, - }}); - - // Run all setTimeouts for dispatches and room list rate limiting - clock.runAll(); - - expectRoomInSubList(movingRoom, destSubListTest); - } - it('rooms to people', () => { expectCorrectMove(undefined, 'im.vector.fake.direct'); }); @@ -179,6 +226,67 @@ describe('RoomList', () => { expectCorrectMove('m.favourite', 'm.lowpriority'); }); }); + } + + describe('when no tags are selected', () => { + itDoesCorrectOptimisticUpdatesForDraggedRoomTiles(); + }); + + describe('when tags are selected', () => { + function setupSelectedTag() { + // Simulate a complete sync BEFORE dispatching anything else + dis.dispatch({ + action: 'MatrixActions.sync', + prevState: null, + state: 'PREPARED', + matrixClient: client, + }, true); + + // Simulate joined groups being received + dis.dispatch({ + action: 'GroupActions.fetchJoinedGroups.success', + result: { + groups: ['+group:domain'], + }, + }, true); + + // Simulate receiving tag ordering account data + dis.dispatch({ + action: 'MatrixActions.accountData', + event_type: 'im.vector.web.tag_ordering', + event_content: { + tags: ['+group:domain'], + }, + }, true); + + // GroupStore is not flux, mock and notify + GroupStore.getGroupRooms = (groupId) => { + return [movingRoom]; + }; + GroupStore._notifyListeners(); + + // Select tag + dis.dispatch({action: 'select_tag', tag: '+group:domain'}, true); + } + + beforeEach(() => { + setupSelectedTag(); + }); + + it('displays the correct rooms when the groups rooms are changed', () => { + GroupStore.getGroupRooms = (groupId) => { + return [movingRoom, otherRoom]; + }; + GroupStore._notifyListeners(); + + // Run through RoomList debouncing + clock.runAll(); + + // By default, the test will + expectRoomInSubList(otherRoom, (s) => s.props.label.endsWith('Rooms')); + }); + + itDoesCorrectOptimisticUpdatesForDraggedRoomTiles(); }); }); From c06a04af97de05698e6e15773843c0c237deafd7 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 3 May 2018 18:11:32 +0100 Subject: [PATCH 6/6] Fix unrelated linting issue --- test/components/structures/GroupView-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/components/structures/GroupView-test.js b/test/components/structures/GroupView-test.js index 76baafe1c8..3b3510f26e 100644 --- a/test/components/structures/GroupView-test.js +++ b/test/components/structures/GroupView-test.js @@ -18,7 +18,6 @@ import React from 'react'; import ReactDOM from 'react-dom'; import ReactTestUtils from 'react-dom/test-utils'; import expect from 'expect'; -import Promise from 'bluebird'; import MockHttpBackend from 'matrix-mock-request'; import MatrixClientPeg from '../../../src/MatrixClientPeg';