Feature/click and log #1
Conversation
…o feature/click-and-log
App.test.js
Outdated
| it('renders correctly', () => { | ||
| const tree = renderer.create(<App />).toJSON(); | ||
| expect(tree).toMatchSnapshot(); | ||
| }); |
There was a problem hiding this comment.
Wahey! Tests from the first PR 👍
How should I run these? There's a test script in package.json but both tests fail and the file pattern doesn't actually match this file.
We'll probably want to focus the snapshot tests on smaller components as the project progresses. They're great for ensuring we're not affecting the output of a component as we refactor - but as almost any change will modify the application the snapshots will most likely be rebuilt each time.
There was a problem hiding this comment.
I just ran npm run test or install jest globally and run jest - they seem to be failing since updating App with react navigation.
I also didn't update the snapshot but updating locally seems to return null. Still looking into this, not too familiar with jest.
There was a problem hiding this comment.
Cool cool, maybe this would be a good first thing to pair on to make sure we both understand how testing will work in the app
There was a problem hiding this comment.
Updates:
Fixed previous test errors by updating snapshots and adding mock for 'PersistGate' which was causing App to return null initially - rt2zz/redux-persist#822 (comment)
Added additional unit tests for LogButton component and countReducer reducer.
Modified test script in package.json: running 'npm run test ' should run individual test file or just 'npm run test' to run all.
src/components/LogButton.js
Outdated
| const mapStateToProps = (state) => ({ | ||
| count: state.countReducer.count, | ||
| updated: state.countReducer.updated | ||
| }) | ||
|
|
||
| const mapActionsToProps = { | ||
| incrementCount, | ||
| } | ||
|
|
||
| export default connect(mapStateToProps, mapActionsToProps)(LogButton); |
There was a problem hiding this comment.
Don't worry about it for this PR, but I'd be interested to explore hooks for connecting components to the redux store. I'm not sure if they would be clearer to read, or maybe just make testing harder...
| return { | ||
| ...state, | ||
| count: state.count + 1, | ||
| updated: setUpdated() |
There was a problem hiding this comment.
Ooh! I'm torn on this one. Should the action or the reducer set this timestamp? I think it would be an interesting exercise to write a test for this reducer
There was a problem hiding this comment.
I've left it in the reducer for now, unit test for the reducer can be found in reducers.test.js file and run with 'npm run test reducers.test.js'.
I've bundled tests into a test file at root level - is this best practice or would it be better to have each test file nested with it's component?
There was a problem hiding this comment.
Solid stuff! Works nicely on the iOS simulator. I've left a few questions but let me know if they don't make much sense. As a lot of this will be iterated on quite soon feel free to make whatever changes seem sensible and merge when you're comfortable and we can make further improvements as we go
Updating reducer key as per suggested comment for clarity Co-authored-by: John Bannister <johnb@hey.com>
This includes the first basic component to increment habit count on click and persist locally.
Nothing pushed to main branch yet, keeping on dev and feature branching from there. Happy to change this practice moving forward based on what works best.
Other points:
@jdbann - Could you please review and feedback on how this has been implemented? Anything that could be done better / differently, anything missing etc?