Skip to content

Conversation

@mdjastrzebski
Copy link
Member

Summary

Update React Navigation example app to use newest features: includeHiddenElements, *ByRole queries, etc.

Test plan

All new tests pass.

@codecov
Copy link

codecov bot commented Nov 22, 2022

Codecov Report

Base: 94.91% // Head: 94.91% // No change to project coverage 👍

Coverage data is based on head (5bb4e97) compared to base (63586b1).
Patch has no changes to coverable lines.

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

mdjastrzebski and others added 3 commits November 24, 2022 12:47
Co-authored-by: MattAgn <32499425+MattAgn@users.noreply.github.com>
Co-authored-by: MattAgn <32499425+MattAgn@users.noreply.github.com>
@mdjastrzebski
Copy link
Member Author

@MattAgn @pierrezimmermannbam I've reworked single screen test scenario based on you feedback, pls take a look at DetailsScreen.test.ts and tell me what do you think.

@MattAgn
Copy link
Collaborator

MattAgn commented Nov 24, 2022

Very nice! Maybe we could also add a note somewhere explaining that useNavigation and useRoute won't work for this test unless they're mocked globally (or is it too obvious?)

@mdjastrzebski
Copy link
Member Author

@MattAgn I've actually added mocking for useNavigation hook to work with navigation prop. Pls take a look at current DetailsScreen.test.ts.

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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Note: Go Back button get navigation from `useNavigation` hook
// Note: Go Back button gets navigation from `useNavigation` hook
@MattAgn
Copy link
Collaborator

MattAgn commented Nov 27, 2022

Awesome documentation 😍

Copy link
Collaborator

@pierrezimmermannbam pierrezimmermannbam left a 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 !

Copy link
Member

@thymikee thymikee left a 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.
Copy link
Member

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.

Copy link
Member Author

@mdjastrzebski mdjastrzebski Nov 28, 2022

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 DetailsScreenContent that accepts callbacks like onNavigateBack, onNavigateToSettings(...), etc
  • make DetailsScreen a thin wrapper over DetailsScreenContent so 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.

Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

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:

  1. 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.

  1. 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.

@mdjastrzebski mdjastrzebski merged commit a8d15a5 into main Dec 7, 2022
@mdjastrzebski mdjastrzebski deleted the feature/update-react-nav-example branch December 7, 2022 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

7 participants