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

[Performance] Implement more reassure tests #33229

Closed
2 of 34 tasks
mountiny opened this issue Dec 18, 2023 · 61 comments
Closed
2 of 34 tasks

[Performance] Implement more reassure tests #33229

mountiny opened this issue Dec 18, 2023 · 61 comments
Assignees
Labels
Engineering Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Planning Changes still in the thought process Reviewing Has a PR in review Weekly KSv2

Comments

@mountiny
Copy link
Contributor

mountiny commented Dec 18, 2023

Problem

In the first phase of the Reassure implementation, we focused on creating performance tests to ensure that important part of the app were working properly. We looked at the LHN (SidebarLinks), ReportScreen and Composer to monitor any potential regressions in the app. We also added performance tests using measureFunction API to the methods from SidebarUtils, ReportActionsUtils and ReportUtils .

Now we can also test other parts of the application as our overall goal is to have a robust system for checking for regression issues.

Solution

As part of our solution for regression monitoring, we are actively identifying the additional components and methods to add to our tracking list. This involves adding them to Reassure tests, which will allow us to monitor their performance and catch any potential issues early on.

Here is the updated list of components and functions to export to the community:

Components:

Functions:

Next components for which we want to add Reassure tests:

  • SignInPage
  • SearchPage
  • NewChatPage
  • MoneyRequestConfirmationList
  • TaskAssignSelectorModal
  • ReportParticipantPage
  • RoomInvitePage
  • RoomMembersPage
  • EmojiPicker

The main scope of testing above components:

  • rendering lists with multiple items (we try to add as much data as possible to get a more reliable result when it comes to potential regressions)
  • scrolling performance
  • onPress/onSelect action list item
  • interactions with text input
  • pressing the button

Additionally we'll use the measureFunction API to test and track the performance of methods from:

  • EmojiPickerUtils
  • OptionsListUtils
  • PaymentUtils
  • PersonalDetailsUtils
  • PolicyUtils

Our goal is to focus on functions from the above list that contain heavy computations and are frequently used in the application.
We will continuously expand this tracking list by adding new components and methods that we identify during our daily work. This proactive approach allows us to catch any performance regressions early on, maintaining a fast and efficient user experience.
If you have any suggestions for further components/scope that we want to add performance tests for, or if you have already identified methods involving heavy calculations that are worth adding to the list, please feel free to contribute.


We should prioritize the list components first

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~010343afd21b08bf6e
  • Upwork Job ID: 1736750358470123520
  • Last Price Increase: 2023-12-18
@mountiny mountiny added Engineering Daily KSv2 Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. labels Dec 18, 2023
@mountiny mountiny self-assigned this Dec 18, 2023
Copy link

melvin-bot bot commented Dec 18, 2023

Copy link

melvin-bot bot commented Dec 18, 2023

Job added to Upwork: https://www.upwork.com/jobs/~010343afd21b08bf6e

@melvin-bot melvin-bot bot added the Weekly KSv2 label Dec 18, 2023
@melvin-bot melvin-bot bot removed the Daily KSv2 label Dec 18, 2023
Copy link

melvin-bot bot commented Dec 18, 2023

Triggered auto assignment to Contributor Plus for review of internal employee PR - @s77rt (Internal)

@mountiny
Copy link
Contributor Author

c+ or BZ not needed for now

@OlimpiaZurek
Copy link
Contributor

Hi, I'm Olimpia from Callstack - expert contributor group. I'd like to work on this.

@OlimpiaZurek
Copy link
Contributor

OlimpiaZurek commented Dec 20, 2023

@mountiny Here are the components/functions we should test first (marked as * those for which we could ask the community to add tests):

Lists :

  • components/OptionSelector/BaseOptionsSelector.js
  • components/OptionsList/BaseOptionsList.js *
  • components/InvertedFlatList/BaseInvertedFlatList.js*

Components:

  • pages/SearchPage.js
  • pages/NewChatPage.js *
  • pages/RoomInvitePage.js *
  • pages/ReportParticipantsPage.js *
  • pages/workspaces/WorkspacesListPage

Functions:

  • getOptions(OptionsListUtils)
  • formatSectionsFromSearchTerm(OptionsListUtils) *
  • getMemberAccountIDsForWorkspace(PolicyUtils) *

At a later stage, we can add the test to the remaining components from initial list and these functions:

  • getAvatarsForAccountIDs (OptionsListUtils)
  • getPersonalDetailsForAccountIDs(OptionsListUtils)
  • getLastMessageTextForReport(OptionsListUtils)
  • createOption(OptionsListUtils)
  • getActivePolicies(PolicyUtils)
  • getIneligibleInvitees(PolicyUtils)
  • getPersonalDetailsByIDs (PersonalDetailsUtils)
  • getAccountIDsByLogins(PersonalDetailsUtils)
  • getLoginsByAccountIDs(PersonalDetailsUtils)
  • getNewPersonalDetailsOnyxData(PersonalDetailsUtils)

I'm also considering adding some quick instructions/tips related to testing with Reassure to README.md that could help the community implement these tests. I will prepare a separate PR for this.

@mountiny
Copy link
Contributor Author

Thank you! That sounds great! @OlimpiaZurek

I'm also considering adding some quick instructions/tips related to testing with Reassure to README.md that could help the community implement these tests. I will prepare a separate PR for this.

Could you try to make these before holidays and we could export the issues for community over the holidays?

@OlimpiaZurek
Copy link
Contributor

Sure, I'm working on it now.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Dec 22, 2023
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Dec 22, 2023
@OlimpiaZurek
Copy link
Contributor

@mountiny The PR with instructions is ready. I also opened a PR with Reassure tests for OptionsSelector.
The component we would like to test next is SearchPage.

Since I will be on vacation for the next two weeks, one of the developer from Callstack will take over this part.

@muttmuure
Copy link
Contributor

Sorry, I've been a little too swamped this week to split these out into separate issues for the community to take. I will work on it early next week

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Mar 6, 2024
@melvin-bot melvin-bot bot changed the title [Performance] Implement more reassure tests [HOLD for payment 2024-03-13] [Performance] Implement more reassure tests Mar 6, 2024
Copy link

melvin-bot bot commented Mar 6, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Mar 6, 2024
Copy link

melvin-bot bot commented Mar 6, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.47-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-03-13. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Mar 6, 2024

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@OlimpiaZurek] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@mountiny mountiny added Planning Changes still in the thought process and removed Awaiting Payment Auto-added when associated PR is deployed to production labels Mar 12, 2024
Copy link

melvin-bot bot commented Mar 13, 2024

Skipping the payment summary for this issue since all the assignees are employees or vendors. If this is incorrect, please manually add the payment summary SO.

@mountiny mountiny changed the title [HOLD for payment 2024-03-13] [Performance] Implement more reassure tests [Performance] Implement more reassure tests Mar 13, 2024
@melvin-bot melvin-bot bot added the Overdue label Mar 15, 2024
@mountiny
Copy link
Contributor Author

This is underway, we got 4 issues branched off

@melvin-bot melvin-bot bot removed the Overdue label Mar 18, 2024
@melvin-bot melvin-bot bot added the Overdue label Apr 1, 2024
@mountiny
Copy link
Contributor Author

mountiny commented Apr 2, 2024

I will create more issues

@melvin-bot melvin-bot bot removed the Overdue label Apr 2, 2024
@melvin-bot melvin-bot bot added the Overdue label Apr 11, 2024
@mountiny
Copy link
Contributor Author

We are still looking into some flakiness which is being worked on here #39913 so we would only create new tests after that is resolved.

@melvin-bot melvin-bot bot removed the Overdue label Apr 16, 2024
@melvin-bot melvin-bot bot added the Overdue label Apr 24, 2024
@mountiny
Copy link
Contributor Author

Lets close this one for now and I will look into creating more issues for functions in future

@melvin-bot melvin-bot bot removed the Overdue label Apr 24, 2024
@github-project-automation github-project-automation bot moved this from CRITICAL to Done in [#whatsnext] #quality Apr 24, 2024
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Planning Changes still in the thought process Reviewing Has a PR in review Weekly KSv2
Projects
Development

No branches or pull requests

6 participants