[HOLD for payment 2024-08-05] [$125] Fix flakey SubscriptionUtils test

[mountiny]

Problem

It seems like the SubscriptionUtils tests are flaky:

Summary of all failing tests
FAIL tests/unit/SubscriptionUtilsTest.ts (32.87 s)
  ● SubscriptionUtils › getSubscriptionStatus › should return POLICY_OWNER_WITH_AMOUNT_OWED status

    expect(received).toEqual(expected) // deep equality

    - Expected  - 2
    + Received  + 1

      Object {
    -   "isError": true,
    -   "status": "policy_owner_with_amount_owed",
    +   "status": "policy_owner_with_amount_owed_overdue",
      }

      337 |             });
      338 |
    > 339 |             expect(SubscriptionUtils.getSubscriptionStatus()).toEqual({
          |                                                               ^
      340 |                 status: PAYMENT_STATUS.POLICY_OWNER_WITH_AMOUNT_OWED,
      341 |                 isError: true,
      342 |             });

      at Object.toEqual (tests/unit/SubscriptionUtilsTest.ts:339:63)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:22:9)

  ● SubscriptionUtils › getSubscriptionStatus › should return OWNER_OF_POLICY_UNDER_INVOICING status

    expect(received).toEqual(expected) // deep equality

    - Expected  - 1
    + Received  + 1

      Object {
    -   "status": "owner_of_policy_under_invoicing",
    +   "status": "owner_of_policy_under_invoicing_overdue",
      }

      370 |             });
      371 |
    > 372 |             expect(SubscriptionUtils.getSubscriptionStatus()).toEqual({
          |                                                               ^
      373 |                 status: PAYMENT_STATUS.OWNER_OF_POLICY_UNDER_INVOICING,
      374 |             });
      375 |         });

      at Object.toEqual (tests/unit/SubscriptionUtilsTest.ts:372:63)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:22:9)

Here is a run

Solution

Fix the flakiness cc @JKobrynski @blimpich @mananjadhav coming from #44072

Issue Owner

Current Issue Owner: @

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01fc7c3dad62f3576b
• Upwork Job ID: 1816873686533642172
• Last Price Increase: 2024-07-26

Issue Owner

Current Issue Owner: @twisterdotcom

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @twisterdotcom (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

[mountiny]

Or maybe this is fixed in main

[jaydamani]

Proposal

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

tests for SubscriptionUtils.getSubscriptionStatus are flakey

What is the root cause of that problem?

The GRACE_PERIOD_DATE is declared at the beginning and is 1 second + current time so if the other tests take more than 1 second time in total, SubscriptionUtils.getSubscriptionStatus will return overdue status instead of non-overdue.

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

Update the 1 second added in GRACE_PERIOD_DATE to be a larger value like 1 hour.

What alternative solutions did you explore? (Optional)

Instead of using just a constant at the beginning, we could update the GRACE_PERIOD_DATE to be now + 1 second before each test or we could also just use a function that returns now + 1 second

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[mountiny]

@rojiphil can you please review the proposal?

[rojiphil]

RCA of @jaydamani proposal makes sense as 1 second is too tight to check. The current time can become greater than the set time when the tests are executed to cause this flaky behavior.
I think we can safely increase to 1 hour as proposed.
🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @flodnv, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[mountiny]

Assigned @jaydamani as this flakiness is causing some headaches so hopefully we can get rid of it sooner than later

[mountiny]

@jaydamani What is your ETA for the PR? THanks

[jaydamani]

@mountiny, I will do this tonight (IST time)

[jaydamani]

Created this PR
Also @mountiny , not sure if you are the correct person but can you please share/create the Upwork Job for this.
Thanks

[mountiny]

@twisterdotcom will handle that part 🙌

[melvin-bot]

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

[melvin-bot]

Current assignee @rojiphil is eligible for the External assigner, not assigning anyone new.

[twisterdotcom]

Man that was way simpler than creating a job manually!

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

• [No QA] fix(tests/SubscriptionUtils): update time for testing non-overdue #46050

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

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

• @rojiphil requires payment (Needs manual offer from BZ)
• @jaydamani requires payment (Needs manual offer from BZ)

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

• [@rojiphil] The PR that introduced the bug has been identified. Link to the PR:
• [@rojiphil] 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:
• [@rojiphil] 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:
• [@rojiphil] Determine if we should create a regression test for this bug.
• [@rojiphil] 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.
• [@twisterdotcom] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

Payment Summary

Upwork Job

• C+: @rojiphil paid $125 via Upwork here
• Contributor: @jaydamani paid $125 via Upwork here

BugZero Checklist (@twisterdotcom)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1816873686533642172/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[twisterdotcom]

Updated summary above. Closing as we don't need regression for this.