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: Add NoteBodyViewer tests #10747

Merged

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Jul 15, 2024

Summary

This pull request allows testing code that includes communication with a WebView. At present, it includes tests for NoteBodyViewer.

Note

This pull request includes code from #10731 and a version #10748. As such, #10731 and #10748 should be reviewed and merged first.

Motivation

The goal of this pull request is to prevent regressions by increasing the code coverage of the mobile app.

This pull request was motivated, in particular, by these two issues:

Although this pull request currently only includes tests for NoteBodyViewer, it should allow writing tests for Note.tsx and additional tests for NoteEditor.tsx. It should also allow more complete testing of the plugin logic on mobile.

Changes made

  • Mocks ExtendedWebview using JSDOM.
  • Only uses jsdom for certain tests for code intended to run in a WebView, rather than for all tests. The JSDOM constructor doesn't seem to work in a jsdom environment (it fails with a TextEncoder is undefined error).
  • Fixes warnings in seeveral other mobile tests.
  • Adds sharp as a development dependency of app-mobile -- it's used by shim-init-node when attaching pictures to notes during testing.

Manual testing

Android 13:

  • Started the app
  • Opened the note viewer -- verified that markdown is still rendered.
  • Opened the note editor -- verified that notes can still be edited.
  • Opened settings -- verified that plugins still load.

@personalizedrefrigerator personalizedrefrigerator changed the title Mobile: Add NoteBodyViewer tests Chore: Mobile: Add NoteBodyViewer tests Jul 15, 2024
Comment on lines +81 to +85
// 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 };
return (
<View style={props.style} testID={props.testID} {...additionalProps}/>
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently, the virtual DOM for this mock is accessed with code similar to the following:

// Gets the <View></View> that is assigned a testID above:
const webviewContent = await screen.findByTestId('NoteBodyViewer');

// Get the `document` prop provided to the <View> through `additionalProps`:
const doc = webviewContent.props.document;

// Use the `document`
expect(doc.querySelector('h1').textContent).toBe('something'));

Above, document is retrieved through the props passed to the <View> object.

Previously, I was trying to access document through the ref passed to the parent of the ExtendedWebView. This, however, 1) seems to involve tree navigation and 2) (await screen.findByTestId('id')).parent.props.ref does not seem to contain the ref forwarded to the WebView's parent.

@laurent22
Copy link
Owner

There is now a conflict on shim-init-react.ts

@laurent22 laurent22 merged commit 821daec into laurent22:dev Jul 18, 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