From 5fceb5a3c94f9ca0cb155064a6fb5b496ad55f0e Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Sat, 28 Sep 2024 08:20:46 -0700 Subject: [PATCH] Chore: Reduce mobile note screen test flakiness (#11145) --- .../components/ScreenHeader/Menu.tsx | 1 + .../components/screens/Note.test.tsx | 98 +++++++++++-------- packages/lib/testing/test-utils.ts | 3 +- 3 files changed, 59 insertions(+), 43 deletions(-) diff --git a/packages/app-mobile/components/ScreenHeader/Menu.tsx b/packages/app-mobile/components/ScreenHeader/Menu.tsx index c5f41ed0b..c51acae3c 100644 --- a/packages/app-mobile/components/ScreenHeader/Menu.tsx +++ b/packages/app-mobile/components/ScreenHeader/Menu.tsx @@ -139,6 +139,7 @@ const MenuComponent: React.FC = props => { style={styles.menuContentScroller} aria-modal={true} accessibilityViewIsModal={true} + testID={`menu-content-${refocusCounter ? 'refocusing' : ''}`} >{menuOptionComponents} diff --git a/packages/app-mobile/components/screens/Note.test.tsx b/packages/app-mobile/components/screens/Note.test.tsx index 1997a9459..a903803c5 100644 --- a/packages/app-mobile/components/screens/Note.test.tsx +++ b/packages/app-mobile/components/screens/Note.test.tsx @@ -1,13 +1,14 @@ import * as React from 'react'; import { describe, it, beforeEach } from '@jest/globals'; -import { act, fireEvent, render, screen, userEvent } from '@testing-library/react-native'; +import { act, fireEvent, render, screen, userEvent, waitFor } from '@testing-library/react-native'; import '@testing-library/jest-native/extend-expect'; import { Provider } from 'react-redux'; import NoteScreen from './Note'; import { MenuProvider } from 'react-native-popup-menu'; -import { setupDatabaseAndSynchronizer, switchClient, simulateReadOnlyShareEnv, waitFor } from '@joplin/lib/testing/test-utils'; +import { setupDatabaseAndSynchronizer, switchClient, simulateReadOnlyShareEnv, runWithFakeTimers } from '@joplin/lib/testing/test-utils'; +import { waitFor as waitForWithRealTimers } from '@joplin/lib/testing/test-utils'; import Note from '@joplin/lib/models/Note'; import { AppState } from '../../utils/types'; import { Store } from 'redux'; @@ -61,7 +62,7 @@ const getNoteEditorControl = async () => { }; const waitForNoteToMatch = async (noteId: string, note: Partial) => { - await act(() => waitFor(async () => { + await act(() => waitForWithRealTimers(async () => { const loadedNote = await Note.load(noteId); expect(loadedNote).toMatchObject(note); })); @@ -72,7 +73,6 @@ const openNewNote = async (noteProperties: NoteEntity) => { parent_id: (await Folder.defaultFolder()).id, ...noteProperties, }); - const displayParentId = getDisplayParentId(note, await Folder.load(note.parent_id)); store.dispatch({ @@ -106,20 +106,31 @@ const openNoteActionsMenu = async () => { cursor = cursor.parent; } - await userEvent.press(actionMenuButton); + // Wrap in act(...) -- this tells the test library that component state is intended to update (prevents + // warnings). + await act(async () => { + await runWithFakeTimers(async () => { + await userEvent.press(actionMenuButton); + }); + + // State can update until the menu content is marked as in the process of refocusing (part of the + // menu transition). + await waitFor(async () => { + expect(await screen.findByTestId('menu-content-refocusing')).toBeVisible(); + }); + }); }; const openEditor = async () => { const editButton = await screen.findByLabelText('Edit'); - await userEvent.press(editButton); + + fireEvent.press(editButton); + await waitFor(() => { + expect(screen.queryByLabelText('Edit')).toBeNull(); + }); }; describe('screens/Note', () => { - beforeAll(() => { - // advanceTimers: Needed by internal note save logic - jest.useFakeTimers({ advanceTimers: true }); - }); - beforeEach(async () => { await setupDatabaseAndSynchronizer(0); await switchClient(0); @@ -160,20 +171,23 @@ describe('screens/Note', () => { await waitForNoteToMatch(noteId, { title: 'New title', body: 'Unchanged body' }); - let expectedTitle = 'New title'; - for (let i = 0; i <= 10; i++) { - for (const chunk of ['!', ' test', '!!!', ' Testing']) { - jest.advanceTimersByTime(i % 5); - await user.type(titleInput, chunk); - expectedTitle += chunk; + // Use fake timers to allow advancing timers without pausing the test + await runWithFakeTimers(async () => { + let expectedTitle = 'New title'; + for (let i = 0; i <= 10; i++) { + for (const chunk of ['!', ' test', '!!!', ' Testing']) { + jest.advanceTimersByTime(i % 5); + await user.type(titleInput, chunk); + expectedTitle += chunk; - // Don't verify after each input event -- this allows the save action queue to fill. - if (i % 4 === 0) { - await waitForNoteToMatch(noteId, { title: expectedTitle }); + // Don't verify after each input event -- this allows the save action queue to fill. + if (i % 4 === 0) { + await waitForNoteToMatch(noteId, { title: expectedTitle }); + } } + await waitForNoteToMatch(noteId, { title: expectedTitle }); } - await waitForNoteToMatch(noteId, { title: expectedTitle }); - } + }); }); it('changing the note body in the editor should update the note\'s body', async () => { @@ -181,27 +195,27 @@ describe('screens/Note', () => { const noteId = await openNewNote({ title: 'Unchanged title', body: defaultBody }); const noteScreen = render(); + await act(async () => await runWithFakeTimers(async () => { + await openEditor(); + const editor = await getNoteEditorControl(); + editor.select(defaultBody.length, defaultBody.length); - await openEditor(); - const editor = await getNoteEditorControl(); - editor.select(defaultBody.length, defaultBody.length); + editor.insertText(' Testing!!!'); + await waitForNoteToMatch(noteId, { body: 'Change me! Testing!!!' }); - editor.insertText(' Testing!!!'); - await waitForNoteToMatch(noteId, { body: 'Change me! Testing!!!' }); + editor.insertText(' This is a test.'); + await waitForNoteToMatch(noteId, { body: 'Change me! Testing!!! This is a test.' }); - editor.insertText(' This is a test.'); - await waitForNoteToMatch(noteId, { body: 'Change me! Testing!!! This is a test.' }); + // should also save changes made shortly before unmounting + editor.insertText(' Test!'); - // should also save changes made shortly before unmounting - editor.insertText(' Test!'); - - // TODO: Decreasing this below 100 causes the test to fail. - // See issue #11125. - await jest.advanceTimersByTimeAsync(150); - - noteScreen.unmount(); - await waitForNoteToMatch(noteId, { body: 'Change me! Testing!!! This is a test. Test!' }); + // TODO: Decreasing this below 100 causes the test to fail. + // See issue #11125. + await jest.advanceTimersByTimeAsync(450); + noteScreen.unmount(); + await waitForNoteToMatch(noteId, { body: 'Change me! Testing!!! This is a test. Test!' }); + })); }); it('pressing "delete" should move the note to the trash', async () => { @@ -212,9 +226,9 @@ describe('screens/Note', () => { const deleteButton = await screen.findByText('Delete'); fireEvent.press(deleteButton); - await act(() => waitFor(async () => { + await waitFor(async () => { expect((await Note.load(noteId)).deleted_time).toBeGreaterThan(0); - })); + }); }); it('pressing "delete permanently" should permanently delete a note', async () => { @@ -229,9 +243,9 @@ describe('screens/Note', () => { const deleteButton = await screen.findByText('Permanently delete note'); fireEvent.press(deleteButton); - await act(() => waitFor(async () => { + await waitFor(async () => { expect(await Note.load(noteId)).toBeUndefined(); - })); + }); expect(shim.showMessageBox).toHaveBeenCalled(); }); diff --git a/packages/lib/testing/test-utils.ts b/packages/lib/testing/test-utils.ts index cc5f558ce..da0bdef06 100644 --- a/packages/lib/testing/test-utils.ts +++ b/packages/lib/testing/test-utils.ts @@ -1125,7 +1125,8 @@ export const runWithFakeTimers = async (callback: ()=> Promise) => { throw new Error('Fake timers are only supported in jest.'); } - jest.useFakeTimers(); + // advanceTimers: Needed by Joplin's database driver + jest.useFakeTimers({ advanceTimers: true }); // The shim.setTimeout and similar functions need to be changed to // use fake timers.