Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Add tests for tab reducer => APP_TAB_CLOSED #9179

Merged
merged 1 commit into from
Jun 5, 2017

Conversation

bsclifton
Copy link
Member

@bsclifton bsclifton commented May 31, 2017

Test all branches for tabsReducer => APP_TAB_CLOSED
Fixes #9178

During this process, three issues were found/fixed:

  • window was not being checked if truthy when closed
  • tab close action NEXT was not being respected
  • because getNextTabIdByIndex searches forward and backwards, the setting to first block was dead code (and was removed)

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Test Plan:

Reviewer Checklist:

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

})

it('does nothing if tabId is TAB_ID_NONE')
describe('when updating active tab', function () {
// TODO: test all values for TAB_CLOSE_ACTION
Copy link
Member Author

@bsclifton bsclifton May 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what's left; I'll get back to this soon

@bsclifton bsclifton force-pushed the update-active-tab-test branch from a33e1d1 to 8eb2068 Compare May 31, 2017 23:17
Fixes #9178

During this process, three issues were found/fixed:
- window was not being checked if truthy when closed
- tab close action NEXT was not being respected
- because getNextTabIdByIndex searches forward and backwards, the setting to first block was dead code (and was removed)
@bsclifton bsclifton force-pushed the update-active-tab-test branch from 8eb2068 to 3443c48 Compare June 1, 2017 08:10
@bsclifton bsclifton requested a review from bbondy June 1, 2017 08:11
@bsclifton
Copy link
Member Author

bsclifton commented Jun 1, 2017

@bridiver @bbondy @NejcZdovc ready for review! 😄

Because this resolves an issue that @alexwykoff found, I'm going to merge into 0.15.x

BTW, this brings branch coverage for tabsReducer.js from ~15% to ~40%

@bsclifton
Copy link
Member Author

Has been up for ~5 days without review; it's only tests, so I'll go ahead and merge (please do review if you have time). Thanks!

@bsclifton bsclifton merged commit e14a9db into master Jun 5, 2017
@bsclifton bsclifton deleted the update-active-tab-test branch June 5, 2017 21:11
@bridiver
Copy link
Collaborator

bridiver commented Jun 5, 2017

sorry, last time I looked it said wip

@bridiver
Copy link
Collaborator

bridiver commented Jun 5, 2017

maybe there should be a notification when it changes out of work in progress?

@bsclifton
Copy link
Member Author

@bridiver no worries; I just noticed now that I didn't ping you in this PR (or on Slack). I'll do that next time 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add test for new tab activation
2 participants