[HOLD for payment 2024-05-02] Investigate Search, SignInpage and ReportScreen flaky perf tests

[mountiny]

Problem

We have noticed recently that some of the SearchPage and SignInPage reassure tests are being flaky. Slack thread.

There is also a ReportScreen test flaky https://expensify.slack.com/archives/C01GTK53T8Q/p1712676878465949?thread_ts=1712669146.879679&cid=C01GTK53T8Q [ReportScreen] should press of the report item

This could have happened after some refactoring of the lower level components, implementing some caching mechanism for example.

This is a problem as its undermining the reliability of the tests.

Solution

Investigate the flakiness and make sure the tests are robust again.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0130b2a9f98f63ea9f
• Upwork Job ID: 1783652764398231552
• Last Price Increase: 2024-04-26

[melvin-bot]

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[OlimpiaZurek]

Hi! I'm Olimpia from Callstack. I want to work on this issue.

[mountiny]

@OlimpiaZurek how is this looking?

[OlimpiaZurek]

@mountiny I have prepared a PR with refactored tests and we are discussing these changes internally. In this PR, I removed the SignInPage tests and moved them down the component tree, but I'm wondering if they should be removed completely. SingInPage is not part of the audit and catching regressions is not important in this part of the UI. Also removing this will also reduce the time taken by Reassure.

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

• Fix: skip flaky reassure tests #39901

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

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

• @OlimpiaZurek does not require payment (Contractor)

[melvin-bot]

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

• [@mountiny] The PR that introduced the bug has been identified. Link to the PR:
• [@mountiny] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@mountiny] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@OlimpiaZurek] Determine if we should create a regression test for this bug.
• [@OlimpiaZurek] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@sakluger] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[OlimpiaZurek]

@mountiny We discussed this with the team and agreed that we should only stick to the tests covering existing metrics. Can you confirm if this approach is acceptable? Thanks.

[mountiny]

We discussed this with the team and agreed that we should only stick to the tests covering existing metrics. Can you confirm if this approach is acceptable? Thanks.

@OlimpiaZurek can you please elaborate? do you mean that we should only have tests which cover the metrics we focus on in audit? Wouldn't that be quite limiting?

[OlimpiaZurek]

@mountiny I think when it comes to testing more complex UI components, it would be better to stick to audit metrics (we’ve already covered most important scenarios for ReportScreen, SearchPage and LHN). This doesn't mean we can't add the tests to other parts of the app that come in the future or that are lower in the component tree, such as rendering and scrolling lists, functions etc.

In this particular case, I suggested removing the tests for SignInPage because we don't think it's very important to catch performance regressions there.

[mountiny]

that does make sense, thanks!

[OlimpiaZurek]

@mountiny PR is ready for review.

[sakluger]

It looks like @hungvu193 is a reviewer on the new PR. @mountiny I'm assuming payment will be $250, and we can handle payment on this issue. @mountiny can you confirm if that plan sounds good?

[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.65-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [NoQA] Reassure: refactor tests for more complex components #40163

If no regressions arise, payment will be issued on 2024-05-02. 🎊

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

• @OlimpiaZurek does not require payment (Contractor)

[melvin-bot]

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

• [@mountiny] The PR that introduced the bug has been identified. Link to the PR:
• [@mountiny] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@mountiny] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@OlimpiaZurek] Determine if we should create a regression test for this bug.
• [@OlimpiaZurek] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@sakluger] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0130b2a9f98f63ea9f

[melvin-bot]

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

[sakluger]

Added @hungvu193 as C+ since he reviewed the PR.

[sakluger]

Sent offer to @hungvu193 https://www.upwork.com/nx/wm/offer/102036441

[sakluger]

Moving to daily since payment is tomorrow.

[sakluger]

All paid, thanks everyone