WIP: tests for the examples #278
Conversation
|
|
||
| export default class App extends Component { | ||
| render() { | ||
| const store = createCounterStore(this.props.initialState); |
There was a problem hiding this comment.
The store should not be created inside the render function, otherwise you may have performance and correctness problems. See #287.
|
Thank you so much! It's good, I left a few comments to simplify the approach. |
|
crap haven't seen that one before, I guess my PR is redundant then |
|
@gaearon How do these look? I've learnt more about testing React components as I've progressed, hence the counter components use renderIntoDocument and the todo tests use shallow rendering. The CounterApp tests in particular are more like integration tests at the moment and should maybe be simplified using shallow rendering? |
|
I had a look at them as I started working on them before as well. They look good to me. Great job @sambs! |
|
Thanks @knowbody! Sorry we managed to duplicate our efforts. |
|
haha, no worries |
|
This is RATHER thorough. Extremely fine work. |
|
Whether to use shallow or normal rendering is up to you now. You know about testing React+Redux apps more than me at this point :-) |
| expect(actions.increment()).toEqual({ type: types.INCREMENT_COUNTER }); | ||
| }); | ||
|
|
||
| it('decrement should create descrement action', () => { |
There was a problem hiding this comment.
small typo here, should be decrement
|
Looks like I need to refactor this before merging since #373 removed ES7 features. |
|
@sambs Yeah. There shouldn't be a lot there right? Occasional spread here and there. |
|
I've rebased against the breaking-changes-1.0 branch, removed ES7 stuff and created a new pull request #380 to keep things nice a clean :) |
I thought I'd have a go at writing tests for the examples (Issue #157). Am I heading in the right direction?
I separated out the creation of the store, so that I could more easily instantiate it in the actions tests... though maybe I should just be mocking the
dispatchandgetStateand testing at a lower level?I also added an initialState prop to the App container as its pretty useful to be able to initiate the app in a given state ready for a specific test.