Skip to content
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

Chore: Mobile: Improve note screen tests and fix CI warning #11126

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Sep 26, 2024

Summary

This pull request improves the mobile note screen tests.

Goal: Simplify writing tests that 1) modify the content of the mobile note editor, then 2) check for a change based on that modification (e.g. note save).

In particular, it:

Note

#11127 can be observed while running the new tests.

This commit improves the mobile note screen tests. In particular, it:
- Tests repeatedly updating the note title, verifying that the note is
  indeed saved.
- Tests updating the note body:
  - Adding chunks and checking that they save before the component
    unmounts.
  - FAILING TEST: Adding text to the note editor then rapidly unmounting the
    component.
@personalizedrefrigerator
Copy link
Collaborator Author

personalizedrefrigerator commented Sep 26, 2024

This pull request seems to fix a large number of warnings observed recently in CI:

Related logs from a recent CI run
 Warning: An update to Animated(Pressable) inside a test was not wrapped in act(...).
➤ YN0000: [@joplin/app-mobile]: 
➤ YN0000: [@joplin/app-mobile]: When testing, code that causes React state updates should be wrapped into act(...):
➤ YN0000: [@joplin/app-mobile]: 
➤ YN0000: [@joplin/app-mobile]: act(() => {
➤ YN0000: [@joplin/app-mobile]:   /* fire events that update state */
➤ YN0000: [@joplin/app-mobile]: });
➤ YN0000: [@joplin/app-mobile]: /* assert on the output */
➤ YN0000: [@joplin/app-mobile]: 
➤ YN0000: [@joplin/app-mobile]: This ensures that you're testing the behavior the user would see in the browser. Learn more at https://reactjs.org/link/wrap-tests-with-act
➤ YN0000: [@joplin/app-mobile]:     at /home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native/Libraries/Animated/createAnimatedComponent.js:35:59
➤ YN0000: [@joplin/app-mobile]:     at View
➤ YN0000: [@joplin/app-mobile]:     at Component (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native/jest/mockComponent.js:30:18)
➤ YN0000: [@joplin/app-mobile]:     at actions (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-paper/src/components/FAB/FABGroup.tsx:203:3)
➤ YN0000: [@joplin/app-mobile]:     at apply (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/@callstack/react-theme-provider/src/createThemeProvider.js:14:3)
➤ YN0000: [@joplin/app-mobile]:     at View
➤ YN0000: [@joplin/app-mobile]:     at Component (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native/jest/mockComponent.js:30:18)
➤ YN0000: [@joplin/app-mobile]:     at PortalManager (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-paper/src/components/Portal/PortalManager.tsx:14:35)
➤ YN0000: [@joplin/app-mobile]:     at PortalHost (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-paper/src/components/Portal/PortalHost.tsx:45:32)
➤ YN0000: [@joplin/app-mobile]:     at RNCSafeAreaProvider
➤ YN0000: [@joplin/app-mobile]:     at Component (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native/jest/mockNativeComponent.js:17:18)
➤ YN0000: [@joplin/app-mobile]:     at children (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-safe-area-context/src/SafeAreaContext.tsx:35:3)
➤ YN0000: [@joplin/app-mobile]:     at children (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-paper/src/core/SafeAreaProviderCompat.tsx:38:50)
➤ YN0000: [@joplin/app-mobile]:     at theme (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-paper/src/core/PaperProvider.tsx:25:11)
➤ YN0000: [@joplin/app-mobile]:     at View
➤ YN0000: [@joplin/app-mobile]:     at Component (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native/jest/mockComponent.js:30:18)
➤ YN0000: [@joplin/app-mobile]:     at View
➤ YN0000: [@joplin/app-mobile]:     at Component (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native/jest/mockComponent.js:30:18)
➤ YN0000: [@joplin/app-mobile]:     at _classCallCheck (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-popup-menu/src/MenuProvider.js:28:22)
➤ YN0000: [@joplin/app-mobile]:     at WrappedNoteScreen
➤ YN0000: [@joplin/app-mobile]: Warning: An update to Animated(View) inside a test was not wrapped in act(...).
➤ YN0000: [@joplin/app-mobile]: 
➤ YN0000: [@joplin/app-mobile]: When testing, code that causes React state updates should be wrapped into act(...):
➤ YN0000: [@joplin/app-mobile]: 
➤ YN0000: [@joplin/app-mobile]: act(() => {
➤ YN0000: [@joplin/app-mobile]:   /* fire events that update state */
➤ YN0000: [@joplin/app-mobile]: });
➤ YN0000: [@joplin/app-mobile]: /* assert on the output */
➤ YN0000: [@joplin/app-mobile]: 
➤ YN0000: [@joplin/app-mobile]: This ensures that you're testing the behavior the user would see in the browser. Learn more at https://reactjs.org/link/wrap-tests-with-act
➤ YN0000: [@joplin/app-mobile]:     at /home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native/Libraries/Animated/createAnimatedComponent.js:35:59
➤ YN0000: [@joplin/app-mobile]:     at elevation (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-paper/src/components/Surface.tsx:158:7)
➤ YN0000: [@joplin/app-mobile]:     at elevation (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-paper/src/components/Surface.tsx:260:7)
➤ YN0000: [@joplin/app-mobile]:     at icon (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-paper/src/components/FAB/FAB.tsx:185:7)
➤ YN0000: [@joplin/app-mobile]:     at View
➤ YN0000: [@joplin/app-mobile]:     at Component (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native/jest/mockComponent.js:30:18)
➤ YN0000: [@joplin/app-mobile]:     at View
➤ YN0000: [@joplin/app-mobile]:     at Component (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native/jest/mockComponent.js:30:18)
➤ YN0000: [@joplin/app-mobile]:     at actions (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-paper/src/components/FAB/FABGroup.tsx:203:3)
➤ YN0000: [@joplin/app-mobile]:     at apply (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/@callstack/react-theme-provider/src/createThemeProvider.js:14:3)
➤ YN0000: [@joplin/app-mobile]:     at View
➤ YN0000: [@joplin/app-mobile]:     at Component (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native/jest/mockComponent.js:30:18)
➤ YN0000: [@joplin/app-mobile]:     at PortalManager (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-paper/src/components/Portal/PortalManager.tsx:14:35)
➤ YN0000: [@joplin/app-mobile]:     at PortalHost (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-paper/src/components/Portal/PortalHost.tsx:45:32)
➤ YN0000: [@joplin/app-mobile]:     at RNCSafeAreaProvider
➤ YN0000: [@joplin/app-mobile]:     at Component (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native/jest/mockNativeComponent.js:17:18)
➤ YN0000: [@joplin/app-mobile]:     at children (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-safe-area-context/src/SafeAreaContext.tsx:35:3)
➤ YN0000: [@joplin/app-mobile]:     at children (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-paper/src/core/SafeAreaProviderCompat.tsx:38:50)
➤ YN0000: [@joplin/app-mobile]:     at theme (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-paper/src/core/PaperProvider.tsx:25:11)
➤ YN0000: [@joplin/app-mobile]:     at View
➤ YN0000: [@joplin/app-mobile]:     at Component (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native/jest/mockComponent.js:30:18)
➤ YN0000: [@joplin/app-mobile]:     at View
➤ YN0000: [@joplin/app-mobile]:     at Component (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native/jest/mockComponent.js:30:18)
➤ YN0000: [@joplin/app-mobile]:     at _classCallCheck (/home/runner/work/joplin/joplin/packages/app-mobile/node_modules/react-native-popup-menu/src/MenuProvider.js:28:22)
➤ YN0000: [@joplin/app-mobile]:     at WrappedNoteScreen
➤ YN0000: [@joplin/app-mobile]: Warning: An update to Animated(Pressable) inside a test was not wrapped in act(...).

@personalizedrefrigerator personalizedrefrigerator changed the title Chore: Mobile: Improve note screen tests Chore: Mobile: Improve note screen tests and fix CI warning Sep 27, 2024
@laurent22 laurent22 merged commit 5935c9c into laurent22:dev Sep 27, 2024
10 checks passed
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.

2 participants