[HOLD for payment 2024-04-25] [$250] [Reassure] Add measureFunction test for formatSectionsFromSearchTerm

[mountiny]

Problem

The formatSectionsFromSearchTerm is a method that is used often in the App and if its performance degrades, it can have considerable impact on the App performance.

Solution

Let's add a measure function reassure test to cover various cases of this method call similarly as we have created such tests for ReportActionUtils methods for example.

---

Please refer to other tests in the repository, here is a readme for reassure tests which should help you get familiar with the tool.

All new tests should be written in TS.

Please provide a proposal stating, how you could write such test and scenarios for the method such that we test the slowest execution path as well.

cc @OlimpiaZurek

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015c574cb1d199b7bd
• Upwork Job ID: 1767635884215123968
• Last Price Increase: 2024-03-12
• Automatic offers:
• getusha | Reviewer | 0
• brandonhenry | Contributor | 0

Issue Owner

Current Issue Owner: @alexpensify

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~015c574cb1d199b7bd

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @getusha (External)

[melvin-bot]

Triggered auto assignment to @alexpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[melvin-bot]

Upwork job price has been updated to $250

[brandonhenry]

Proposal

Please re-state the problem that we are trying to solve in this issue.

We want to ensure the performance of the formatSectionsFromSearchTerm method does not degrade over time as it is a frequently used method in the app. Slow performance of this method could negatively impact overall app performance and user experience.

What is the root cause of that problem?

The method has several conditional paths and loops through multiple data structures (selectedOptions, filteredRecentReports, filteredPersonalDetails). As the size of these data structures grows, it could lead to slow execution if the implementation is not optimized.

What changes do you think we should make in order to solve the problem?

I suggest adding reassure measureFunction tests to benchmark the execution time of formatSectionsFromSearchTerm under different scenarios:

1. Test with an empty search term and a large number of selectedOptions (tests the first conditional path)
2. Test with a search term that matches a subset of a large selectedOptions array
3. Test with a search term that matches recent reports but not personal details
4. Test with a search term that matches personal details but not recent reports
5. Test with a search term that matches neither recent reports nor personal details (tests the slow path of looping through all 3 data structures)

For each scenario, provide large mock data structures as inputs to stress test performance. Use reassure's measureFunction to get the average execution time over multiple runs.

We should aim to cover the main conditional paths and potential performance bottlenecks. The tests will serve as a benchmark, and we can compare execution times if the implementation of the method changes in the future.

I would put the measureFunction tests in a separate describe block from the regular unit tests, so it's clear which ones are for performance.

[alexpensify]

@getusha when you get a chance, can you review the proposal here? Thanks!

[ShridharGoel]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Add performance test for formatSectionsFromSearchTerm.

What is the root cause of that problem?

New tests to ensure stability of performance in formatSectionsFromSearchTerm.

What changes do you think we should make in order to solve the problem?

Add multiple scenarios with different combinations of searchTerm, selectedOptions, filteredRecentReports, filteredPersonalDetails, maxOptionsSelected, indexOffset, personalDetails and shouldGetOptionDetails.

const searchTerm = 'John';
const selectedOptions = Array.from({ length: 1000 }, (_, i) => ({ accountID: i + 1, searchText: `John Doe ${i + 1}` }));
const filteredRecentReports = Array.from({ length: 10000 }, (_, i) => ({ accountID: i + 1 }));
const filteredPersonalDetails = Array.from({ length: 10000 }, (_, i) => ({ accountID: i + 1 }));
const maxOptionsSelected = false;
const indexOffset = 0;
const personalDetails = {}; // Assuming this is populated accordingly
const shouldGetOptionDetails = false;

A scenario with large data sets:

{
    searchTerm: 'John',
    selectedOptions: selectedOptions,
    filteredRecentReports: filteredRecentReports,
    filteredPersonalDetails: filteredPersonalDetails,
    maxOptionsSelected: false,
    indexOffset: 0,
    personalDetails: personalDetails,
    shouldGetOptionDetails: false,
}

A scenario with max options selected and large data sets:

{
    searchTerm: 'Alice',
    selectedOptions: selectedOptions,
    filteredRecentReports: filteredRecentReports,
    filteredPersonalDetails: filteredPersonalDetails,
    maxOptionsSelected: true,
    indexOffset: 0,
    personalDetails: personalDetails,
    shouldGetOptionDetails: false,
}

We can pass these to the tests.

test(`With large data sets`, async () => {
    await measureFunction(() => formatSectionsFromSearchTerm(
        scenario1.searchTerm,
        scenario1.selectedOptions,
        scenario1.filteredRecentReports,
        scenario1.filteredPersonalDetails,
        scenario1.maxOptionsSelected,
        scenario1.indexOffset,
        scenario1.personalDetails,
        scenario1.shouldGetOptionDetails
    ));
});

test(`With max options selected and large data sets`, async () => {
    await measureFunction(() => formatSectionsFromSearchTerm(
        scenario2.searchTerm,
        scenario2.selectedOptions,
        scenario2.filteredRecentReports,
        scenario2.filteredPersonalDetails,
        scenario2.maxOptionsSelected,
        scenario2.indexOffset,
        scenario2.personalDetails,
        scenario2.shouldGetOptionDetails
    ));
});

Similarly, we can have more combinations.

[getusha]

@mountiny The scenarios @brandonhenry provided looks good to me #38166 (comment)

[ShridharGoel]

Proposal

Updated

[mountiny]

thank you @brandonhenry can you please raise a PR when you get a chance

[melvin-bot]

📣 @getusha 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @brandonhenry 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[brandonhenry]

Will get started on this one right away. Plan to have this turned around by next Friday

[alexpensify]

Thank you @brandonhenry for the update!

[alexpensify]

Thanks, the PR is moving forward in the review process.

[mountiny]

@brandonhenry changes requested, please try to resolve this soon

[brandonhenry]

Updated PR - was having some build issues and couldn't get to this one til now. Will have this back into review tomorrow @mountiny

[brandonhenry]

Still working through this. Have it around soon! (Just fixed final build issues - looks like switching branches and pulling from main requires fresh clean build on both devices, duh...)

[brandonhenry]

@mountiny tests have been updated, back to you 👍🏿

[mallenexpensify]

@brandonhenry per CONTRIBUTING.md,

New contributors are limited to working on one job at a time, however experienced contributors may work on numerous jobs simultaneously.

You currently have 6 issues open with non merged or closed. Please do not submit proposals for new issues until all six PRs have been merged.

Also from CONTRIBUTING.md

Daily updates on weekdays are highly recommended. If you know you won’t be able to provide updates within 48 hours, please comment on the PR or issue stating how long you plan to be out so that we may plan accordingly. We understand everyone needs a little vacation here and there. Any issue that doesn't receive an update for 5 days (including weekend days) may be considered abandoned and the original contract terminated.

Please keep the issue and PR moving along with urgency, along with all others.

[brandonhenry]

This is actively being worked on. Just about wrapped up

[brandonhenry]

Consolidated tests as requested by @OlimpiaZurek . This is back to @getusha

[brandonhenry]

Still in review. Just wrapped some changes up and got that back over

[mallenexpensify]

Thanks for the regular updates @brandonhenry . Please continue to provide them on your assigned issues. When possible, include links.

[brandonhenry]

PR tied to this issue has been merged 🚀 All done here

[alexpensify]

Weekly update: I believe we are waiting for this one to go to Production

[alexpensify]

Heads up, I will be offline until Tuesday, April 23, 2024, and will not actively watch over this GitHub during that period.

If anything urgent is needed here, please ask for help in the #expensify-open-source Slack Room-- thanks!

[melvin-bot]

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

[melvin-bot]

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

• [Reassure] Add measureFunction test for formatSectionsFromSearchTerm  #38551

If no regressions arise, payment will be issued on 2024-04-25. 🎊

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

• @brandonhenry requires payment automatic offer (Contributor)
• @getusha requires payment automatic offer (Reviewer)

[melvin-bot]

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:

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

[mountiny]

$250 to @getusha and to @brandonhenry

[alexpensify]

@getusha and @brandonhenry - there was an issue with Upwork. I updated the job link and need you to accept it here:

https://www.upwork.com/jobs/~01ac085e1e841dfb81

After you accept, I can continue with the next steps. Thanks!

[brandonhenry]

@alexpensify proposal on upwork submitted 👍🏿

[alexpensify]

Payouts due: April 25, 2024

• Contributor: $250 @brandonhenry
• Contributor+: $250 @getusha

Upwork job is here.

[alexpensify]

Everyone has been paid via Upwork -- closing!