From a33e1d13f4e66dd82d6b26576970cd7ec3db0747 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Wed, 31 May 2017 11:18:57 -0700 Subject: [PATCH] Add tests for tab reducer => APP_TAB_CLOSED Fixes https://github.com/brave/browser-laptop/issues/9178 --- app/browser/windows.js | 2 +- .../app/browser/reducers/tabsReducerTest.js | 161 +++++++++++++----- 2 files changed, 120 insertions(+), 43 deletions(-) diff --git a/app/browser/windows.js b/app/browser/windows.js index 66c040e290e..07819e3c980 100644 --- a/app/browser/windows.js +++ b/app/browser/windows.js @@ -328,7 +328,7 @@ const api = { let win = api.getWindow(windowId) try { setImmediate(() => { - if (!win.isDestroyed()) { + if (win && !win.isDestroyed()) { win.close() } }) diff --git a/test/unit/app/browser/reducers/tabsReducerTest.js b/test/unit/app/browser/reducers/tabsReducerTest.js index 2d9cf4e54bf..7acf5987ef4 100644 --- a/test/unit/app/browser/reducers/tabsReducerTest.js +++ b/test/unit/app/browser/reducers/tabsReducerTest.js @@ -20,23 +20,28 @@ describe('tabsReducer unit tests', function () { this.state = Immutable.fromJS({ tabs: [{ tabId: 1, + index: 0, windowId: 1, pinned: false, active: true }, { tabId: 2, + index: 1, pinned: true, windowId: 1 }, { tabId: 3, + index: 2, pinned: false, windowId: 2, - active: true + active: false }, { tabId: 4, + index: 3, pinned: false, windowId: 2, - active: false + active: true, + openerTabId: 3 }], tabsInternal: { index: { @@ -44,6 +49,20 @@ describe('tabsReducer unit tests', function () { 2: 1, 3: 2, 4: 3 + }, + displayIndex: { + 1: { + 0: 1, + 1: 2 + }, + 2: { + 2: 3, + 3: 4 + } + }, + lastActive: { + 1: [0, 1], + 2: [3, 2] } }, windows: [{ @@ -64,27 +83,12 @@ describe('tabsReducer unit tests', function () { }, toggleDevTools: sinon.mock(), closeTab: sinon.mock(), - moveTo: sinon.mock() + moveTo: sinon.mock(), + setActive: sinon.spy() } - this.windowsAPI = { - closeWindow: sinon.mock() - } - - this.tabStateAPI = { - TAB_ID_NONE: -1, - TAB_ID_ACTIVE: -2, - removeTabByTabId: sinon.mock(), - getActiveTabId: sinon.mock(), - resolveTabId: function (state, tabId) { - return tabId - }, - getWindowId: function (state, tabId) { - return 1 - }, - getNonPinnedTabsByWindowId: function () { return [] }, - getPinnedTabsByWindowId: function () { return [] } - } + this.windowsAPI = require('../../../../../app/browser/windows') + this.tabStateAPI = require('../../../../../app/common/state/tabState') mockery.registerMock('tabs', this.tabsAPI) mockery.registerMock('../tabs', this.tabsAPI) @@ -137,42 +141,104 @@ describe('tabsReducer unit tests', function () { }) }) - describe.skip('APP_TAB_CLOSED', function () { + describe('APP_TAB_CLOSED', function () { const action = { actionType: appConstants.APP_TAB_CLOSED, - tabId: 3 + tabId: 4 } + before(function () { this.clock = sinon.useFakeTimers() }) + after(function () { this.clock.restore() }) + + beforeEach(function () { + this.removeTabByTabIdSpy = sinon.stub(this.tabStateAPI, 'removeTabByTabId', (state) => state) + this.tabsAPI.setActive.reset() + }) + afterEach(function () { - this.tabsAPI.toggleDevTools.reset() - this.tabsAPI.closeTab.reset() - this.tabsAPI.moveTo.reset() - this.tabsAPI.isDevToolsFocused.restore() + this.removeTabByTabIdSpy.restore() }) - it('closes devtools when opened and focused', function () { - this.isDevToolsFocused = sinon.stub(this.tabsAPI, 'isDevToolsFocused', () => true) + + it('calls tabState.removeTabByTabId', function () { tabsReducer(this.state, action) - this.clock.tick(1510) - assert(this.tabsAPI.toggleDevTools.withArgs(this.state, 1).calledOnce) - assert(this.tabsAPI.closeTab.notCalled) + assert(this.tabStateAPI.removeTabByTabId.withArgs(this.state, action.tabId).calledOnce) }) - it('closes tab when tab is focused with no devtools', function () { - this.isDevToolsFocused = sinon.stub(this.tabsAPI, 'isDevToolsFocused', () => false) - tabsReducer(this.state, action) + + it('does nothing if tabId is TAB_ID_NONE', function () { + const invalidAction = { + actionType: action.actionType, + tabId: this.tabStateAPI.TAB_ID_NONE + } + tabsReducer(this.state, invalidAction) this.clock.tick(1510) - assert(this.tabsAPI.toggleDevTools.notCalled) - assert(this.tabsAPI.closeTab.withArgs(this.state, 1).calledOnce) + assert(this.tabStateAPI.removeTabByTabId.notCalled) }) - it('does nothing if tabId is TAB_ID_NONE') + describe('when updating active tab', function () { + // TODO: test all values for TAB_CLOSE_ACTION + it('chooses last active tab id (if possible)', function () { + tabsReducer(this.state, action) + this.clock.tick(1510) + assert(this.tabsAPI.setActive.withArgs(3).calledOnce) + }) - it('calls tabState.removeTabByTabId', function () { - tabsReducer(this.state, action) + describe('when last active tab is not set', function () { + beforeEach(function () { + this.getLastActiveTabIdStub = sinon.stub(this.tabStateAPI, 'getLastActiveTabId') + }) + afterEach(function () { + this.getLastActiveTabIdStub.restore() + }) + + it('chooses `next tab` if nextTabId is TAB_ID_NONE', function () { + const pickNextAction = { + actionType: action.actionType, + tabId: 3 + } + + const testState = this.state + .setIn(['tabs', 2, 'active'], true) + .setIn(['tabs', 3, 'active'], false) + + tabsReducer(testState, pickNextAction) + this.clock.tick(1510) + assert(this.tabsAPI.setActive.withArgs(4).calledOnce) + }) + + describe('if no unpinned tabs come after this', function () { + it('considers pinned tabs which come after this', function () { + const pickNextAction = { + actionType: action.actionType, + tabId: 3 + } + + const testState = this.state + .setIn(['tabs', 2, 'active'], true) + .setIn(['tabs', 3, 'active'], false) + .setIn(['tabs', 3, 'pinned'], true) + + tabsReducer(testState, pickNextAction) + this.clock.tick(1510) + assert(this.tabsAPI.setActive.withArgs(4).calledOnce) + }) + }) + + describe('if no eligible next tabs', function () { + it('chooses the first tab', function () { + const testState = this.state + .deleteIn(['tabs', 3, 'openerTabId']) + + tabsReducer(testState, action) + this.clock.tick(1510) + assert(this.tabsAPI.setActive.withArgs(3).calledOnce) + }) + }) + }) }) }) @@ -227,19 +293,29 @@ describe('tabsReducer unit tests', function () { before(function () { this.clock = sinon.useFakeTimers() }) + after(function () { this.clock.restore() }) + beforeEach(function () { + this.closeWindowSpy = sinon.spy(this.windowsAPI, 'closeWindow') + }) + afterEach(function () { this.tabsAPI.toggleDevTools.reset() this.tabsAPI.closeTab.reset() this.tabsAPI.moveTo.reset() - this.windowsAPI.closeWindow.reset() - this.tabStateAPI.getActiveTabId.reset() + this.closeWindowSpy.restore() }) describe('when tabId == TAB_ID_ACTIVE', function () { + beforeEach(function () { + this.getActiveTabIdSpy = sinon.spy(this.tabStateAPI, 'getActiveTabId') + }) + afterEach(function () { + this.getActiveTabIdSpy.restore() + }) it('calls getActiveTabId to get the actual tabId', function () { const actionActiveTab = { actionType: action.actionType, @@ -413,6 +489,7 @@ describe('tabsReducer unit tests', function () { // frameOpts being dragged is for the first tab assert.deepEqual(args[2], { tabId: 1, windowId: 1, + index: 0, pinned: false, active: true, indexByFrameKey: undefined,