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

.eslintignore

@@ -787,6 +787,7 @@ packages/app-mobile/utils/shim-init-react/injectedJs.js
 packages/app-mobile/utils/shim-init-react/shimInitShared.js
 packages/app-mobile/utils/testing/createMockReduxStore.js
 packages/app-mobile/utils/testing/getWebViewDomById.js
+packages/app-mobile/utils/testing/getWebViewWindowById.js
 packages/app-mobile/utils/types.js
 packages/app-mobile/web/serviceWorker.js
 packages/default-plugins/build.js

.gitignore

@@ -764,6 +764,7 @@ packages/app-mobile/utils/shim-init-react/injectedJs.js
 packages/app-mobile/utils/shim-init-react/shimInitShared.js
 packages/app-mobile/utils/testing/createMockReduxStore.js
 packages/app-mobile/utils/testing/getWebViewDomById.js
+packages/app-mobile/utils/testing/getWebViewWindowById.js
 packages/app-mobile/utils/types.js
 packages/app-mobile/web/serviceWorker.js
 packages/default-plugins/build.js

packages/app-mobile/components/ExtendedWebView/index.jest.tsx

@@ -93,7 +93,7 @@ const ExtendedWebView = (props: Props, ref: Ref<WebViewControl>) => {
 	}, [dom]);
 
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- HACK: Allow wrapper testing logic to access the DOM.
-	const additionalProps: any = { document: dom?.window?.document };
+	const additionalProps: any = { window: dom?.window };
 	return (
 		<View style={props.style} testID={props.testID} {...additionalProps}/>
 	);

packages/app-mobile/components/NoteEditor/NoteEditor.tsx

@@ -569,6 +569,7 @@ function NoteEditor(props: Props, ref: any) {
 			}}>
 				<ExtendedWebView
 					webviewInstanceId='NoteEditor'
+					testID='NoteEditor'
 					scrollEnabled={true}
 					ref={webviewRef}
 					html={html}

packages/app-mobile/components/screens/Note.test.tsx

@@ -1,13 +1,13 @@
 import * as React from 'react';
 
 import { describe, it, beforeEach } from '@jest/globals';
-import { fireEvent, render, screen, userEvent, waitFor } from '@testing-library/react-native';
+import { act, fireEvent, render, screen, userEvent } 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 { runWithFakeTimers, setupDatabaseAndSynchronizer, switchClient, simulateReadOnlyShareEnv } from '@joplin/lib/testing/test-utils';
+import { setupDatabaseAndSynchronizer, switchClient, simulateReadOnlyShareEnv, waitFor } from '@joplin/lib/testing/test-utils';
 import Note from '@joplin/lib/models/Note';
 import { AppState } from '../../utils/types';
 import { Store } from 'redux';
@@ -24,6 +24,8 @@ import { getDisplayParentId } from '@joplin/lib/services/trash';
 import { itemIsReadOnlySync, ItemSlice } from '@joplin/lib/models/utils/readOnly';
 import { LayoutChangeEvent } from 'react-native';
 import shim from '@joplin/lib/shim';
+import getWebViewWindowById from '../../utils/testing/getWebViewWindowById';
+import CodeMirrorControl from '@joplin/editor/CodeMirror/CodeMirrorControl';
 
 interface WrapperProps {
 }
@@ -44,6 +46,27 @@ const getNoteViewerDom = async () => {
 	return await getWebViewDomById('NoteBodyViewer');
 };
 
+const getNoteEditorControl = async () => {
+	const noteEditor = await getWebViewWindowById('NoteEditor');
+	const getEditorControl = () => {
+		if ('cm' in noteEditor.window && noteEditor.window.cm) {
+			return noteEditor.window.cm as CodeMirrorControl;
+		}
+		return null;
+	};
+	await waitFor(async () => {
+		expect(getEditorControl()).toBeTruthy();
+	});
+	return getEditorControl();
+};
+
+const waitForNoteToMatch = async (noteId: string, note: Partial<NoteEntity>) => {
+	await act(() => waitFor(async () => {
+		const loadedNote = await Note.load(noteId);
+		expect(loadedNote).toMatchObject(note);
+	}));
+};
+
 const openNewNote = async (noteProperties: NoteEntity) => {
 	const note = await Note.save({
 		parent_id: (await Folder.defaultFolder()).id,
@@ -62,6 +85,9 @@ const openNewNote = async (noteProperties: NoteEntity) => {
 		id: note.id,
 		folderId: displayParentId,
 	});
+
+	await waitForNoteToMatch(note.id, { parent_id: note.parent_id, title: note.title, body: note.body });
+
 	return note.id;
 };
 
@@ -80,10 +106,20 @@ const openNoteActionsMenu = async () => {
 		cursor = cursor.parent;
 	}
 
-	await runWithFakeTimers(() => userEvent.press(actionMenuButton));
+	await userEvent.press(actionMenuButton);
 };
 
-describe('Note', () => {
+const openEditor = async () => {
+	const editButton = await screen.findByLabelText('Edit');
+	await userEvent.press(editButton);
+};
+
+describe('screens/Note', () => {
+	beforeAll(() => {
+		// advanceTimers: Needed by internal note save logic
+		jest.useFakeTimers({ advanceTimers: true });
+	});
+
 	beforeEach(async () => {
 		await setupDatabaseAndSynchronizer(0);
 		await switchClient(0);
@@ -113,19 +149,59 @@ describe('Note', () => {
 
 	it('changing the note title input should update the note\'s title', async () => {
 		const noteId = await openNewNote({ title: 'Change me!', body: 'Unchanged body' });
+
 		render(<WrappedNoteScreen />);
 
 		const titleInput = await screen.findByDisplayValue('Change me!');
-		// We need to use fake timers while using userEvent to avoid warnings:
+
-		await runWithFakeTimers(async () => {
 		const user = userEvent.setup();
 		await user.clear(titleInput);
 		await user.type(titleInput, 'New title');
+
+		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;
+
+				// 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 waitFor(async () => {
+	it('changing the note body in the editor should update the note\'s body', async () => {
-			expect(await Note.load(noteId)).toMatchObject({ title: 'New title', body: 'Unchanged body' });
+		const defaultBody = 'Change me!';
-		});
+		const noteId = await openNewNote({ title: 'Unchanged title', body: defaultBody });
+
+		const noteScreen = render(<WrappedNoteScreen />);
+
+		await openEditor();
+		const editor = await getNoteEditorControl();
+		editor.select(defaultBody.length, defaultBody.length);
+
+		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.' });
+
+		// 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!' });
+
 	});
 
 	it('pressing "delete" should move the note to the trash', async () => {
@@ -136,9 +212,9 @@ describe('Note', () => {
 		const deleteButton = await screen.findByText('Delete');
 		fireEvent.press(deleteButton);
 
-		await waitFor(async () => {
+		await act(() => waitFor(async () => {
 			expect((await Note.load(noteId)).deleted_time).toBeGreaterThan(0);
-		});
+		}));
 	});
 
 	it('pressing "delete permanently" should permanently delete a note', async () => {
@@ -153,9 +229,9 @@ describe('Note', () => {
 		const deleteButton = await screen.findByText('Permanently delete note');
 		fireEvent.press(deleteButton);
 
-		await waitFor(async () => {
+		await act(() => waitFor(async () => {
 			expect(await Note.load(noteId)).toBeUndefined();
-		});
+		}));
 		expect(shim.showMessageBox).toHaveBeenCalled();
 	});
 

packages/app-mobile/utils/testing/getWebViewDomById.ts

@@ -1,15 +1,7 @@
-import { screen, waitFor } from '@testing-library/react-native';
+import getWebViewWindowById from './getWebViewWindowById';
+
 const getWebViewDomById = async (id: string): Promise<Document> => {
-	const webviewContent = await screen.findByTestId(id);
+	return (await getWebViewWindowById(id)).document;
-	expect(webviewContent).toBeVisible();
-
-	await waitFor(() => {
-		expect(!!webviewContent.props.document).toBe(true);
-	});
-
-	// Return the composite ExtendedWebView component
-	// See https://callstack.github.io/react-native-testing-library/docs/advanced/testing-env#tree-navigation
-	return webviewContent.props.document;
 };
 
 export default getWebViewDomById;

packages/app-mobile/utils/testing/getWebViewWindowById.ts

@@ -0,0 +1,15 @@
+import { screen, waitFor } from '@testing-library/react-native';
+
+const getWebViewWindowById = async (id: string): Promise<Window> => {
+	const webviewContent = await screen.findByTestId(id);
+	expect(webviewContent).toBeVisible();
+
+	await waitFor(() => {
+		expect(!!webviewContent.props.window).toBe(true);
+	});
+
+	const webviewWindow = webviewContent.props.window;
+	return webviewWindow;
+};
+
+export default getWebViewWindowById;

packages/lib/testing/test-utils.ts

@@ -1092,6 +1092,34 @@ export const mockMobilePlatform = (platform: string) => {
 	};
 };
 
+// Waits for callback to not throw. Similar to react-native-testing-library's waitFor, but works better
+// with Joplin's mix of real and fake Jest timers.
+const realSetTimeout = setTimeout;
+export const waitFor = async (callback: ()=> Promise<void>) => {
+	const timeout = 10_000;
+	const startTime = performance.now();
+	let passed = false;
+	let lastError: Error|null = null;
+
+	while (!passed && performance.now() - startTime < timeout) {
+		try {
+			await callback();
+			passed = true;
+			lastError = null;
+		} catch (error) {
+			lastError = error;
+
+			await new Promise<void>(resolve => {
+				realSetTimeout(() => resolve(), 10);
+			});
+		}
+	}
+
+	if (lastError) {
+		throw lastError;
+	}
+};
+
 export const runWithFakeTimers = async (callback: ()=> Promise<void>) => {
 	if (typeof jest === 'undefined') {
 		throw new Error('Fake timers are only supported in jest.');