[$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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[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[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 https://github.com/Expensify/App/issues/38166#issuecomment-1992651656

[ShridharGoel]

Proposal

Updated

[mountiny]

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

[melvin-bot[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[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!

[mountiny]

@brandonhenry have you got a chance to start on a draft at least?

[brandonhenry]

Will have draft up today @mountiny - spent the entire weekend trying to fix my ios build / sim local hosting. Turns out i just needed to update my mac..

[mountiny]

Turns out i just needed to update my mac

Everyone has days like this sometimes

[mountiny]

@brandonhenry whats the latest here?

[brandonhenry]

@mountiny have this back to you today!

[mountiny]

Thanks

[brandonhenry]

Still working through this one, the day got away from me and this took a bit more tinkering. Will have an update tomorrow!

[alexpensify]

Thanks, Brandon, we will keep an eye out for any update for this one.

[brandonhenry]

@alexpensify @mountiny all done with this one - sending for review 😄

[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