- Notifications
You must be signed in to change notification settings - Fork 278
feature: update react nav example #1237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportBase: 94.91% // Head: 94.91% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@ ## main #1237 +/- ## ======================================= Coverage 94.91% 94.91% ======================================= Files 45 45 Lines 3069 3069 Branches 456 456 ======================================= Hits 2913 2913 Misses 156 156 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Co-authored-by: MattAgn <32499425+MattAgn@users.noreply.github.com>
Co-authored-by: MattAgn <32499425+MattAgn@users.noreply.github.com>
Co-authored-by: MattAgn <32499425+MattAgn@users.noreply.github.com>
| @MattAgn @pierrezimmermannbam I've reworked single screen test scenario based on you feedback, pls take a look at |
| Very nice! Maybe we could also add a note somewhere explaining that |
| @MattAgn I've actually added mocking for |
| expect(screen.getByText('The number you have chosen is 100.')).toBeTruthy(); | ||
| | ||
| fireEvent.press(screen.getByRole('button', { name: 'Go back' })); | ||
| // Note: Go Back button get navigation from `useNavigation` hook |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // Note: Go Back button get navigation from `useNavigation` hook | |
| // Note: Go Back button gets navigation from `useNavigation` hook |
| Awesome documentation 😍 |
pierrezimmermannbam left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work ! I see that there is a section regarding testing with react navigation on the website, it would be great to update it as well !
thymikee left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nice. cc @satya164 if you'd like to have a peek — and maybe include this in React Navigation docs
| | ||
| There are two types of tests: | ||
| 1. integration tests operating on whole navigators, they should use `renderNavigator` helper to render a navigator component used in the app. It is useful when you want to test a scenario that includes multiple screens. | ||
| 2. single screen tests where you would pass mock `navigation` prop, built using `buildNavigationMock()` helper, and `route` prop to the screen component using regular `render` function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't show this approach since this will only work for very simple cases. For more involved use cases, mocking the navigation object can quickly get complex ad the available methods are different based on the navigator and nesting. There are also other hooks that would fail when not used in a navigator.
Instead, I'd suggest writing components that don't use anything React Navigation specific and instead accept explicit callbacks and props for what they need, so that it's easy to test them in isolation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@satya164 to understand you correctly, instead of mocking React Nav props/hooks for single screen test, you would rather refactor the code of e.g. DetailsScreen (that accepts route & navigation props) by
- extracting
DetailsScreenContentthat accepts callbacks likeonNavigateBack,onNavigateToSettings(...), etc - make
DetailsScreena thin wrapper overDetailsScreenContentso that it only provides the above callbacks and connects them to navigation actions.
Is that correct or would you do it in some other way?
If that is the case, I think it would be painful for users wanting to test apps using React Nav hooks, since it will require them to refactor each of these components into navigation-less Content and navigation Wrapper components.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like that, but more for standalone components than screens.
If that is the case, I think it would be painful for users wanting to test apps using React Nav hooks
It's not intended to be the primary way of testing the app, it's more about smaller components - more like integration vs unit tests. They don't need to refactor anything since this is another approach - they can still use the first approach to test their screens.
React Navigation has a huge surface area, and it'll be way more painful to write mocks and ensure that they match the correct behavior. Each navigator exposes its own methods and event listeners, the methods can have different combinations depending on which navigator they are nested in. There are many hooks such as useHeaderHeight, useTabbarHeight, useCardAnimation, useIsFocused, useFocusEffect, useNavigationState etc. which can take a lot of time depending on the project, etc. That's why I would encourage people to write unit tests without mocking React Navigation APIs. Otherwise, they can end up writing incorrect mocks and testing incorrect behavior.
Mocking these methods also encourages focusing on wrong things, e.g. people usually assert if navigation.navigate was called with specific screen name, params etc. but that's usually handled by TS already, what really matters is that the new screen is visible - it shouldn't matter whether it called navigation.navigate or navigation.reset, they should be implementation details.
To test screens and write integration tests, I would always recommend creating a navigator so that all of the appropriate APIs are available without incorrect mocking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think testing using navigators is not always feasible, e.g. in case you want to test various scenarios that are on a screen "late" in the stack navigator. I think that in this case user would still be content being able to verify that navigation.navigate(...) or other relevant methods were/weren't called with relevant route and params.
@satya164 Would it be possible/feasible to have some mock TestNavigator that could that could be used to verify assertion. It could be used in following setup:
const Test = createTestNavigator(); test("...", () => { render( <NavigationContainer> <Test.Navigator> <Test.Screen component={ScreenUnderTest} initialParams={{ ... }} /> </Test.Navigator> <NavigationContainer> ); fireEvent.press(getByText('Some button')); expect(Test.navigation.navigate).toHaveBeenCalledWith(['Details', { id: 5 }]); }Such mock would be a full navigator but it would render only a single screen and expose navigation object so that user would be able to verify if they were called with proper params.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a bit of a disconnect here, for me, there are 2 areas for testing:
- Testing screens: Screens can often use navigator-specific methods, rely on navigation lifecycle, navigator-specific behavior, etc. There's no good way to test a screen properly without rendering the navigator. You can mock some stuff, but different screens will need different stuff mocked and in the end, you end up replicating React Navigation's API but it may not have the same behavior. These are integration tests - so we need to test them in an appropriate environment that doesn't behave too differently.
And we're only talking about navigation-related stuff. A screen would usually contain data fetching and other things as well which will need to be mocked.
If we're testing whether navigation.navigate is called or not, it's not the right thing to test for. If we want to test that user goes to Details screen, then we should check if we're on that screen. The navigation.navigate method is an implementation detail - if someone changes its signature or uses a different method like navigation.jumpTo, it may do the exact same thing but will break the tests.
- Testing components: These are unit tests for components that might use React Navigation's API. Mocking is less problematic here, but it's still difficult. One approach would be to write a test navigator (react-navigation can't provide one since all navigators can behave differently and can have different methods). Due to this, I would really recommend dependency injection - pass the callbacks it needs. It'd simplify the tests a lot and ensure that we're only testing the public API of the component, i.e. what the component exposes.
I think that in this case user would still be content being able to verify that navigation.navigate(...) or other relevant methods were/weren't called with relevant route and params.
People may be content with doing them, but imo it's important to suggest best practices and discourage testing implementation details in official docs. Testing the actual thing is harder, yes, but it also ensures that the tests actually help identify issues and don't fail due to a refactor. I am pretty sure people would still mock and test implementation details, but at least they'd have an idea of what a good test should look like by reading official docs. After all, these are just guidelines, people don't have to do it the exact same way - but they'll get more value from their tests if they do.
I'd also bring up the guiding principles of testing library which are in-line with how I see this: The more your tests resemble the way your software is used, the more confidence they can give you. and Avoid testing implementation details.
Summary
Update React Navigation example app to use newest features:
includeHiddenElements,*ByRolequeries, etc.Test plan
All new tests pass.