Skip to content

Commit 41a3ee5

Browse files
committed
fix(frontend): repair lint and test failures blocking CI
`pnpm lint` (and therefore `pnpm build`) failed on main, and 19 unit tests failed alongside it. All of these predate the current branch. Lint: - `aiAssistant.test.ts` did not parse: the last `it()` in the abort-signal suite was never closed, so its `});` closed the `it` and left the `describe` open. Added the missing brace. - `apiSchemas.ts` stapled `status` and `response` onto a plain `Error` through two `any` casts. Replaced with an `HttpResponseError` class that declares both as typed readonly fields. - `apiSchemas.test.ts` had an unused `startTime` and an `as any` on a `setTimeout` spy. - `OfflineStatusBanner.tsx` kept a `pendingCount` state nothing read and two optimistic-update callbacks nothing called. The queued count is owned by `offlineMessageQueue` and published by `useChat`; the banner only mirrors it, so the local copies were redundant. Removed, along with the imports they were the only users of. - `OfflineStatusBanner.test.tsx` imported `renderHook` without using it. Two real bugs surfaced while fixing the above: - `withRetry` decided status retries with `error instanceof Response`. The value thrown by `fetchWithRetry` is an `Error`, never a `Response`, so that arm was dead and 500/503/429 responses were never retried — the `any` casts were what hid the status from the check. It now tests `HttpResponseError` (keeping the `Response` arm for callers that throw one directly). An explicit `AbortError` guard was also added, so an aborted request is never retried regardless of a caller's `retryableErrors`. - `OfflineStatusBanner` ignored `wasOffline`, announcing a reconnect only when it observed the offline render itself. Mounting after the connection was already back skipped the toast and never called `resetWasOffline`, leaving the hook's latch set forever. The reconnect branch now also fires on a latched `wasOffline` and consumes the latch immediately. Tests: the failures were fake-timer deadlocks. Suites installed `vi.useFakeTimers()` and then awaited code that sleeps between retries, or polled with `waitFor` while the clock was frozen — so the promise never settled and the test hit its 15s timeout. Fixed by draining timers with `vi.runAllTimersAsync()`, attaching rejection handlers before draining (an unhandled rejection otherwise escapes), and replacing frozen-clock `waitFor` calls with assertions made directly after an explicit `advanceTimersByTime`. `OfflineStatusBanner`'s pending-count test also pulled from `getQueuedMessageCount`, which the component never reads; it now drives the subscription the component actually listens to. pnpm typecheck, pnpm lint, pnpm build and pnpm test:unit (805 tests) all pass.
1 parent c2d8ada commit 41a3ee5

5 files changed

Lines changed: 191 additions & 130 deletions

File tree

Lines changed: 76 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
2-
import { renderHook, act } from '@testing-library/react';
3-
import { render, screen, waitFor } from '@testing-library/react';
2+
import { act, render, screen } from '@testing-library/react';
43
import OfflineStatusBanner from './OfflineStatusBanner';
5-
import * as offlineMessageQueue from '@/lib/offlineMessageQueue';
4+
import { useOnlineStatus } from '@/hooks/useOnlineStatus';
5+
import { subscribeToQueuedMessageCount } from '@/lib/offlineMessageQueue';
66

77
// Mock dependencies
88
vi.mock('@/hooks/useOnlineStatus', () => ({
@@ -26,14 +26,42 @@ vi.mock('@/lib/offlineStatusSchema', () => ({
2626
}));
2727

2828
vi.mock('@/lib/offlineMessageQueue', () => ({
29-
subscribeToQueuedMessageCount: vi.fn(),
29+
subscribeToQueuedMessageCount: vi.fn(() => () => {}),
3030
setQueuedMessageCount: vi.fn(),
3131
getQueuedMessageCount: vi.fn(() => 0),
3232
}));
3333

34+
const mockedUseOnlineStatus = vi.mocked(useOnlineStatus);
35+
const mockedSubscribe = vi.mocked(subscribeToQueuedMessageCount);
36+
37+
/** Elapse the 300ms initial loading gate so it cannot mask later renders. */
38+
function settleLoadingGate() {
39+
act(() => {
40+
vi.advanceTimersByTime(300);
41+
});
42+
}
43+
44+
/** Point `useOnlineStatus` at a value the test can flip between renders. */
45+
function stubOnlineStatus(getIsOnline: () => boolean, wasOffline = false) {
46+
const resetWasOffline = vi.fn();
47+
mockedUseOnlineStatus.mockImplementation(() => ({
48+
get isOnline() {
49+
return getIsOnline();
50+
},
51+
wasOffline,
52+
resetWasOffline,
53+
}));
54+
return resetWasOffline;
55+
}
56+
3457
describe('OfflineStatusBanner - Optimistic UI Updates', () => {
3558
beforeEach(() => {
3659
vi.clearAllMocks();
60+
mockedSubscribe.mockImplementation(() => () => {});
61+
// Fake timers are required for the 300ms loading skeleton and the 500ms
62+
// reconnect dismissal. Assertions below are made directly after an explicit
63+
// `advanceTimersByTime` rather than through `waitFor`, whose polling never
64+
// fires while the clock is frozen.
3765
vi.useFakeTimers();
3866
});
3967

@@ -42,138 +70,104 @@ describe('OfflineStatusBanner - Optimistic UI Updates', () => {
4270
vi.useRealTimers();
4371
});
4472

45-
it('should show banner immediately when going offline', async () => {
46-
const { useOnlineStatus } = await import('@/hooks/useOnlineStatus');
47-
(useOnlineStatus as any).mockReturnValue({
48-
isOnline: false,
49-
wasOffline: false,
50-
resetWasOffline: vi.fn(),
51-
});
73+
it('should show banner immediately when going offline', () => {
74+
stubOnlineStatus(() => false);
5275

5376
render(<OfflineStatusBanner />);
5477

55-
await waitFor(() => {
56-
expect(screen.getByRole('status')).toBeInTheDocument();
57-
});
58-
78+
expect(screen.getByRole('status')).toBeInTheDocument();
5979
expect(screen.getByText(/You are offline/i)).toBeInTheDocument();
6080
});
6181

62-
it('should show reconnecting state when coming back online', async () => {
63-
const { useOnlineStatus } = await import('@/hooks/useOnlineStatus');
82+
it('should show reconnecting state when coming back online', () => {
6483
let isOnline = false;
65-
66-
(useOnlineStatus as any).mockImplementation(() => ({
67-
get isOnline() { return isOnline; },
68-
wasOffline: true,
69-
resetWasOffline: vi.fn(),
70-
}));
84+
stubOnlineStatus(() => isOnline, true);
7185

7286
const { rerender } = render(<OfflineStatusBanner />);
73-
74-
await waitFor(() => {
75-
expect(screen.getByText(/You are offline/i)).toBeInTheDocument();
76-
});
87+
settleLoadingGate();
88+
expect(screen.getByText(/You are offline/i)).toBeInTheDocument();
7789

7890
// Simulate coming back online
7991
isOnline = true;
8092
rerender(<OfflineStatusBanner />);
8193

82-
await waitFor(() => {
83-
expect(screen.getByText(/Reconnecting/i)).toBeInTheDocument();
84-
});
94+
expect(screen.getByText(/Reconnecting/i)).toBeInTheDocument();
8595
});
8696

87-
it('should display optimistic pending count', async () => {
88-
const { useOnlineStatus } = await import('@/hooks/useOnlineStatus');
89-
(useOnlineStatus as any).mockReturnValue({
90-
isOnline: false,
91-
wasOffline: false,
92-
resetWasOffline: vi.fn(),
97+
it('should display optimistic pending count', () => {
98+
stubOnlineStatus(() => false);
99+
// The count is pushed by `offlineMessageQueue` subscribers, not pulled.
100+
mockedSubscribe.mockImplementation((listener) => {
101+
listener(3);
102+
return () => {};
93103
});
94104

95-
(offlineMessageQueue.getQueuedMessageCount as any).mockReturnValue(3);
96-
97105
render(<OfflineStatusBanner />);
98106

99-
await waitFor(() => {
100-
expect(screen.getByText(/3 messages waiting to send/i)).toBeInTheDocument();
107+
expect(screen.getByText(/3 messages waiting to send/i)).toBeInTheDocument();
108+
});
109+
110+
it('should pluralise a single pending message', () => {
111+
stubOnlineStatus(() => false);
112+
mockedSubscribe.mockImplementation((listener) => {
113+
listener(1);
114+
return () => {};
101115
});
116+
117+
render(<OfflineStatusBanner />);
118+
119+
expect(screen.getByText(/1 message waiting to send/i)).toBeInTheDocument();
102120
});
103121

104-
it('should hide banner after reconnection delay', async () => {
105-
const { useOnlineStatus } = await import('@/hooks/useOnlineStatus');
122+
it('should hide banner after reconnection delay', () => {
106123
let isOnline = false;
107-
108-
(useOnlineStatus as any).mockImplementation(() => ({
109-
get isOnline() { return isOnline; },
110-
wasOffline: true,
111-
resetWasOffline: vi.fn(),
112-
}));
124+
const resetWasOffline = stubOnlineStatus(() => isOnline, true);
113125

114126
const { rerender } = render(<OfflineStatusBanner />);
115-
116-
await waitFor(() => {
117-
expect(screen.getByRole('status')).toBeInTheDocument();
118-
});
127+
settleLoadingGate();
128+
expect(screen.getByRole('status')).toBeInTheDocument();
119129

120130
// Simulate coming back online
121131
isOnline = true;
122132
rerender(<OfflineStatusBanner />);
133+
expect(screen.getByRole('status')).toBeInTheDocument();
123134

124135
act(() => {
125136
vi.advanceTimersByTime(500);
126137
});
127138

128-
await waitFor(() => {
129-
expect(screen.queryByRole('status')).not.toBeInTheDocument();
130-
});
139+
expect(screen.queryByRole('status')).not.toBeInTheDocument();
140+
expect(resetWasOffline).toHaveBeenCalled();
131141
});
132142

133-
it('should update aria-label based on connection state', async () => {
134-
const { useOnlineStatus } = await import('@/hooks/useOnlineStatus');
143+
it('should update aria-label based on connection state', () => {
135144
let isOnline = false;
136-
137-
(useOnlineStatus as any).mockImplementation(() => ({
138-
get isOnline() { return isOnline; },
139-
wasOffline: true,
140-
resetWasOffline: vi.fn(),
141-
}));
145+
stubOnlineStatus(() => isOnline, true);
142146

143147
const { rerender } = render(<OfflineStatusBanner />);
144-
145-
await waitFor(() => {
146-
expect(screen.getByLabelText('Offline status')).toBeInTheDocument();
147-
});
148+
settleLoadingGate();
149+
expect(screen.getByLabelText('Offline status')).toBeInTheDocument();
148150

149151
isOnline = true;
150152
rerender(<OfflineStatusBanner />);
151153

152-
await waitFor(() => {
153-
expect(screen.getByLabelText('Reconnecting')).toBeInTheDocument();
154-
});
154+
expect(screen.getByLabelText('Reconnecting')).toBeInTheDocument();
155155
});
156156

157-
it('should show loading skeleton initially when online', async () => {
158-
const { useOnlineStatus } = await import('@/hooks/useOnlineStatus');
159-
(useOnlineStatus as any).mockReturnValue({
160-
isOnline: true,
161-
wasOffline: false,
162-
resetWasOffline: vi.fn(),
163-
});
157+
it('should show loading skeleton initially when online', () => {
158+
stubOnlineStatus(() => true);
164159

165160
render(<OfflineStatusBanner />);
166161

167162
// Should show loading skeleton initially
168-
const skeleton = document.querySelector('[aria-hidden="true"]');
169-
expect(skeleton).toBeInTheDocument();
163+
expect(document.querySelector('[aria-hidden="true"]')).toBeInTheDocument();
170164

171165
act(() => {
172166
vi.advanceTimersByTime(300);
173167
});
174168

175-
await waitFor(() => {
176-
expect(document.querySelector('[aria-hidden="true"]')).not.toBeInTheDocument();
177-
});
169+
expect(
170+
document.querySelector('[aria-hidden="true"]'),
171+
).not.toBeInTheDocument();
178172
});
179173
});

Dechat/dex_with_fiat_frontend/src/components/OfflineStatusBanner.tsx

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
'use client';
22

3-
import { useEffect, useState, useCallback, useRef } from 'react';
3+
import { useEffect, useState, useRef } from 'react';
44
import { AlertTriangle, WifiOff } from 'lucide-react';
55
import { useOnlineStatus } from '@/hooks/useOnlineStatus';
66
import { useToast } from '@/hooks/useToast';
77
import { offlineStatusToastSchema } from '@/lib/offlineStatusSchema';
8-
import { subscribeToQueuedMessageCount, setQueuedMessageCount, getQueuedMessageCount } from '@/lib/offlineMessageQueue';
8+
import { subscribeToQueuedMessageCount } from '@/lib/offlineMessageQueue';
99

1010
/**
1111
* Offline Status Banner Component
@@ -18,7 +18,6 @@ export default function OfflineStatusBanner() {
1818
const { addToast } = useToast();
1919
const [showBanner, setShowBanner] = useState(false);
2020
const [isLoading, setIsLoading] = useState(true);
21-
const [pendingCount, setPendingCount] = useState(0);
2221
const [optimisticPendingCount, setOptimisticPendingCount] = useState(0);
2322
const [isReconnecting, setIsReconnecting] = useState(false);
2423
const previousOnlineState = useRef<boolean>(true);
@@ -31,33 +30,27 @@ export default function OfflineStatusBanner() {
3130
return () => clearTimeout(timer);
3231
}, []);
3332

33+
// The queue count is owned by `offlineMessageQueue` and published by
34+
// `useChat` as sends are queued and drained; this component only mirrors it.
3435
useEffect(() => {
35-
return subscribeToQueuedMessageCount((count) => {
36-
setPendingCount(count);
37-
setOptimisticPendingCount(count);
38-
});
39-
}, []);
40-
41-
// Optimistic update: increment pending count immediately when message is queued
42-
const optimisticallyIncrementPending = useCallback(() => {
43-
setOptimisticPendingCount((prev: number) => prev + 1);
44-
setQueuedMessageCount(getQueuedMessageCount() + 1);
45-
}, []);
46-
47-
// Optimistic update: decrement pending count immediately when message is sent
48-
const optimisticallyDecrementPending = useCallback(() => {
49-
setOptimisticPendingCount((prev: number) => Math.max(0, prev - 1));
50-
setQueuedMessageCount(Math.max(0, getQueuedMessageCount() - 1));
36+
return subscribeToQueuedMessageCount(setOptimisticPendingCount);
5137
}, []);
5238

5339
useEffect(() => {
40+
// A reconnect is either observed live (we saw the offline render) or
41+
// reported after the fact by `wasOffline` — the latter happens when this
42+
// banner mounts only once the connection is already back. Without the
43+
// `wasOffline` arm the toast is skipped and the latch is never reset, so
44+
// the hook keeps reporting a reconnect that was never announced.
45+
const cameBackOnline = isOnline && (!previousOnlineState.current || wasOffline);
46+
5447
// Optimistic UI: Show banner immediately when going offline
5548
if (!isOnline && previousOnlineState.current) {
5649
setShowBanner(true);
5750
setIsReconnecting(false);
5851
}
5952
// Optimistic UI: Hide banner immediately when coming back online
60-
else if (isOnline && !previousOnlineState.current) {
53+
else if (cameBackOnline) {
6154
setIsReconnecting(true);
6255
// Show toast when coming back online
6356
const toastOptions = {
@@ -82,11 +75,14 @@ export default function OfflineStatusBanner() {
8275
addToast(errorMessage);
8376
}
8477

78+
// Consume the latch straight away, so the reconnect is announced exactly
79+
// once rather than on every subsequent render.
80+
resetWasOffline();
81+
8582
// Optimistically hide banner after short delay
8683
setTimeout(() => {
8784
setShowBanner(false);
8885
setIsReconnecting(false);
89-
resetWasOffline();
9086
}, 500);
9187
}
9288

Dechat/dex_with_fiat_frontend/src/lib/aiAssistant.test.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,7 @@ describe('AIAssistant abort signal support', () => {
240240

241241
expect(toastAddMock).toHaveBeenCalled();
242242
consoleErrorSpy.mockRestore();
243-
244-
243+
});
245244
});
246245

247246
/**
@@ -489,7 +488,11 @@ describe('aiAssistant request retry with exponential backoff', () => {
489488
}),
490489
);
491490

492-
const result = await assistant.analyzeUserMessage('hello');
491+
// Fake timers are installed, so the backoff sleep between attempts only
492+
// resolves once the pending timers are drained.
493+
const promise = assistant.analyzeUserMessage('hello');
494+
await vi.runAllTimersAsync();
495+
const result = await promise;
493496

494497
expect(result.intent).toBe('query');
495498
expect(fetch).toHaveBeenCalledTimes(2);
@@ -503,7 +506,9 @@ describe('aiAssistant request retry with exponential backoff', () => {
503506

504507
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
505508

506-
const result = await assistant.analyzeUserMessage('hello');
509+
const promise = assistant.analyzeUserMessage('hello');
510+
await vi.runAllTimersAsync();
511+
const result = await promise;
507512

508513
expect(result.intent).toBe('unknown');
509514
expect(fetch).toHaveBeenCalledTimes(4); // initial + 3 retries
@@ -582,7 +587,9 @@ describe('aiAssistant request retry with exponential backoff', () => {
582587
}),
583588
);
584589

585-
const result = await assistant.generateFollowUpQuestion('query', ['name']);
590+
const promise = assistant.generateFollowUpQuestion('query', ['name']);
591+
await vi.runAllTimersAsync();
592+
const result = await promise;
586593

587594
expect(result).toBe('What is your name?');
588595
expect(fetch).toHaveBeenCalledTimes(2);

0 commit comments

Comments
 (0)