[Due for payment 2025-09-02] [$125] Fix flaky test UnreadIndicatorsTest

[mountiny]

This tests is flaky on main, please, identify the RCA and try to identify the pattern that we could try to avoid in other places. Lets fix this same issue in other tests if present and propose if there is some guard we could put in place to avoid this.

  ● Unread Indicators › Do not display the new line indicator when tracking an expense on self DM while offline

    expect(received).toHaveLength(expected)

    Expected length: 0
    Received length: 1
    Received array:  [{"_fiber": [FiberNode]}]

      672 |         const newMessageLineIndicatorHintText = translateLocal('accessibilityHints.newMessageLineIndicator');
      673 |         const unreadIndicator = screen.queryAllByLabelText(newMessageLineIndicatorHintText);
    > 674 |         expect(unreadIndicator).toHaveLength(0);
          |                                 ^
      675 |     });
      676 |     it('Mark the chat as unread on clicking "Mark as unread" on an item in LHN when the last message of the chat was deleted by another user', async () => {
      677 |         await signInAndGetAppWithUnreadChat();

      at Object.toHaveLength (tests/ui/UnreadIndicatorsTest.tsx:674:33)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:17)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:17:9)

Issue Owner

Current Issue Owner: @abekkala

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @maddylewis (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.

[nkdengineer]

Proposal

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

Fix flaky test UnreadIndicatorsTest

What is the root cause of that problem?

1. We don't return a promise to wait for the Onyx clear to be complete

App/tests/ui/UnreadIndicatorsTest.tsx

Line 190 in 8540a98

Onyx.clear();

2. We can have the same problem as here when calling signInWithTestUser and rendering the sidebar

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

We should return a promise here and also set the fetch to our mock fetch

beforeEach(() => {
    jest.clearAllMocks();

    global.fetch = TestHelper.getGlobalFetchMock();
    // Unsubscribe to pusher channels
    PusherHelper.teardown();

    return Onyx.clear().then(waitForBatchedUpdates);
});

App/tests/ui/UnreadIndicatorsTest.tsx

Line 190 in 8540a98

Onyx.clear();

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

None

What alternative solutions did you explore? (Optional)

NA

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.

[nabi-ebrahimi]

Proposal

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

Fix flaky test UnreadIndicatorsTest

What is the root cause of that problem?

• Race Condition Between State Updates and Assertion
  The app heavily relies on async state updates, which may not be flushed by the time the test reaches the assertion.

• Test Uses Static Queries Instead of waitFor
  The test uses screen.queryAllByLabelText without waiting for the DOM to settle.
  This is risky, especially with asynchronous render cycles or delayed state propagation.

  Using synchronous assertions without awaiting side effects can easily lead to flakiness.

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

Use waitFor when querying elements affected by state or side effects:

await waitFor(() => {
    expect(screen.queryAllByLabelText(newMessageLineIndicatorHintText)).toHaveLength(0);
});

This ensures that React Native's async rendering and Onyx-based updates complete before assertion.

Audit similar tests for risky direct assertions:

• Search tests in the suite using screen.queryBy... or screen.queryAllBy... with immediate expect(...).toHaveLength(0).
• Replace with waitFor where state can be async.

We can consider introducing a shared test utility:

export const waitForUnreadIndicatorToDisappear = async (label: string) => {
    await waitFor(() => {
        expect(screen.queryAllByLabelText(label)).toHaveLength(0);
    });
};

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

[getusha]

@nkdengineer @nabi-ebrahimi it's not a new test case your RCA fails to explain why it suddenly started to fail

[nkdengineer]

@getusha The test will fail sometimes. It's based on Onyx.clear is done before the next test or not. Because we don't return a promise in beforeEach, the next test can be executed before Onyx.clear is complete.

[getusha]

@getusha The test will fail sometimes. It's based on Onyx.clear is done before the next test or not. Because we don't return a promise in beforeEach, the next test can be executed before Onyx.clear is complete.

Is there a way to make sure we don't forget to return the promise when there's an Onyx operation in a test case?