Skip to content

Conversation

@peteruithoven
Copy link
Collaborator

@peteruithoven peteruithoven commented Apr 30, 2016

More changes to finish #86.

  • Updated unit tests
  • Remove initial state filter
  • Added future filtering

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling c9f12bc on peteruithoven:fix-unit-tests into * on omnidan:master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b68d07f on peteruithoven:fix-unit-tests into * on omnidan:master*.

@peteruithoven peteruithoven changed the title WIP: Fix unit tests Finishing #86 Apr 30, 2016
@peteruithoven
Copy link
Collaborator Author

I'm ready for feedback.
I've changed the title and message of the pull request because the changes went beyond just the unit tests.

expect(decrementedState.future).to.deep.equal(mockInitialState.future)
const excludedAction = { type: testConfig.FOR_TEST_ONLY_excludedActions[0] }
const notFilteredReducer = undoable(countReducer, { ...testConfig, filter: null })
let expected = notFilteredReducer(mockInitialState, excludedAction)
Copy link
Owner

Choose a reason for hiding this comment

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

small detail, but you can do:

{
  ...notFilteredReducer(mockInitialState, excludedAction),
  wasFiltered: true
}

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b97d745 on peteruithoven:fix-unit-tests into * on omnidan:master*.

// because the action might have been was filtered it won't be added to the future
if (testConfig && !testConfig.FOR_TEST_ONLY_includeActions) {
if (incrementedState.past.length > jumpToPastIndex) {
expect(jumpToPastState.future.length).to.be.above(incrementedState.future.length)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Check. Makes sense

@omnidan omnidan merged commit f910530 into omnidan:master Apr 30, 2016
@joshkel joshkel mentioned this pull request May 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants