Add tests for tab reducer => APP_TAB_CLOSED

app/browser/reducers/tabsReducer.js

@@ -47,7 +47,7 @@ const updateActiveTab = (state, closeTabId) => {
     case tabCloseAction.LAST_ACTIVE:
       nextTabId = tabState.getLastActiveTabId(state, windowId)
       break
-    default:
+    case tabCloseAction.PARENT:
       {
         const openerTabId = tabState.getOpenerTabId(state, closeTabId)
         const lastActiveTabId = tabState.getLastActiveTabId(state, windowId)
@@ -67,11 +67,6 @@ const updateActiveTab = (state, closeTabId) => {
     }
   }
 
-  // if we can't find anything else just pick the first tab
-  if (nextTabId === tabState.TAB_ID_NONE) {
-    nextTabId = tabState.getTabIdByIndex(state, windowId, 0, true)
-  }
-
   if (nextTabId !== tabState.TAB_ID_NONE) {
     setImmediate(() => {
       tabs.setActive(nextTabId)

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()
         }
       })

test/unit/app/browser/reducers/tabsReducerTest.js

@@ -7,10 +7,14 @@ const appConstants = require('../../../../../js/constants/appConstants')
 const dragTypes = require('../../../../../js/constants/dragTypes')
 const fakeElectron = require('../../../lib/fakeElectron')
 const fakeAdBlock = require('../../../lib/fakeAdBlock')
+const {tabCloseAction} = require('../../../../../app/common/constants/settingsEnums')
+const settings = require('../../../../../js/constants/settings')
 require('../../../braveUnit')
 
 describe('tabsReducer unit tests', function () {
   let tabsReducer
+  let tabCloseSetting
+
   before(function () {
     mockery.enable({
       warnOnReplace: false,
@@ -20,30 +24,57 @@ 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
+      }, {
+        tabId: 5,
+        index: 4,
+        pinned: false,
+        windowId: 2,
+        active: true,
+        openerTabId: 4
       }],
       tabsInternal: {
         index: {
           1: 0,
           2: 1,
           3: 2,
-          4: 3
+          4: 3,
+          5: 4
+        },
+        displayIndex: {
+          1: {
+            0: 1,
+            1: 2
+          },
+          2: {
+            2: 3,
+            3: 4,
+            4: 5
+          }
+        },
+        lastActive: {
+          1: [0, 1],
+          2: [4, 3, 2]
         }
       },
       windows: [{
@@ -64,32 +95,25 @@ 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.windowsAPI = require('../../../../../app/browser/windows')
+    this.tabStateAPI = require('../../../../../app/common/state/tabState')
 
-    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 [] }
-    }
+    tabCloseSetting = tabCloseAction.PARENT
 
     mockery.registerMock('tabs', this.tabsAPI)
     mockery.registerMock('../tabs', this.tabsAPI)
     mockery.registerMock('../windows', this.windowsAPI)
     mockery.registerMock('../../common/state/tabState', this.tabStateAPI)
+    mockery.registerMock('../../../js/settings', { getSetting: (settingKey, settingsCollection, value) => {
+      if (settingKey === settings.TAB_CLOSE_ACTION) {
+        return tabCloseSetting
+      }
+      return false
+    }})
     tabsReducer = require('../../../../../app/browser/reducers/tabsReducer')
   })
 
@@ -137,42 +161,138 @@ 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: 5
     }
+
     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 () {
+      describe('when TAB_CLOSE_ACTION is set to LAST_ACTIVE', function () {
+        before(function () {
+          tabCloseSetting = tabCloseAction.LAST_ACTIVE
+        })
+        after(function () {
+          tabCloseSetting = tabCloseAction.PARENT
+        })
+        it('chooses the last active tab', 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 TAB_CLOSE_ACTION is set to NEXT', function () {
+        before(function () {
+          tabCloseSetting = tabCloseAction.NEXT
+        })
+        after(function () {
+          tabCloseSetting = tabCloseAction.PARENT
+        })
+        it('chooses the next tab', function () {
+          const pickNextAction = {
+            actionType: action.actionType,
+            tabId: 3
+          }
+          const testState = this.state
+            .setIn(['tabs', 2, 'active'], true)
+            .setIn(['tabs', 2, 'openerTabId'], 5)
+            .setIn(['tabsInternal', 'lastActive', '2'], Immutable.fromJS([4, 5, 3]))
+          tabsReducer(testState, pickNextAction)
+          this.clock.tick(1510)
+          assert(this.tabsAPI.setActive.withArgs(4).calledOnce)
+        })
+      })
+
+      describe('when TAB_CLOSE_ACTION is set to PARENT', function () {
+        it('chooses parent tab id (if parent tab was last active)', function () {
+          tabsReducer(this.state, action)
+          this.clock.tick(1510)
+          assert(this.tabsAPI.setActive.withArgs(4).calledOnce)
+        })
+      })
+
+      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 unpinned 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)
+            .setIn(['tabs', 3, 'pinned'], true)
+          tabsReducer(testState, pickNextAction)
+          this.clock.tick(1510)
+          assert(this.tabsAPI.setActive.withArgs(5).calledOnce)
+        })
+
+        it('chooses previous unpinned tab if nextTabId is TAB_ID_NONE', function () {
+          const testState = this.state
+            .setIn(['tabs', 2, 'active'], true)
+            .setIn(['tabs', 3, 'active'], false)
+            .setIn(['tabs', 3, 'pinned'], true)
+          tabsReducer(testState, action)
+          this.clock.tick(1510)
+          assert(this.tabsAPI.setActive.withArgs(3).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, 'pinned'], true)
+              .setIn(['tabs', 4, 'active'], false)
+              .setIn(['tabs', 4, 'pinned'], true)
+            tabsReducer(testState, pickNextAction)
+            this.clock.tick(1510)
+            assert(this.tabsAPI.setActive.withArgs(4).calledOnce)
+          })
+        })
+      })
     })
   })
 
@@ -227,19 +347,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,