[HOLD for payment 2023-03-16] Investigate failed GH action - jest job3 not completing

[chiragsalian]

Problem

We have GH actions failing because jjest ob3 does not complete, examples
https://github.com/Expensify/App/actions/runs/4242889966
https://github.com/Expensify/App/actions/runs/4238427792
https://github.com/Expensify/App/actions/runs/4237306121

Why is this important?

It would be good to improve our GH actions for a more robust process.

Solution

Investigate and fix. I think the issue should be tackled internally.
cc @Expensify/mobile-deployers

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c2dc3b1be4e79d3f
• Upwork Job ID: 1628489993946345472
• Last Price Increase: 2023-02-22

[MelvinBot]

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

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~01c2dc3b1be4e79d3f

[MelvinBot]

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

[MelvinBot]

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

[chiragsalian]

I dont think autoassigners are needed here since its a bug with our process and not really the UI of the app. I'll remove the autoassigners for now.

[chiragsalian]

Hmm all of them seem to have failed right after tests/unit/NumberUtilsTest.js. I wonder if that's a clue.

[roryabraham]

I've seen this before, it's really weird. If you cancel the job and then refresh and look at the logs you'll see that the tests actually complete and pass, but it seems like GitHub doesn't notice that the command is finished and the job keeps running.

Should we create a GitHub support ticket?

[Julesssss]

Should we create a GitHub support ticket?

Yeah 👍 (it's annoying that we can comment but can't react)

[MelvinBot]

Eep! 4 days overdue now. Issues have feelings too...

[parasharrajat]

I've seen this before, it's really weird. If you cancel the job and then refresh and look at the logs you'll see that the tests actually complete and pass, but it seems like GitHub doesn't notice that the command is finished and the job keeps running.

Maybe there is some promise running in background which is causing this issue. (haven't checked the code)

[TMisiukiewicz]

hey, i'll take a look on this one

[MelvinBot]

6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[TMisiukiewicz]

I will continue investigating this issue on Monday morning, seems pretty difficult. I went through all the test suites from the chunk and if I remove SessionTest.js, tests are passing correctly each time, so I think the suspect is somewehere in this file - most possible is Promise not resolved correctly

[TMisiukiewicz]

Proposal

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

Github action responsible for running tests is hanging randomly until the job is cancelled after timeout. It only happens in 3rd chunk of tests.

What is the root cause of that problem?

Running tests with --runInBand flag indicates that each time job is hanging on nativeVersionUpdaterTest.js file. Tests for updateAndroidVersion are false-positive, because of updateAndroidVersion not being returned. Because of that test didn't know this part of code was asynchronous.

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

We should return updateAndroidVersion in the test to prevent tests from waiting until promise is resolved.

What alternative solutions did you explore? (Optional)

[mountiny]

Oh nice, investigation, I think this makes sense, we should ensure any promise does not hang in the tests. Can you create a PR for this?

[mountiny]

This is in main now and seems to work fine so I think we can close this and re-open in case this shows to still fail

[MelvinBot]

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

[MelvinBot]

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

• [No QA] Fix failing GH Action #15704

If no regressions arise, payment will be issued on 2023-03-16. 🎊

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

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[MelvinBot]

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:
• [] Determine if we should create a regression test for the bug.
• [@TMisiukiewicz] If we decide to create a regression test for the bug, please propose the regression test steps to the appropriate location to ensure the same bug will not reach production again.
• [] Review the proposed regression test steps and location.
• [] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[MelvinBot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.