Add tests for tab reducer => APP_TAB_CLOSED

Fixes #9178

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

@@ -20,30 +20,49 @@ 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: {
           1: 0,
           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,