ReportScreen Reassure test Flakiness

[mountiny]

Problem

As we roll out more Reassure based performance test, we are facing some flakiness in their outpust where they provide false positives. More context here https://expensify.slack.com/archives/C03UK30EA1Z/p1699546341608409?thread_ts=1699521471.426869&cid=C03UK30EA1Z

This is a problem as flakiness in these tests undermines its entire purpose, its not clear as of now what exactly is the root cause of the flaky tests here.

Solution

Identify the root cause of the flakiness and update the reassure test suite to get rid it of, since we measure the count of re-renders, this should not be happening with reassure so most likely this is an error on our side

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[mountiny]

cc @adhorodyski

[mountiny]

Tests have been skipped in this PR #31131 so we can resolve the root cause next week

[mountiny]

@adhorodyski @OlimpiaZurek How is it looking with this one, do you have any ETA for fix?

[OlimpiaZurek]

Hey @mountiny I'll look into it today. We don't have a specific ETA for this as we need to find the root cause first.

[mountiny]

Thanks!

[OlimpiaZurek]

I’ve been investigating the test flakiness issues we've been encountering, and there seem to be two main causes:

• timeouts seem to be a significant factor where certain tests take longer than expected to complete (due to the fact that we run perf tests multiple times to gather reliable results), leading to intermittent crashes. Fixing this issue is relatively simple by increasing the timeout and adding the global configuration to the performance tests folder (I'm preparing a PR early next week)
• the second issue, related to the fact that we count re-renders, was caused by the fact that the feature branches were not updated with the main, as Adam explained in the Slack thread above.

To address this issue we have 2 options:

• keep the current workflow as it is, but emphasize the importance of regular rebasing and eg. and this information to the README.
• implement another workflow on CI that applies the PR changes and then run a performance test.

I've already started exploring possible solutions to improve this workflow and will continue next week.

@mountiny Let me know which approach seems acceptable and if I should continue looking for workflow improvements to avoid test failures in the future.

[adhorodyski]

@mountiny also, please let us know as soon as anything alerting pops up again. Real-time monitoring for cases where we think it actually was up to date with main and the crash remained would be crucial. (however 2nd approach will skip this part if we go for it)

[mountiny]

Thank you very much for digging 🚀

I think being able to sync up the branch with main before running the tests would be great, we can fail the tests if there are merge conflicts and in that case we cannot merge the PR eitherway.

I think we can separate these to new PRs though, if there is no flakiness but simply the fact branch is not synced up with main than that is not a blocker, I think we could start with the global config for timeouts and then re-enabling the ReportScreen tests which were disabled before.

Then we can explore the options to improve the flow with sync with main

[OlimpiaZurek]

@mountiny PR with the global config for timeouts is ready for review .

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

• [NoQA] add global config for timeouts #31549

If no regressions arise, payment will be issued on 2023-12-04. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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.
• [@JmillsExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[JmillsExpensify]

@mountiny We don't need a regression test on this, right? If not, we can close it out now.

[OlimpiaZurek]

We're still exploring ways to improve sync with the main, so I think we should leave it open.

[melvin-bot]

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

• [NoQA] change ressure perf test workflow #31624
• Revert "[NoQA] change ressure perf test workflow" #31825
• [NoQA] enable perf tests for ReportScreen #31828

If no regressions arise, payment will be issued on 2023-12-06. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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.
• [@JmillsExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[mountiny]

Made more progress seems like we got another PR in draft

[OlimpiaZurek]

@mountiny PR with workflow improvements for performance tests is ready for review. I think once we merge and test it, we can close this issue.

[JmillsExpensify]

Still working through this one.

[mountiny]

Last PR was merged for this! Seems like its working well now! Great job @OlimpiaZurek!

Not sure if there is anyone external to pay here so I am going to close this one!