remove redundant performance tests

Expensify · Oct 25, 2024 · 06b0489 · 06b0489
1 parent 904e715
commit 06b0489
Show file tree

Hide file tree

Showing 8 changed files with 64 additions and 564 deletions.
diff --git a/tests/perf-test/README.md → ...butingGuides/REASSURE_PERFORMANCE_TEST.md b/tests/perf-test/README.md → ...butingGuides/REASSURE_PERFORMANCE_TEST.md
@@ -7,7 +7,7 @@ We use Reassure for monitoring performance regression. It helps us check if our
 - Reassure builds on the existing React Testing Library setup and adds a performance measurement functionality. It's intended to be used on local machine and on a remote server as part of your continuous integration setup.
 - To make sure the results are reliable and consistent, Reassure runs tests twice – once for the current branch and once for the base branch.
 
-## Performance Testing Strategy (`measurePerformance`)
+## Performance Testing Strategy (`measureRenders`)
 
 - The primary focus is on testing business cases rather than small, reusable parts that typically don't introduce regressions, although some tests in that area are still necessary.
 - To achieve this goal, it's recommended to stay relatively high up in the React tree, targeting whole screens to recreate real-life scenarios that users may encounter.
@@ -84,7 +84,7 @@ test('Count increments on press', async () => {
     await screen.findByText('Count: 2');
   };
 
-  await measurePerformance(
+  await measureRenders(
     <Counter />,
     { scenario, runs: 20 }
   );

diff --git a/package.json b/package.json
@@ -33,6 +33,7 @@
     "ios-build": "bundle exec fastlane ios build_unsigned",
     "android-build": "bundle exec fastlane android build_local",
     "test": "TZ=utc NODE_OPTIONS=--experimental-vm-modules jest",
+    "perf-test": "NODE_OPTIONS=--experimental-vm-modules npx reassure",
     "typecheck": "NODE_OPTIONS=--max_old_space_size=8192 tsc",
     "lint": "NODE_OPTIONS=--max_old_space_size=8192 eslint . --max-warnings=0 --cache --cache-location=node_modules/.cache/eslint",
     "lint-changed": "NODE_OPTIONS=--max_old_space_size=8192 eslint --max-warnings=0 --config ./.eslintrc.changed.js $(git diff --diff-filter=AM --name-only origin/main HEAD -- \"*.ts\" \"*.tsx\")",

diff --git a/src/components/PopoverMenu.tsx b/src/components/PopoverMenu.tsx
@@ -1,7 +1,7 @@
 /* eslint-disable react/jsx-props-no-spreading */
 import lodashIsEqual from 'lodash/isEqual';
-import type {RefObject} from 'react';
-import React, {Fragment, useLayoutEffect, useState} from 'react';
+import type {ReactNode, RefObject} from 'react';
+import React, {useLayoutEffect, useState} from 'react';
 import {StyleSheet, View} from 'react-native';
 import type {StyleProp, TextStyle, ViewStyle} from 'react-native';
 import type {ModalProps} from 'react-native-modal';
@@ -128,6 +128,14 @@ type PopoverMenuProps = Partial<PopoverModalProps> & {
     shouldUpdateFocusedIndex?: boolean;
 };
 
+const renderWithConditionalWrapper = (shouldUseScrollView: boolean, contentContainerStyle: StyleProp<ViewStyle>, children: ReactNode): React.JSX.Element => {
+    if (shouldUseScrollView) {
+        return <ScrollView contentContainerStyle={contentContainerStyle}>{children}</ScrollView>;
+    }
+    // eslint-disable-next-line react/jsx-no-useless-fragment
+    return <>{children}</>;
+};
+
 function PopoverMenu({
     menuItems,
     onItemSelected,
@@ -169,7 +177,6 @@ function PopoverMenu({
     const {windowHeight} = useWindowDimensions();
 
     const [focusedIndex, setFocusedIndex] = useArrowKeyFocusManager({initialFocusedIndex: currentMenuItemsFocusedIndex, maxIndex: currentMenuItems.length - 1, isActive: isVisible});
-    const WrapComponent = shouldUseScrollView ? ScrollView : Fragment;
 
     const selectItem = (index: number) => {
         const selectedItem = currentMenuItems.at(index);
@@ -233,6 +240,37 @@ function PopoverMenu({
         );
     };
 
+    const renderedMenuItems = currentMenuItems.map((item, menuIndex) => {
+        const {text, onSelected, subMenuItems, shouldCallAfterModalHide, ...menuItemProps} = item;
+        return (
+            <OfflineWithFeedback
+                // eslint-disable-next-line react/no-array-index-key
+                key={`${item.text}_${menuIndex}`}
+                pendingAction={item.pendingAction}
+            >
+                <FocusableMenuItem
+                    // eslint-disable-next-line react/no-array-index-key
+                    key={`${item.text}_${menuIndex}`}
+                    title={text}
+                    onPress={() => selectItem(menuIndex)}
+                    focused={focusedIndex === menuIndex}
+                    shouldShowSelectedItemCheck={shouldShowSelectedItemCheck}
+                    shouldCheckActionAllowedOnPress={false}
+                    onFocus={() => {
+                        if (!shouldUpdateFocusedIndex) {
+                            return;
+                        }
+                        setFocusedIndex(menuIndex);
+                    }}
+                    style={{backgroundColor: item.isSelected ? theme.activeComponentBG : undefined}}
+                    titleStyle={StyleSheet.flatten([styles.flex1, item.titleStyle])}
+                    // Spread other props dynamically
+                    {...menuItemProps}
+                />
+            </OfflineWithFeedback>
+        );
+    });
+
     const renderHeaderText = () => {
         if (!headerText || enteredSubMenuIndexes.length !== 0) {
             return;
@@ -296,39 +334,7 @@ function PopoverMenu({
                 <View style={[isSmallScreenWidth ? {maxHeight: windowHeight - 250} : styles.createMenuContainer, containerStyles]}>
                     {renderHeaderText()}
                     {enteredSubMenuIndexes.length > 0 && renderBackButtonItem()}
-                    {/** eslint-disable-next-line react/jsx-props-no-spreading */}
-                    <WrapComponent {...(shouldUseScrollView && {contentContainerStyle: scrollContainerStyle})}>
-                        {currentMenuItems.map((item, menuIndex) => {
-                            const {text, onSelected, subMenuItems, shouldCallAfterModalHide, ...menuItemProps} = item;
-                            return (
-                                <OfflineWithFeedback
-                                    // eslint-disable-next-line react/no-array-index-key
-                                    key={`${item.text}_${menuIndex}`}
-                                    pendingAction={item.pendingAction}
-                                >
-                                    <FocusableMenuItem
-                                        // eslint-disable-next-line react/no-array-index-key
-                                        key={`${item.text}_${menuIndex}`}
-                                        title={text}
-                                        onPress={() => selectItem(menuIndex)}
-                                        focused={focusedIndex === menuIndex}
-                                        shouldShowSelectedItemCheck={shouldShowSelectedItemCheck}
-                                        shouldCheckActionAllowedOnPress={false}
-                                        onFocus={() => {
-                                            if (!shouldUpdateFocusedIndex) {
-                                                return;
-                                            }
-                                            setFocusedIndex(menuIndex);
-                                        }}
-                                        style={{backgroundColor: item.isSelected ? theme.activeComponentBG : undefined}}
-                                        titleStyle={StyleSheet.flatten([styles.flex1, item.titleStyle])}
-                                        // eslint-disable-next-line react/jsx-props-no-spreading
-                                        {...menuItemProps}
-                                    />
-                                </OfflineWithFeedback>
-                            );
-                        })}
-                    </WrapComponent>
+                    {renderWithConditionalWrapper(shouldUseScrollView, scrollContainerStyle, renderedMenuItems)}
                 </View>
             </FocusTrapForModal>
         </PopoverWithMeasuredContent>

diff --git a/tests/perf-test/ReportActionCompose.perf-test.tsx b/tests/perf-test/ReportActionCompose.perf-test.tsx
@@ -113,52 +113,13 @@ test('[ReportActionCompose] should render Composer with text input interactions'
     await measureRenders(<ReportActionComposeWrapper />, {scenario});
 });
 
-test('[ReportActionCompose] should scroll to hide suggestions', async () => {
+test('[ReportActionCompose] should press create button', async () => {
     const scenario = async () => {
-        // Query for the composer
-        const composer = await screen.findByTestId('composer');
-
-        // scroll to hide suggestions
-        fireEvent.scroll(composer);
-    };
-
-    await waitForBatchedUpdates();
-    await measureRenders(<ReportActionComposeWrapper />, {scenario});
-});
-
-test('[ReportActionCompose] should press to block suggestions', async () => {
-    const scenario = async () => {
-        // Query for the composer
-        const composer = await screen.findByTestId('composer');
-
-        // press to block suggestions
-        fireEvent.press(composer);
-    };
-
-    await waitForBatchedUpdates();
-    await measureRenders(<ReportActionComposeWrapper />, {scenario});
-});
-
-test('[ReportActionCompose] should press add attachemnt button', async () => {
-    const scenario = async () => {
-        // Query for the attachment button
+        // Query for the create button
         const hintAttachmentButtonText = Localize.translateLocal('common.create');
-        const attachmentButton = await screen.findByLabelText(hintAttachmentButtonText);
-
-        fireEvent.press(attachmentButton, mockEvent);
-    };
-
-    await waitForBatchedUpdates();
-    await measureRenders(<ReportActionComposeWrapper />, {scenario});
-});
-
-test('[ReportActionCompose] should press add emoji button', async () => {
-    const scenario = async () => {
-        // Query for the emoji button
-        const hintEmojiButtonText = Localize.translateLocal('reportActionCompose.emoji');
-        const emojiButton = await screen.findByLabelText(hintEmojiButtonText);
+        const createButton = await screen.findByLabelText(hintAttachmentButtonText);
 
-        fireEvent.press(emojiButton);
+        fireEvent.press(createButton, mockEvent);
     };
 
     await waitForBatchedUpdates();
@@ -177,37 +138,3 @@ test('[ReportActionCompose] should press send message button', async () => {
     await waitForBatchedUpdates();
     await measureRenders(<ReportActionComposeWrapper />, {scenario});
 });
-
-test('[ReportActionCompose] press add attachment button', async () => {
-    const scenario = async () => {
-        const hintAddAttachmentButtonText = Localize.translateLocal('reportActionCompose.addAttachment');
-
-        const addAttachmentButton = await screen.findByLabelText(hintAddAttachmentButtonText);
-        fireEvent.press(addAttachmentButton, mockEvent);
-    };
-
-    await waitForBatchedUpdates();
-    await measureRenders(<ReportActionComposeWrapper />, {scenario});
-});
-
-test('[ReportActionCompose] should press split bill button', async () => {
-    const scenario = async () => {
-        const hintSplitBillButtonText = Localize.translateLocal('iou.splitExpense');
-        const splitBillButton = await screen.findByLabelText(hintSplitBillButtonText);
-        fireEvent.press(splitBillButton, mockEvent);
-    };
-
-    await waitForBatchedUpdates();
-    await measureRenders(<ReportActionComposeWrapper />, {scenario});
-});
-
-test('[ReportActionCompose] should press assign task button', async () => {
-    const scenario = async () => {
-        const hintAssignTaskButtonText = Localize.translateLocal('newTaskPage.assignTask');
-        const assignTaskButton = await screen.findByLabelText(hintAssignTaskButtonText);
-        fireEvent.press(assignTaskButton, mockEvent);
-    };
-
-    await waitForBatchedUpdates();
-    await measureRenders(<ReportActionComposeWrapper />, {scenario});
-});
diff --git a/tests/perf-test/ReportActionsList.perf-test.tsx b/tests/perf-test/ReportActionsList.perf-test.tsx
@@ -1,4 +1,4 @@
-import {fireEvent, screen} from '@testing-library/react-native';
+import {screen} from '@testing-library/react-native';
 import type {ComponentType} from 'react';
 import Onyx from 'react-native-onyx';
 import {measureRenders} from 'reassure';
@@ -7,12 +7,10 @@ import type Navigation from '@libs/Navigation/Navigation';
 import ComposeProviders from '@src/components/ComposeProviders';
 import {LocaleContextProvider} from '@src/components/LocaleContextProvider';
 import OnyxProvider from '@src/components/OnyxProvider';
-import * as Localize from '@src/libs/Localize';
 import ONYXKEYS from '@src/ONYXKEYS';
 import ReportActionsList from '@src/pages/home/report/ReportActionsList';
 import {ReportAttachmentsProvider} from '@src/pages/home/report/ReportAttachmentsContext';
 import {ActionListContext, ReactionListContext} from '@src/pages/home/ReportScreenContext';
-import variables from '@src/styles/variables';
 import type {PersonalDetailsList} from '@src/types/onyx';
 import createRandomReportAction from '../utils/collections/reportActions';
 import * as LHNTestUtilsModule from '../utils/LHNTestUtils';
@@ -117,50 +115,3 @@ test('[ReportActionsList] should render ReportActionsList with 500 reportActions
 
     await measureRenders(<ReportActionsListWrapper />, {scenario});
 });
-
-test('[ReportActionsList] should render list items', async () => {
-    const scenario = async () => {
-        const hintText = Localize.translateLocal('accessibilityHints.chatMessage');
-        await screen.findAllByLabelText(hintText);
-    };
-
-    await waitForBatchedUpdates();
-
-    Onyx.multiSet({
-        [ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtilsModule.fakePersonalDetails,
-    });
-
-    await measureRenders(<ReportActionsListWrapper />, {scenario});
-});
-
-test('[ReportActionsList] should scroll through list of items', async () => {
-    const eventData = {
-        nativeEvent: {
-            contentOffset: {
-                y: variables.optionRowHeight * 5,
-            },
-            contentSize: {
-                // Dimensions of the scrollable content
-                height: variables.optionRowHeight * 10,
-                width: 100,
-            },
-            layoutMeasurement: {
-                // Dimensions of the device
-                height: variables.optionRowHeight * 5,
-                width: 100,
-            },
-        },
-    };
-
-    const scenario = async () => {
-        const reportActionsList = await screen.findByTestId('report-actions-list');
-        fireEvent.scroll(reportActionsList, eventData);
-    };
-    await waitForBatchedUpdates();
-
-    Onyx.multiSet({
-        [ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtilsModule.fakePersonalDetails,
-    });
-
-    await measureRenders(<ReportActionsListWrapper />, {scenario});
-});