[Due for payment 2026-06-25] [$250] Flaky test: UnreadIndicatorsTest.tsx › Display bold in the LHN (120s timeout)

[mountiny]

Flaky test

tests/ui/UnreadIndicatorsTest.tsx › Display bold in the LHN for unread chat and new line indicator above the chat message when we navigate to it intermittently fails in CI with:

Exceeded timeout of 120000 ms for a test.

This is a timeout flake, not an assertion failure — the test mounts a full <App /> and occasionally exceeds the 120s CI limit under load.

Where it surfaced

• Failing test (job 7) on main: https://github.com/Expensify/App/actions/runs/27682268050/job/81872373685
• Triggered by merge of [No QA] Remove explanation modal #93779

Prior work

PR #92587 fixed the final scroll/badge assertion in this test (replacing waitFor + act with waitForBatchedUpdates + synchronous assert), but the test still timed out on main (Jun 17, 2026). Two other tests in the same file still used the old waitFor badge pattern (~lines 456, 490).

Affected test

tests/ui/UnreadIndicatorsTest.tsx — Display bold in the LHN for unread chat and new line indicator above the chat message when we navigate to it

Prior occurrence

Related closed issues/PRs:

• [Due for payment 2026-06-11] Fix flaky UnreadIndicatorsTest - mark as unread last message deleted #92122 (different test case in same file)
• [No QA] Fix flaky ContactMethodsPage and UnreadIndicators tests #92587 (partial fix for badge assertion)
• Investigate workflow job failing on main: test / test (job 7) #92250 (bot investigation for job 7 failure)

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~022067266188442126473
• Upwork Job ID: 2067266188442126473
• Last Price Increase: 2026-06-17

Issue Owner

Current Issue Owner: @mallenexpensify

[mountiny]

PR: #93830

[ReaperOAK]

Proposal

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

The test Display bold in the LHN for unread chat and new line indicator above the chat message when we navigate to it in tests/ui/UnreadIndicatorsTest.tsx intermittently hits the 120-second Jest timeout in CI without an assertion failure, caused by waitFor-based polling assertions that stall under CI load.

What is the root cause of that problem?

Disclaimer: The investigation context contains no source code windows for tests/ui/UnreadIndicatorsTest.tsx, so exact line numbers and variable names cannot be verified from static analysis alone. The following is based on the issue description's explicit statements about the file.

The issue description states directly:

"Two other tests in the same file still used the old waitFor badge pattern (~lines 456, 490)."

The pattern being referred to is the anti-pattern that PR #92587 partially fixed elsewhere in the same test: using waitFor(() => expect(...)) for assertions that depend on Onyx-driven React re-renders.

waitFor from @testing-library/react polls a callback on a timer interval. When the test environment uses fake timers (common in <App /> integration tests that need to control setTimeout/setInterval), waitFor's internal polling and Jest's fake timer advancement can interact such that Onyx's batched update queue — which schedules React re-renders via microtasks or deferred callbacks — never gets flushed between waitFor poll ticks. The DOM never reaches the expected state, the callback never passes, and waitFor keeps polling until the 120s Jest process-level timeout terminates the test with no assertion error, only a timeout.

Important caveat from the evidence: The issue body notes that PR #92587 applied waitForBatchedUpdates to fix a prior occurrence and the test still timed out on main afterward. This means the two remaining waitFor usages at ~lines 456 and ~490 are the remaining candidates — but it also means a single waitForBatchedUpdates replacement may not be sufficient if additional async cycles are involved (Onyx write → subscriber → re-render → effect → second Onyx write → second re-render). Multiple await waitForBatchedUpdates() calls may be required.

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

File: tests/ui/UnreadIndicatorsTest.tsx

Step 1: Open the actual file and locate the waitFor usages near lines 456 and 490.

Since no source is available in the investigation context, the implementer must read the actual file. The pattern to find is:

// Pattern A (from issue description — what PR #92587 replaced elsewhere):
await waitFor(() => expect(someElement).toBeSomething());

// Pattern B (variant with act):
await act(async () => {
  await waitFor(() => expect(someElement).toBeSomething());
});

Step 2: Replace each occurrence with flush-then-assert.

The replacement pattern, consistent with PR #92587's already-merged fix:

// Before (at ~line 456, exact code TBD from reading the file):
await waitFor(() => expect(<someElement>).<someMatcher>());

// After:
await waitForBatchedUpdates();
await waitForBatchedUpdates(); // second flush if Onyx triggers a follow-on write
expect(<someElement>).<someMatcher>();

// Before (at ~line 490, exact code TBD from reading the file):
await waitFor(() => expect(<someElement>).<someMatcher>());

// After:
await waitForBatchedUpdates();
await waitForBatchedUpdates();
expect(<someElement>).<someMatcher>();

Why two waitForBatchedUpdates calls? The test navigates through a full <App />, which triggers: navigation state update → Onyx read for report data → Onyx write for lastReadTime → subscriber notification → re-render → possibly a second Onyx write for unreadCount. Each write-notify-render cycle is one "batch." A single flush may not cover the full chain. Using two calls matches the pattern already established elsewhere in this test file (per the issue's reference to related fix PRs) and is safe because a no-op extra flush has zero side effects.

Required imports: Verify waitForBatchedUpdates is already imported. It typically comes from:

import {waitForBatchedUpdates} from '../utils/TestHelper';
// or
import waitForBatchedUpdates from '@libs/waitForBatchedUpdates';

If not present, add the appropriate import. Remove the waitFor import from @testing-library/react if it is no longer used elsewhere in the file after this change.

Why this works: waitForBatchedUpdates resolves after all pending Onyx subscriber notifications and their React re-renders have been processed synchronously. The synchronous expect(...) after it cannot fail due to timing — the state is already committed to the DOM. This eliminates the polling loop entirely.

Why the test will not silently regress: If waitForBatchedUpdates does not flush enough cycles, the test will fail with a clear assertion error on the synchronous expect(...) line — not a silent 120s timeout. This makes any remaining issue immediately diagnosable.

What alternative solutions did you explore?

1. Increasing the Jest timeout (jest.setTimeout(240000)).
Masks the symptom, doubles CI resource consumption, and does not fix the root cause. The timeout will simply reappear at 240s under heavier load. Rejected.

2. Keeping waitFor but wrapping with act.
This was the prior pattern PR #92587 replaced. act flushes React-internal state but does not guarantee Onyx's subscriber notification queue has fully propagated before the inner callback is evaluated. Less reliable than an explicit flush. Rejected.

3. Splitting the monolithic <App /> test into smaller unit tests.
The root issue is a polling/flush incompatibility, not the test architecture per se. Splitting would be a significant refactor with high regression risk and is out of scope for a targeted flake fix. Worth considering separately for long-term test health, but not proposed here.

On PR #92587 as evidence: The critique correctly notes that PR #92587 did not stop the timeout — the test still timed out after its merge. This is cited correctly in the issue body. This does not invalidate waitForBatchedUpdates as the right tool; it confirms that #92587's fix was incomplete (only one location was fixed, two remained). The proposed fix targets the two remaining locations, which is the logical completion of #92587's approach. If the test still times out after fixing both remaining waitFor calls, the failure will manifest as an assertion error (not a silent timeout), immediately identifying any further missing flush.

[MelvinBot]

Issue Analysis

Root Cause: This is a timeout-budget flake, not an assertion failure. The test mounts a full <App /> and the whole file's per-test budget is capped at 120s (tests/ui/UnreadIndicatorsTest.tsx:37). Under loaded CI runners (job 7), the mount + navigation + scroll-effect draining occasionally exceeds 120s and Jest aborts with Exceeded timeout of 120000 ms. The failing assertion was already fixed by #92587 — the badge check now uses waitForBatchedUpdates() + a synchronous assert (UnreadIndicatorsTest.tsx:286-288), so what remains is purely the wall-clock budget plus residual act()-drain stalls during chat navigation.

Recommended Fix: Raise the file's jest.setTimeout to give the full-<App /> mount headroom under CI contention, and reduce act()-drain stalls by using fireEvent + waitForBatchedUpdates() for navigation instead of waitForBatchedUpdatesWithAct(). This is exactly what the already-open PR #93830 does.

⚠️ One correction to the issue description: the "two other tests still using the old waitFor badge pattern (~lines 456, 490)" are both inside the xit(...) block at UnreadIndicatorsTest.tsx:428 (and a second xit at line 527). Because those tests are skipped, their waitFor calls never execute and cannot contribute to the timeout. Converting them is good for consistency, but the effective fix for the active "Display bold in the LHN…" test is the timeout raise + the act()-drain reduction in navigation — not the line 456/490 changes.

Investigation details

Affected test: UnreadIndicatorsTest.tsx:244-289 — Display bold in the LHN for unread chat and new line indicator above the chat message when we navigate to it.

Why it's a timeout flake, not an assertion flake:

• The CI error is Exceeded timeout of 120000 ms for a test, which Jest emits when a test promise never resolves within jest.setTimeout. An assertion failure would instead print a diff.
• The badge assertion most likely to hang under waitFor + act was already rewritten in [No QA] Fix flaky ContactMethodsPage and UnreadIndicators tests #92587 to a synchronous check after waitForBatchedUpdates() (lines 286-288). The comment on lines 284-285 explicitly notes waitFor "can hang under heavy CI load while draining scroll-triggered effects."

Alternative explanation considered: a genuine logic hang (e.g., a never-resolving promise in navigation). Ruled out as the primary cause because the same test passes locally in ~9–43s and only times out on loaded runners — consistent with a wall-clock budget overrun, not a deterministic deadlock.

PR #93830 (open) does three things:

1. jest.setTimeout 120s → 240s — the actual safety-net fix.
2. Replaces the remaining waitFor(() => expect(isNewMessagesBadgeVisible())...) calls at lines 456/490 — consistency only; these live in skipped xit tests.
3. Adds navigateToSidebarOptionWithoutAct() (fireEvent + waitForBatchedUpdates()) to cut act()-drain stalls during chat navigation — this is the part that meaningfully reduces runtime for the active test.

---

Next Steps for Contributor+ team:
A fix PR is already open and in review: #93830. The analysis above validates its approach; the only refinement worth noting is that the line 456/490 changes are consistency-only (skipped tests), so the timeout raise + navigateToSidebarOptionWithoutAct() carry the actual stabilization.

[melvin-bot]

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

[melvin-bot]

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