From ecb748b9e38300e2bc45aa779f61e7b14febe3ca Mon Sep 17 00:00:00 2001 From: Jack Kingsman Date: Mon, 23 Feb 2026 22:28:09 -0800 Subject: [PATCH] Drop out crappy tests, and improve quality overall --- frontend/src/App.tsx | 12 +- frontend/src/components/SettingsModal.tsx | 28 +- frontend/src/test/integration.test.ts | 176 +--------- frontend/src/test/messageParser.test.ts | 16 - frontend/src/test/radioPresets.test.ts | 44 +-- frontend/src/test/repeaterMode.test.ts | 90 +----- frontend/src/test/unreadCounts.test.ts | 219 +------------ .../src/test/useConversationMessages.test.ts | 72 ----- frontend/src/test/websocket.test.ts | 264 --------------- frontend/src/utils/messageParser.ts | 8 + frontend/src/utils/radioPresets.ts | 41 +++ tests/test_bot.py | 29 +- tests/test_decoder.py | 38 ++- tests/test_event_handlers.py | 50 --- tests/test_radio_sync.py | 28 -- tests/test_repository.py | 302 +++++++----------- tests/test_websocket_route.py | 55 ---- 17 files changed, 226 insertions(+), 1246 deletions(-) delete mode 100644 frontend/src/test/websocket.test.ts create mode 100644 frontend/src/utils/radioPresets.ts diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 15e5c5b..b6fe325 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -50,6 +50,7 @@ import { Sheet, SheetContent, SheetHeader, SheetTitle } from './components/ui/sh import { Toaster, toast } from './components/ui/sonner'; import { getStateKey } from './utils/conversationState'; import { appendRawPacketUnique } from './utils/rawPacketIdentity'; +import { messageContainsMention } from './utils/messageParser'; import { cn } from '@/lib/utils'; import type { Contact, Conversation, HealthStatus, Message, MessagePath, RawPacket } from './types'; @@ -110,13 +111,10 @@ export function App() { }, [config?.name]); // Check if a message mentions the user - const checkMention = useCallback((text: string): boolean => { - const name = myNameRef.current; - if (!name) return false; - const escaped = name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const mentionPattern = new RegExp(`@\\[${escaped}\\]`, 'i'); - return mentionPattern.test(text); - }, []); + const checkMention = useCallback( + (text: string): boolean => messageContainsMention(text, myNameRef.current), + [] + ); // useContactsAndChannels is called first — it uses the ref bridge for setActiveConversation const { diff --git a/frontend/src/components/SettingsModal.tsx b/frontend/src/components/SettingsModal.tsx index c765595..9eceff6 100644 --- a/frontend/src/components/SettingsModal.tsx +++ b/frontend/src/components/SettingsModal.tsx @@ -24,33 +24,7 @@ import { getReopenLastConversationEnabled, setReopenLastConversationEnabled, } from '../utils/lastViewedConversation'; - -// Radio presets for common configurations -interface RadioPreset { - name: string; - freq: number; - bw: number; - sf: number; - cr: number; -} - -const RADIO_PRESETS: RadioPreset[] = [ - { name: 'USA/Canada', freq: 910.525, bw: 62.5, sf: 7, cr: 5 }, - { name: 'Australia', freq: 915.8, bw: 250, sf: 10, cr: 5 }, - { name: 'Australia (narrow)', freq: 916.575, bw: 62.5, sf: 7, cr: 8 }, - { name: 'Australia SA, WA', freq: 923.125, bw: 62.5, sf: 8, cr: 8 }, - { name: 'Australia QLD', freq: 923.125, bw: 62.5, sf: 8, cr: 5 }, - { name: 'New Zealand', freq: 917.375, bw: 250, sf: 11, cr: 5 }, - { name: 'New Zealand (narrow)', freq: 917.375, bw: 62.5, sf: 7, cr: 5 }, - { name: 'EU/UK/Switzerland Long Range', freq: 869.525, bw: 250, sf: 11, cr: 5 }, - { name: 'EU/UK/Switzerland Medium Range', freq: 869.525, bw: 250, sf: 10, cr: 5 }, - { name: 'EU/UK/Switzerland Narrow', freq: 869.618, bw: 62.5, sf: 8, cr: 8 }, - { name: 'Czech Republic (Narrow)', freq: 869.432, bw: 62.5, sf: 7, cr: 5 }, - { name: 'EU 433MHz Long Range', freq: 433.65, bw: 250, sf: 11, cr: 5 }, - { name: 'Portugal 433MHz', freq: 433.375, bw: 62.5, sf: 9, cr: 6 }, - { name: 'Portugal 868MHz', freq: 869.618, bw: 62.5, sf: 7, cr: 6 }, - { name: 'Vietnam', freq: 920.25, bw: 250, sf: 11, cr: 5 }, -]; +import { RADIO_PRESETS } from '../utils/radioPresets'; // Import for local use + re-export so existing imports from this file still work import { diff --git a/frontend/src/test/integration.test.ts b/frontend/src/test/integration.test.ts index 3aaeff4..9e616b4 100644 --- a/frontend/src/test/integration.test.ts +++ b/frontend/src/test/integration.test.ts @@ -1,8 +1,8 @@ /** - * Integration tests for WebSocket event handling. + * Integration tests for message deduplication and content key contracts. * - * These tests verify that WebSocket events (as produced by the backend) - * are correctly processed by the frontend state handlers. + * These tests verify that the real messageCache and getMessageContentKey + * functions work correctly with realistic WebSocket event data from fixtures. * * The fixtures in fixtures/websocket_events.json define the contract * between backend and frontend - both sides test against the same data. @@ -13,27 +13,22 @@ import fixtures from './fixtures/websocket_events.json'; import { getMessageContentKey } from '../hooks/useConversationMessages'; import { getStateKey } from '../utils/conversationState'; import * as messageCache from '../messageCache'; -import type { Message, Contact, Channel } from '../types'; +import type { Message } from '../types'; /** - * Simulate the WebSocket message handler from App.tsx. - * This is the core logic we're testing. + * Minimal state for testing message dedup and unread logic. + * Uses real messageCache.addMessage and real getMessageContentKey. */ interface MockState { messages: Message[]; - contacts: Contact[]; - channels: Channel[]; unreadCounts: Record; lastMessageTimes: Record; - /** Active-conversation dedup (mirrors useConversationMessages internal set) */ seenActiveContent: Set; } function createMockState(): MockState { return { messages: [], - contacts: [], - channels: [], unreadCounts: {}, lastMessageTimes: {}, seenActiveContent: new Set(), @@ -41,11 +36,8 @@ function createMockState(): MockState { } /** - * Simulate handling a message WebSocket event. - * Mirrors the logic in App.tsx onMessage handler. - * - * Non-active conversation dedup uses messageCache.addMessage (single source of truth). - * Active conversation dedup uses seenActiveContent (mirrors useConversationMessages). + * Simulate the message handling path from App.tsx. + * Uses real getMessageContentKey and real messageCache.addMessage for dedup. */ function handleMessageEvent( state: MockState, @@ -56,11 +48,9 @@ function handleMessageEvent( let added = false; let unreadIncremented = false; - // Check if message is for active conversation const isForActiveConversation = activeConversationKey !== null && msg.conversation_key === activeConversationKey; - // Add to messages if for active conversation (with deduplication) if (isForActiveConversation) { if (!state.seenActiveContent.has(contentKey)) { state.seenActiveContent.add(contentKey); @@ -69,7 +59,6 @@ function handleMessageEvent( } } - // Update last message time const stateKey = msg.type === 'CHAN' ? getStateKey('channel', msg.conversation_key) @@ -77,8 +66,6 @@ function handleMessageEvent( state.lastMessageTimes[stateKey] = msg.received_at; - // Increment unread if not for active conversation and not outgoing - // Uses messageCache.addMessage as single source of truth for dedup if (!isForActiveConversation) { const isNew = messageCache.addMessage(msg.conversation_key, msg, contentKey); if (!msg.outgoing && isNew) { @@ -90,32 +77,6 @@ function handleMessageEvent( return { added, unreadIncremented }; } -/** - * Simulate handling a contact WebSocket event. - */ -function handleContactEvent(state: MockState, contact: Contact): void { - const idx = state.contacts.findIndex((c) => c.public_key === contact.public_key); - if (idx >= 0) { - // Update existing contact - state.contacts[idx] = { ...state.contacts[idx], ...contact }; - } else { - // Add new contact - state.contacts.push(contact); - } -} - -/** - * Simulate handling a message_acked WebSocket event. - */ -function handleMessageAckedEvent(state: MockState, messageId: number, ackCount: number): boolean { - const idx = state.messages.findIndex((m) => m.id === messageId); - if (idx >= 0) { - state.messages[idx] = { ...state.messages[idx], acked: ackCount }; - return true; - } - return false; -} - // Clear messageCache between tests to avoid cross-test contamination beforeEach(() => { messageCache.clear(); @@ -177,16 +138,12 @@ describe('Integration: Channel Message Events', () => { }); describe('Integration: Duplicate Message Handling', () => { - // Note: duplicate_channel_message fixture references the same packet data as channel_message - it('deduplicates messages by content when adding to list', () => { const state = createMockState(); - // Use channel_message fixture data since duplicate_channel_message references same packet const msgData = fixtures.channel_message.expected_ws_event.data; const msg1 = { ...msgData, id: 1, received_at: 1700000000 } as unknown as Message; const msg2 = { ...msgData, id: 2, received_at: 1700000001 } as unknown as Message; - // Both arrive for active conversation const result1 = handleMessageEvent(state, msg1, msg1.conversation_key); const result2 = handleMessageEvent(state, msg2, msg2.conversation_key); @@ -201,7 +158,6 @@ describe('Integration: Duplicate Message Handling', () => { const msg1 = { ...msgData, id: 1, received_at: 1700000000 } as unknown as Message; const msg2 = { ...msgData, id: 2, received_at: 1700000001 } as unknown as Message; - // Both arrive for non-active conversation const result1 = handleMessageEvent(state, msg1, 'other_conversation'); const result2 = handleMessageEvent(state, msg2, 'other_conversation'); @@ -213,120 +169,6 @@ describe('Integration: Duplicate Message Handling', () => { }); }); -describe('Integration: Contact/Advertisement Events', () => { - const fixture = fixtures.advertisement_with_gps; - - it('creates new contact from advertisement', () => { - const state = createMockState(); - const contact = fixture.expected_ws_event.data as unknown as Contact; - - handleContactEvent(state, contact); - - expect(state.contacts).toHaveLength(1); - expect(state.contacts[0].public_key).toBe(contact.public_key); - expect(state.contacts[0].name).toBe('Can O Mesh 2 🥫'); - expect(state.contacts[0].type).toBe(2); // Repeater - expect(state.contacts[0].lat).toBeCloseTo(49.02056, 4); - expect(state.contacts[0].lon).toBeCloseTo(-123.82935, 4); - }); - - it('updates existing contact from advertisement', () => { - const state = createMockState(); - - // Add existing contact - state.contacts.push({ - public_key: fixture.expected_ws_event.data.public_key, - name: 'Old Name', - type: 0, - on_radio: false, - last_read_at: null, - } as Contact); - - // Process new advertisement - const contact = fixture.expected_ws_event.data as unknown as Contact; - handleContactEvent(state, contact); - - expect(state.contacts).toHaveLength(1); - expect(state.contacts[0].name).toBe('Can O Mesh 2 🥫'); // Updated - expect(state.contacts[0].type).toBe(2); // Updated - }); - - it('preserves contact GPS from chat node advertisement', () => { - const state = createMockState(); - const chatFixture = fixtures.advertisement_chat_node; - const contact = chatFixture.expected_ws_event.data as unknown as Contact; - - handleContactEvent(state, contact); - - expect(state.contacts[0].lat).toBeCloseTo(47.786038, 4); - expect(state.contacts[0].lon).toBeCloseTo(-122.344096, 4); - expect(state.contacts[0].type).toBe(1); // Chat node - }); -}); - -describe('Integration: ACK Events', () => { - const fixture = fixtures.message_acked; - - it('updates message ack count', () => { - const state = createMockState(); - - // Add a message that's waiting for ACK - state.messages.push({ - id: 42, - type: 'PRIV', - conversation_key: 'abc123', - text: 'Hello', - sender_timestamp: 1700000000, - received_at: 1700000000, - paths: null, - txt_type: 0, - signature: null, - outgoing: true, - acked: 0, - }); - - const ackData = fixture.expected_ws_event.data; - const updated = handleMessageAckedEvent(state, ackData.message_id, ackData.ack_count); - - expect(updated).toBe(true); - expect(state.messages[0].acked).toBe(1); - }); - - it('returns false for unknown message id', () => { - const state = createMockState(); - - const ackData = fixture.expected_ws_event.data; - const updated = handleMessageAckedEvent(state, ackData.message_id, ackData.ack_count); - - expect(updated).toBe(false); - }); - - it('updates to multiple ack count for flood echoes', () => { - const state = createMockState(); - - state.messages.push({ - id: 42, - type: 'CHAN', - conversation_key: 'channel123', - text: 'Hello', - sender_timestamp: 1700000000, - received_at: 1700000000, - paths: null, - txt_type: 0, - signature: null, - outgoing: true, - acked: 0, - }); - - // Multiple flood echoes - handleMessageAckedEvent(state, 42, 1); - handleMessageAckedEvent(state, 42, 2); - handleMessageAckedEvent(state, 42, 3); - - expect(state.messages[0].acked).toBe(3); - }); -}); - describe('Integration: No phantom unreads from mesh echoes (hitlist #8 regression)', () => { it('does not increment unread when a mesh echo arrives after many unique messages', () => { const state = createMockState(); @@ -360,7 +202,6 @@ describe('Integration: No phantom unreads from mesh echoes (hitlist #8 regressio expect(state.unreadCounts[stateKey]).toBe(MESSAGE_COUNT); // Now a mesh echo of msg-0 arrives (same content, different id). - // msg-0's key would have been evicted by the old 1000→500 prune. const echo: Message = { id: 9999, type: 'CHAN', @@ -419,7 +260,6 @@ describe('Integration: State Key Contract', () => { const stateKey = getStateKey('contact', publicKey); - // Contact state key uses full public key expect(stateKey).toBe(`contact-${publicKey}`); }); }); diff --git a/frontend/src/test/messageParser.test.ts b/frontend/src/test/messageParser.test.ts index afcff90..cde9690 100644 --- a/frontend/src/test/messageParser.test.ts +++ b/frontend/src/test/messageParser.test.ts @@ -7,7 +7,6 @@ import { describe, it, expect } from 'vitest'; import { parseSenderFromText, formatTime } from '../utils/messageParser'; -import { getStateKey } from '../utils/conversationState'; describe('parseSenderFromText', () => { it('extracts sender and content from "sender: message" format', () => { @@ -96,18 +95,3 @@ describe('formatTime', () => { expect(result).toMatch(/\d{1,2}:\d{2}/); // time portion }); }); - -describe('getStateKey', () => { - it('creates channel state key with full id', () => { - const key = getStateKey('channel', '5'); - - expect(key).toBe('channel-5'); - }); - - it('creates contact state key with full public key', () => { - const fullKey = 'abcdef123456789012345678901234567890'; - const key = getStateKey('contact', fullKey); - - expect(key).toBe(`contact-${fullKey}`); - }); -}); diff --git a/frontend/src/test/radioPresets.test.ts b/frontend/src/test/radioPresets.test.ts index a668dbe..e95d051 100644 --- a/frontend/src/test/radioPresets.test.ts +++ b/frontend/src/test/radioPresets.test.ts @@ -1,47 +1,5 @@ import { describe, it, expect } from 'vitest'; - -// Radio presets - duplicated from SettingsModal for testing -// In a real app, these would be in a shared constants file -interface RadioPreset { - name: string; - freq: number; - bw: number; - sf: number; - cr: number; -} - -const RADIO_PRESETS: RadioPreset[] = [ - { name: 'USA/Canada', freq: 910.525, bw: 62.5, sf: 7, cr: 5 }, - { name: 'Australia', freq: 915.8, bw: 250, sf: 10, cr: 5 }, - { name: 'Australia (narrow)', freq: 916.575, bw: 62.5, sf: 7, cr: 8 }, - { name: 'Australia SA, WA', freq: 923.125, bw: 62.5, sf: 8, cr: 8 }, - { name: 'Australia QLD', freq: 923.125, bw: 62.5, sf: 8, cr: 5 }, - { name: 'New Zealand', freq: 917.375, bw: 250, sf: 11, cr: 5 }, - { name: 'New Zealand (narrow)', freq: 917.375, bw: 62.5, sf: 7, cr: 5 }, - { name: 'EU/UK/Switzerland Long Range', freq: 869.525, bw: 250, sf: 11, cr: 5 }, - { name: 'EU/UK/Switzerland Medium Range', freq: 869.525, bw: 250, sf: 10, cr: 5 }, - { name: 'EU/UK/Switzerland Narrow', freq: 869.618, bw: 62.5, sf: 8, cr: 8 }, - { name: 'Czech Republic (Narrow)', freq: 869.432, bw: 62.5, sf: 7, cr: 5 }, - { name: 'EU 433MHz Long Range', freq: 433.65, bw: 250, sf: 11, cr: 5 }, - { name: 'Portugal 433MHz', freq: 433.375, bw: 62.5, sf: 9, cr: 6 }, - { name: 'Portugal 868MHz', freq: 869.618, bw: 62.5, sf: 7, cr: 6 }, - { name: 'Vietnam', freq: 920.25, bw: 250, sf: 11, cr: 5 }, -]; - -// Preset detection function - matches the logic in SettingsModal -function detectPreset(freq: number, bw: number, sf: number, cr: number): string { - for (const preset of RADIO_PRESETS) { - if (preset.freq === freq && preset.bw === bw && preset.sf === sf && preset.cr === cr) { - return preset.name; - } - } - return 'custom'; -} - -// Find preset by name -function findPreset(name: string): RadioPreset | undefined { - return RADIO_PRESETS.find((p) => p.name === name); -} +import { RADIO_PRESETS, detectPreset, findPreset } from '../utils/radioPresets'; describe('Radio Presets', () => { describe('detectPreset', () => { diff --git a/frontend/src/test/repeaterMode.test.ts b/frontend/src/test/repeaterMode.test.ts index 91a1a4c..5912031 100644 --- a/frontend/src/test/repeaterMode.test.ts +++ b/frontend/src/test/repeaterMode.test.ts @@ -1,16 +1,12 @@ /** * Tests for repeater-specific behavior. * - * These tests verify edge cases in repeater interactions that could easily - * regress if the code is modified: - * - * 1. Repeater messages should NOT have sender parsed from text (colons are common in CLI output) - * 2. Empty password field = guest login, password field with text = password login + * Verifies that CLI responses from repeaters would be mis-parsed by + * parseSenderFromText, motivating the repeater bypass in MessageList.tsx. */ import { describe, it, expect } from 'vitest'; import { parseSenderFromText } from '../utils/messageParser'; -import { CONTACT_TYPE_REPEATER } from '../types'; describe('Repeater message sender parsing', () => { /** @@ -19,8 +15,7 @@ describe('Repeater message sender parsing', () => { * "clock" as a sender name, breaking the display. * * The fix in MessageList.tsx is to check if the contact is a repeater and - * skip parseSenderFromText entirely. These tests document the expected - * behavior pattern. + * skip parseSenderFromText entirely. */ it('parseSenderFromText would incorrectly parse CLI responses with colons', () => { @@ -34,37 +29,7 @@ describe('Repeater message sender parsing', () => { // This would display as "clock" sent "2024-01-09 12:30:00" - WRONG! }); - it('repeater messages should bypass parsing entirely', () => { - // This documents the correct behavior: skip parsing for repeaters - const cliResponse = 'clock: 2024-01-09 12:30:00'; - const contactType = CONTACT_TYPE_REPEATER; - - // The pattern used in MessageList.tsx: - const isRepeater = contactType === CONTACT_TYPE_REPEATER; - const { sender, content } = isRepeater - ? { sender: null, content: cliResponse } - : parseSenderFromText(cliResponse); - - // Correct: full text preserved, no sender extracted - expect(sender).toBeNull(); - expect(content).toBe('clock: 2024-01-09 12:30:00'); - }); - - it('non-repeater messages still get sender parsed', () => { - const channelMessage = 'Alice: Hello everyone!'; - const contactType: number = 1; // client - - const isRepeater = contactType === CONTACT_TYPE_REPEATER; - const { sender, content } = isRepeater - ? { sender: null, content: channelMessage } - : parseSenderFromText(channelMessage); - - // Normal behavior: sender extracted - expect(sender).toBe('Alice'); - expect(content).toBe('Hello everyone!'); - }); - - it('handles various CLI response formats that would be mis-parsed', () => { + it('various CLI response formats are incorrectly parsed without repeater bypass', () => { const cliResponses = [ 'ver: 1.2.3', 'tx: 20 dBm', @@ -78,53 +43,6 @@ describe('Repeater message sender parsing', () => { // All of these would be incorrectly parsed without the repeater check const parsed = parseSenderFromText(response); expect(parsed.sender).not.toBeNull(); - - // But with repeater check, they're preserved - const isRepeater = true; - const { sender, content } = isRepeater - ? { sender: null, content: response } - : parseSenderFromText(response); - - expect(sender).toBeNull(); - expect(content).toBe(response); } }); }); - -describe('Repeater login behavior', () => { - /** - * Repeater login has two modes: - * - Empty password field = guest login (uses repeater's ACL permissions) - * - Password in field = admin login attempt - */ - - it('empty input results in empty password (guest login)', () => { - // This is the logic in MessageInput.tsx handleSubmit - const text = ''; - const trimmed = text.trim(); - - // Empty string is passed directly to onSend - expect(trimmed).toBe(''); - }); - - it('password is passed through unchanged', () => { - const text = 'mySecretPassword'; - const trimmed = text.trim(); - - expect(trimmed).toBe('mySecretPassword'); - }); - - it('whitespace-only input is treated as empty (guest login)', () => { - const text = ' '; - const trimmed = text.trim(); - - expect(trimmed).toBe(''); - }); - - it('password with surrounding whitespace is trimmed', () => { - const text = ' secret123 '; - const trimmed = text.trim(); - - expect(trimmed).toBe('secret123'); - }); -}); diff --git a/frontend/src/test/unreadCounts.test.ts b/frontend/src/test/unreadCounts.test.ts index 6f26670..eaa10fa 100644 --- a/frontend/src/test/unreadCounts.test.ts +++ b/frontend/src/test/unreadCounts.test.ts @@ -1,219 +1,14 @@ /** - * Tests for unread count tracking logic. + * Tests for the messageContainsMention utility function. * - * These tests verify the unread message counting behavior - * without involving React component rendering. + * The unread counting and lookup logic (shouldIncrementUnread, getUnreadCount) + * is tested through component-level and integration tests rather than + * re-implementing the logic locally. See appFavorites.test.tsx, sidebar.test.tsx, + * and integration.test.ts for those paths. */ import { describe, it, expect } from 'vitest'; -import type { Message, Conversation } from '../types'; - -/** - * Determine if a message should increment unread count. - * Extracted logic from App.tsx for testing. - */ -function shouldIncrementUnread( - msg: Message, - activeConversation: Conversation | null -): { key: string } | null { - // Only count incoming messages - if (msg.outgoing) { - return null; - } - - if (msg.type === 'CHAN' && msg.conversation_key) { - const key = `channel-${msg.conversation_key}`; - // Don't count if this channel is active - if (activeConversation?.type === 'channel' && activeConversation?.id === msg.conversation_key) { - return null; - } - return { key }; - } - - if (msg.type === 'PRIV' && msg.conversation_key) { - const key = `contact-${msg.conversation_key}`; - // Don't count if this contact is active - if (activeConversation?.type === 'contact' && activeConversation.id === msg.conversation_key) { - return null; - } - return { key }; - } - - return null; -} - -/** - * Get unread count for a conversation from the counts map. - * Extracted logic from Sidebar.tsx for testing. - */ -function getUnreadCount( - type: 'channel' | 'contact', - id: string, - unreadCounts: Record -): number { - return unreadCounts[`${type}-${id}`] || 0; -} - -describe('shouldIncrementUnread', () => { - const createMessage = (overrides: Partial): Message => ({ - id: 1, - type: 'CHAN', - conversation_key: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA0', // 32-char hex channel key - text: 'Test', - sender_timestamp: null, - received_at: Date.now(), - paths: null, - txt_type: 0, - signature: null, - outgoing: false, - acked: 0, - ...overrides, - }); - - it('returns key for incoming channel message when not viewing that channel', () => { - const msg = createMessage({ - type: 'CHAN', - conversation_key: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA3', - }); - const activeConversation: Conversation = { - type: 'channel', - id: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA5', - name: 'other', - }; - - const result = shouldIncrementUnread(msg, activeConversation); - - expect(result).toEqual({ key: 'channel-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA3' }); - }); - - it('returns null for incoming channel message when viewing that channel', () => { - const msg = createMessage({ - type: 'CHAN', - conversation_key: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA3', - }); - const activeConversation: Conversation = { - type: 'channel', - id: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA3', - name: '#test', - }; - - const result = shouldIncrementUnread(msg, activeConversation); - - expect(result).toBeNull(); - }); - - it('returns null for outgoing messages', () => { - const msg = createMessage({ - type: 'CHAN', - conversation_key: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA3', - outgoing: true, - }); - - const result = shouldIncrementUnread(msg, null); - - expect(result).toBeNull(); - }); - - it('returns key for incoming direct message when not viewing that contact', () => { - const msg = createMessage({ - type: 'PRIV', - conversation_key: 'abc123456789012345678901234567890123456789012345678901234567', - }); - const activeConversation: Conversation = { - type: 'contact', - id: 'xyz999999999012345678901234567890123456789012345678901234567', - name: 'other', - }; - - const result = shouldIncrementUnread(msg, activeConversation); - - // State key uses full public key - expect(result).toEqual({ - key: 'contact-abc123456789012345678901234567890123456789012345678901234567', - }); - }); - - it('returns null for incoming direct message when viewing that contact', () => { - const fullKey = 'abc123456789012345678901234567890123456789012345678901234567'; - const msg = createMessage({ - type: 'PRIV', - conversation_key: fullKey, - }); - const activeConversation: Conversation = { - type: 'contact', - id: fullKey, // Same full key - exact match required - name: 'Alice', - }; - - const result = shouldIncrementUnread(msg, activeConversation); - - expect(result).toBeNull(); - }); - - it('returns key when no conversation is active', () => { - const msg = createMessage({ - type: 'CHAN', - conversation_key: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA0', - }); - - const result = shouldIncrementUnread(msg, null); - - expect(result).toEqual({ key: 'channel-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA0' }); - }); - - it('returns key when viewing raw packet feed', () => { - const msg = createMessage({ - type: 'CHAN', - conversation_key: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA1', - }); - const activeConversation: Conversation = { type: 'raw', id: 'raw', name: 'Packets' }; - - const result = shouldIncrementUnread(msg, activeConversation); - - expect(result).toEqual({ key: 'channel-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA1' }); - }); -}); - -describe('getUnreadCount', () => { - it('returns count for channel by exact key match', () => { - const counts = { 'channel-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA5': 3 }; - - expect(getUnreadCount('channel', 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA5', counts)).toBe(3); - }); - - it('returns 0 for channel with no unread', () => { - const counts = { 'channel-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA5': 3 }; - - expect(getUnreadCount('channel', 'BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB9', counts)).toBe(0); - }); - - it('returns count for contact using full public key', () => { - const fullKey = 'abc123456789fullpublickey123456789012345678901234'; - const counts = { [`contact-${fullKey}`]: 5 }; - - expect(getUnreadCount('contact', fullKey, counts)).toBe(5); - }); - - it('returns 0 for contact with no unread', () => { - const counts = { 'contact-abc123456789fullpublickey123456789012345678901234': 5 }; - - expect( - getUnreadCount('contact', 'xyz999999999fullkey12345678901234567890123456789', counts) - ).toBe(0); - }); -}); - -/** - * Check if a message text contains a mention of the given name in @[name] format. - * Extracted from useUnreadCounts.ts for testing. - */ -function messageContainsMention(text: string, name: string | null): boolean { - if (!name) return false; - // Escape special regex characters in the name - const escaped = name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const mentionPattern = new RegExp(`@\\[${escaped}\\]`, 'i'); - return mentionPattern.test(text); -} +import { messageContainsMention } from '../utils/messageParser'; describe('messageContainsMention', () => { it('returns true when text contains mention of the name', () => { @@ -243,7 +38,6 @@ describe('messageContainsMention', () => { }); it('handles special regex characters in names', () => { - // Names with characters that have special meaning in regex expect(messageContainsMention('Hey @[Test.User] hello', 'Test.User')).toBe(true); expect(messageContainsMention('Hey @[User+1] hello', 'User+1')).toBe(true); expect(messageContainsMention('Hey @[User*Star] hello', 'User*Star')).toBe(true); @@ -251,7 +45,6 @@ describe('messageContainsMention', () => { }); it('does not match partial names', () => { - // @[Alice] should not match a name of just "Ali" expect(messageContainsMention('Hey @[Alice] check this', 'Ali')).toBe(false); }); diff --git a/frontend/src/test/useConversationMessages.test.ts b/frontend/src/test/useConversationMessages.test.ts index 919944e..883054d 100644 --- a/frontend/src/test/useConversationMessages.test.ts +++ b/frontend/src/test/useConversationMessages.test.ts @@ -159,75 +159,3 @@ describe('getMessageContentKey', () => { expect(getMessageContentKey(msg1)).toBe(getMessageContentKey(msg2)); }); }); - -describe('updateMessageAck logic', () => { - // Test the logic that updateMessageAck applies to messages - // This simulates what the setMessages callback does - - function applyAckUpdate( - messages: Message[], - messageId: number, - ackCount: number, - paths?: { path: string; received_at: number }[] - ): Message[] { - const idx = messages.findIndex((m) => m.id === messageId); - if (idx >= 0) { - const updated = [...messages]; - updated[idx] = { - ...messages[idx], - acked: ackCount, - ...(paths !== undefined && { paths }), - }; - return updated; - } - return messages; - } - - it('updates ack count for existing message', () => { - const messages = [createMessage({ id: 42, acked: 0 })]; - - const updated = applyAckUpdate(messages, 42, 3); - - expect(updated[0].acked).toBe(3); - }); - - it('updates paths when provided', () => { - const messages = [createMessage({ id: 42, acked: 0, paths: null })]; - const newPaths = [ - { path: '1A2B', received_at: 1700000000 }, - { path: '1A3C', received_at: 1700000005 }, - ]; - - const updated = applyAckUpdate(messages, 42, 2, newPaths); - - expect(updated[0].acked).toBe(2); - expect(updated[0].paths).toEqual(newPaths); - }); - - it('does not modify paths when not provided', () => { - const existingPaths = [{ path: '1A2B', received_at: 1700000000 }]; - const messages = [createMessage({ id: 42, acked: 1, paths: existingPaths })]; - - const updated = applyAckUpdate(messages, 42, 2); - - expect(updated[0].acked).toBe(2); - expect(updated[0].paths).toEqual(existingPaths); // Unchanged - }); - - it('returns unchanged array for unknown message id', () => { - const messages = [createMessage({ id: 42, acked: 0 })]; - - const updated = applyAckUpdate(messages, 999, 3); - - expect(updated).toEqual(messages); - expect(updated[0].acked).toBe(0); // Unchanged - }); - - it('handles empty paths array', () => { - const messages = [createMessage({ id: 42, acked: 0, paths: null })]; - - const updated = applyAckUpdate(messages, 42, 1, []); - - expect(updated[0].paths).toEqual([]); - }); -}); diff --git a/frontend/src/test/websocket.test.ts b/frontend/src/test/websocket.test.ts deleted file mode 100644 index e48635f..0000000 --- a/frontend/src/test/websocket.test.ts +++ /dev/null @@ -1,264 +0,0 @@ -/** - * Tests for WebSocket message parsing. - * - * These tests verify that WebSocket messages are correctly parsed - * and routed to the appropriate handlers. - */ - -import { describe, it, expect, vi } from 'vitest'; -import type { HealthStatus, Contact, Message, MessagePath, RawPacket } from '../types'; - -/** - * Parse and route a WebSocket message. - * Extracted logic from useWebSocket.ts for testing. - */ -function parseWebSocketMessage( - data: string, - handlers: { - onHealth?: (health: HealthStatus) => void; - onMessage?: (message: Message) => void; - onContact?: (contact: Contact) => void; - onRawPacket?: (packet: RawPacket) => void; - onMessageAcked?: (messageId: number, ackCount: number, paths?: MessagePath[]) => void; - } -): { type: string; handled: boolean } { - try { - const msg = JSON.parse(data); - - switch (msg.type) { - case 'health': - handlers.onHealth?.(msg.data as HealthStatus); - return { type: msg.type, handled: !!handlers.onHealth }; - case 'message': - handlers.onMessage?.(msg.data as Message); - return { type: msg.type, handled: !!handlers.onMessage }; - case 'contact': - handlers.onContact?.(msg.data as Contact); - return { type: msg.type, handled: !!handlers.onContact }; - case 'raw_packet': - handlers.onRawPacket?.(msg.data as RawPacket); - return { type: msg.type, handled: !!handlers.onRawPacket }; - case 'message_acked': { - const ackData = msg.data as { - message_id: number; - ack_count: number; - paths?: MessagePath[]; - }; - handlers.onMessageAcked?.(ackData.message_id, ackData.ack_count, ackData.paths); - return { type: msg.type, handled: !!handlers.onMessageAcked }; - } - case 'pong': - return { type: msg.type, handled: true }; - default: - return { type: msg.type, handled: false }; - } - } catch { - return { type: 'error', handled: false }; - } -} - -describe('parseWebSocketMessage', () => { - it('routes health message to onHealth handler', () => { - const onHealth = vi.fn(); - const data = JSON.stringify({ - type: 'health', - data: { radio_connected: true, connection_info: 'Serial: /dev/ttyUSB0' }, - }); - - const result = parseWebSocketMessage(data, { onHealth }); - - expect(result.type).toBe('health'); - expect(result.handled).toBe(true); - expect(onHealth).toHaveBeenCalledWith({ - radio_connected: true, - connection_info: 'Serial: /dev/ttyUSB0', - }); - }); - - it('routes message_acked to onMessageAcked with message ID and ack count', () => { - const onMessageAcked = vi.fn(); - const data = JSON.stringify({ - type: 'message_acked', - data: { message_id: 42, ack_count: 3 }, - }); - - const result = parseWebSocketMessage(data, { onMessageAcked }); - - expect(result.type).toBe('message_acked'); - expect(result.handled).toBe(true); - expect(onMessageAcked).toHaveBeenCalledWith(42, 3, undefined); - }); - - it('routes message_acked with paths array', () => { - const onMessageAcked = vi.fn(); - const paths = [ - { path: '1A2B', received_at: 1700000000 }, - { path: '1A3C', received_at: 1700000005 }, - ]; - const data = JSON.stringify({ - type: 'message_acked', - data: { message_id: 42, ack_count: 2, paths }, - }); - - const result = parseWebSocketMessage(data, { onMessageAcked }); - - expect(result.type).toBe('message_acked'); - expect(result.handled).toBe(true); - expect(onMessageAcked).toHaveBeenCalledWith(42, 2, paths); - }); - - it('routes new message to onMessage handler', () => { - const onMessage = vi.fn(); - const messageData = { - id: 123, - type: 'CHAN', - channel_idx: 0, - text: 'Hello', - received_at: 1700000000, - outgoing: false, - acked: 0, - }; - const data = JSON.stringify({ type: 'message', data: messageData }); - - const result = parseWebSocketMessage(data, { onMessage }); - - expect(result.type).toBe('message'); - expect(result.handled).toBe(true); - expect(onMessage).toHaveBeenCalledWith(messageData); - }); - - it('handles pong messages silently', () => { - const data = JSON.stringify({ type: 'pong' }); - - const result = parseWebSocketMessage(data, {}); - - expect(result.type).toBe('pong'); - expect(result.handled).toBe(true); - }); - - it('returns unhandled for unknown message types', () => { - const data = JSON.stringify({ type: 'unknown_type', data: {} }); - - const result = parseWebSocketMessage(data, {}); - - expect(result.type).toBe('unknown_type'); - expect(result.handled).toBe(false); - }); - - it('handles invalid JSON gracefully', () => { - const data = 'not valid json {'; - - const result = parseWebSocketMessage(data, {}); - - expect(result.type).toBe('error'); - expect(result.handled).toBe(false); - }); - - it('does not call handler when not provided', () => { - const data = JSON.stringify({ - type: 'health', - data: { radio_connected: true }, - }); - - const result = parseWebSocketMessage(data, {}); - - expect(result.type).toBe('health'); - expect(result.handled).toBe(false); - }); - - it('routes raw_packet to onRawPacket handler', () => { - const onRawPacket = vi.fn(); - const packetData = { - id: 1, - timestamp: 1700000000, - data: 'deadbeef', - payload_type: 'GROUP_TEXT', - decrypted: true, - decrypted_info: { channel_name: '#test', sender: 'Alice' }, - }; - const data = JSON.stringify({ type: 'raw_packet', data: packetData }); - - const result = parseWebSocketMessage(data, { onRawPacket }); - - expect(result.type).toBe('raw_packet'); - expect(result.handled).toBe(true); - expect(onRawPacket).toHaveBeenCalledWith(packetData); - }); -}); - -describe('useWebSocket ref-based handler pattern', () => { - /** - * These tests verify the pattern used in useWebSocket to avoid stale closures. - * The hook stores handlers in a ref and accesses them through the ref in callbacks. - * This ensures that when handlers are updated, the WebSocket still calls the latest version. - */ - - it('demonstrates ref pattern prevents stale closure', () => { - // Simulate the ref pattern used in useWebSocket - interface Handlers { - onMessage?: (msg: string) => void; - } - - // This simulates what the hook does: store handlers in a ref - const handlersRef: { current: Handlers } = { current: {} }; - - // First handler version - const firstHandler = vi.fn(); - handlersRef.current = { onMessage: firstHandler }; - - // Simulate what onmessage does: access handlers through ref - const processMessage = (data: string) => { - // This is the pattern: access through ref.current, not closed-over variable - handlersRef.current.onMessage?.(data); - }; - - // Send first message - processMessage('message1'); - expect(firstHandler).toHaveBeenCalledWith('message1'); - - // Update handler (simulates React re-render with new handler) - const secondHandler = vi.fn(); - handlersRef.current = { onMessage: secondHandler }; - - // Send second message - processMessage('message2'); - - // First handler should NOT be called again (would happen with stale closure) - expect(firstHandler).toHaveBeenCalledTimes(1); - // Second handler should be called (ref pattern works) - expect(secondHandler).toHaveBeenCalledWith('message2'); - }); - - it('demonstrates stale closure problem without ref pattern', () => { - // This demonstrates the bug we fixed - without refs, handlers become stale - interface Handlers { - onMessage?: (msg: string) => void; - } - - // First handler version - const firstHandler = vi.fn(); - let handlers: Handlers = { onMessage: firstHandler }; - - // BAD PATTERN: capture handlers in closure (this is what we fixed) - const capturedHandlers = handlers; - const processMessageBad = (data: string) => { - // This captures `capturedHandlers` at creation time - STALE! - capturedHandlers.onMessage?.(data); - }; - - // Send first message - processMessageBad('message1'); - expect(firstHandler).toHaveBeenCalledWith('message1'); - - // Update handler - const secondHandler = vi.fn(); - handlers = { onMessage: secondHandler }; - - // Send second message - BUG: still calls first handler! - processMessageBad('message2'); - - // This demonstrates the stale closure bug - expect(firstHandler).toHaveBeenCalledTimes(2); // Called twice - bug! - expect(secondHandler).not.toHaveBeenCalled(); // Never called - bug! - }); -}); diff --git a/frontend/src/utils/messageParser.ts b/frontend/src/utils/messageParser.ts index 02c733e..9cfe2d1 100644 --- a/frontend/src/utils/messageParser.ts +++ b/frontend/src/utils/messageParser.ts @@ -36,3 +36,11 @@ export function formatTime(timestamp: number): string { const dateStr = date.toLocaleDateString([], { month: 'short', day: 'numeric' }); return `${dateStr} ${time}`; } + +/** Check if a message text contains a mention of the given name in @[name] format. */ +export function messageContainsMention(text: string, name: string | null): boolean { + if (!name) return false; + const escaped = name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const mentionPattern = new RegExp(`@\\[${escaped}\\]`, 'i'); + return mentionPattern.test(text); +} diff --git a/frontend/src/utils/radioPresets.ts b/frontend/src/utils/radioPresets.ts new file mode 100644 index 0000000..5855d1d --- /dev/null +++ b/frontend/src/utils/radioPresets.ts @@ -0,0 +1,41 @@ +// Radio presets for common LoRa configurations +export interface RadioPreset { + name: string; + freq: number; + bw: number; + sf: number; + cr: number; +} + +export const RADIO_PRESETS: RadioPreset[] = [ + { name: 'USA/Canada', freq: 910.525, bw: 62.5, sf: 7, cr: 5 }, + { name: 'Australia', freq: 915.8, bw: 250, sf: 10, cr: 5 }, + { name: 'Australia (narrow)', freq: 916.575, bw: 62.5, sf: 7, cr: 8 }, + { name: 'Australia SA, WA', freq: 923.125, bw: 62.5, sf: 8, cr: 8 }, + { name: 'Australia QLD', freq: 923.125, bw: 62.5, sf: 8, cr: 5 }, + { name: 'New Zealand', freq: 917.375, bw: 250, sf: 11, cr: 5 }, + { name: 'New Zealand (narrow)', freq: 917.375, bw: 62.5, sf: 7, cr: 5 }, + { name: 'EU/UK/Switzerland Long Range', freq: 869.525, bw: 250, sf: 11, cr: 5 }, + { name: 'EU/UK/Switzerland Medium Range', freq: 869.525, bw: 250, sf: 10, cr: 5 }, + { name: 'EU/UK/Switzerland Narrow', freq: 869.618, bw: 62.5, sf: 8, cr: 8 }, + { name: 'Czech Republic (Narrow)', freq: 869.432, bw: 62.5, sf: 7, cr: 5 }, + { name: 'EU 433MHz Long Range', freq: 433.65, bw: 250, sf: 11, cr: 5 }, + { name: 'Portugal 433MHz', freq: 433.375, bw: 62.5, sf: 9, cr: 6 }, + { name: 'Portugal 868MHz', freq: 869.618, bw: 62.5, sf: 7, cr: 6 }, + { name: 'Vietnam', freq: 920.25, bw: 250, sf: 11, cr: 5 }, +]; + +/** Detect which preset matches the given radio parameters, or 'custom' if none match. */ +export function detectPreset(freq: number, bw: number, sf: number, cr: number): string { + for (const preset of RADIO_PRESETS) { + if (preset.freq === freq && preset.bw === bw && preset.sf === sf && preset.cr === cr) { + return preset.name; + } + } + return 'custom'; +} + +/** Find a preset by exact name. */ +export function findPreset(name: string): RadioPreset | undefined { + return RADIO_PRESETS.find((p) => p.name === name); +} diff --git a/tests/test_bot.py b/tests/test_bot.py index 409827d..d4d4328 100644 --- a/tests/test_bot.py +++ b/tests/test_bot.py @@ -430,20 +430,27 @@ class TestRunBotForMessage: """Bot is triggered for outgoing messages (user can trigger their own bots).""" with patch("app.repository.AppSettingsRepository") as mock_repo: mock_settings = MagicMock() - mock_settings.bots = [] # No enabled bots, but settings ARE checked + mock_settings.bots = [ + BotConfig(id="1", name="Echo", enabled=True, code="def bot(**k): return 'echo'") + ] mock_repo.get = AsyncMock(return_value=mock_settings) - await run_bot_for_message( - sender_name="Me", - sender_key="abc123", - message_text="Hello", - is_dm=True, - channel_key=None, - is_outgoing=True, - ) + with ( + patch("app.bot.asyncio.sleep", new_callable=AsyncMock), + patch("app.bot.execute_bot_code", return_value="echo") as mock_exec, + patch("app.bot.process_bot_response", new_callable=AsyncMock), + ): + await run_bot_for_message( + sender_name="Me", + sender_key="abc123" + "0" * 58, + message_text="Hello", + is_dm=True, + channel_key=None, + is_outgoing=True, + ) - # Should check settings (outgoing no longer skipped) - mock_repo.get.assert_called_once() + # Bot should actually execute for outgoing messages + mock_exec.assert_called_once() @pytest.mark.asyncio async def test_skips_when_no_enabled_bots(self): diff --git a/tests/test_decoder.py b/tests/test_decoder.py index 9c6c52a..7544db8 100644 --- a/tests/test_decoder.py +++ b/tests/test_decoder.py @@ -33,7 +33,10 @@ class TestChannelKeyDerivation: channel_name = "#test" expected_key = hashlib.sha256(channel_name.encode("utf-8")).digest()[:16] - # This matches the meshcore_py implementation + # Verify the derived key produces the expected channel hash + result_hash = calculate_channel_hash(expected_key) + expected_hash = format(hashlib.sha256(expected_key).digest()[0], "02x") + assert result_hash == expected_hash assert len(expected_key) == 16 def test_channel_hash_calculation(self): @@ -342,7 +345,7 @@ class TestAdvertisementParsing: def test_parse_advertisement_extracts_public_key(self): """Advertisement parsing extracts the public key correctly.""" - from app.decoder import PayloadType, parse_packet + from app.decoder import parse_advertisement, parse_packet packet_hex = ( "1100AE92564C5C9884854F04F469BBB2BAB8871A078053AF6CF4AA2C014B18CE8A83" @@ -352,21 +355,27 @@ class TestAdvertisementParsing: ) packet = bytes.fromhex(packet_hex) - # Verify packet is recognized as ADVERT type info = parse_packet(packet) assert info is not None - assert info.payload_type == PayloadType.ADVERT + + result = parse_advertisement(info.payload) + assert result is not None + assert ( + result.public_key == "ae92564c5c9884854f04f469bbb2bab8871a078053af6cf4aa2c014b18ce8a83" + ) def test_non_advertisement_returns_none(self): - """Non-advertisement packets return None when parsed as advertisement.""" - from app.decoder import PayloadType, parse_packet + """Non-advertisement payload returns None when parsed as advertisement.""" + from app.decoder import parse_advertisement, parse_packet # GROUP_TEXT packet, not an advertisement packet = bytes([0x15, 0x00]) + bytes(50) info = parse_packet(packet) assert info is not None - assert info.payload_type != PayloadType.ADVERT + + result = parse_advertisement(info.payload) + assert result is None class TestScalarClamping: @@ -484,18 +493,13 @@ class TestSharedSecretDerivation: def test_derive_shared_secret_different_keys_different_result(self): """Different key pairs produce different shared secrets.""" - other_pub = bytes.fromhex( - "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef" - ) + # Use the real FACE12 public key as a second peer key (valid curve point) + face12_pub = derive_public_key(self.FACE12_PRIV) result1 = derive_shared_secret(self.FACE12_PRIV, self.A1B2C3_PUB) - # This may raise an exception for invalid public key, which is also acceptable - try: - result2 = derive_shared_secret(self.FACE12_PRIV, other_pub) - assert result1 != result2 - except Exception: - # Invalid public keys may fail, which is fine - pass + result2 = derive_shared_secret(self.FACE12_PRIV, face12_pub) + + assert result1 != result2 class TestDirectMessageDecryption: diff --git a/tests/test_event_handlers.py b/tests/test_event_handlers.py index a351573..4920df6 100644 --- a/tests/test_event_handlers.py +++ b/tests/test_event_handlers.py @@ -676,53 +676,3 @@ class TestOnNewContact: contacts = await ContactRepository.get_all() assert len(contacts) == 0 - - @pytest.mark.asyncio - async def test_sets_on_radio_true(self, test_db): - """Contact data passed to upsert has on_radio=True.""" - from app.event_handlers import on_new_contact - - with ( - patch("app.event_handlers.broadcast_event"), - patch("app.event_handlers.time") as mock_time, - ): - mock_time.time.return_value = 1700000000 - - class MockEvent: - payload = { - "public_key": "dd" * 32, - "adv_name": "Delta", - "type": 0, - "flags": 0, - } - - await on_new_contact(MockEvent()) - - contact = await ContactRepository.get_by_key("dd" * 32) - assert contact is not None - assert contact.on_radio is True - - @pytest.mark.asyncio - async def test_sets_last_seen_to_current_timestamp(self, test_db): - """Contact data includes last_seen set to current time.""" - from app.event_handlers import on_new_contact - - with ( - patch("app.event_handlers.broadcast_event"), - patch("app.event_handlers.time") as mock_time, - ): - mock_time.time.return_value = 1700099999 - - class MockEvent: - payload = { - "public_key": "ee" * 32, - "adv_name": "Echo", - "type": 0, - "flags": 0, - } - - await on_new_contact(MockEvent()) - - contact = await ContactRepository.get_by_key("ee" * 32) - assert contact is not None - assert contact.last_seen == 1700099999 diff --git a/tests/test_radio_sync.py b/tests/test_radio_sync.py index 8788aed..d2844e3 100644 --- a/tests/test_radio_sync.py +++ b/tests/test_radio_sync.py @@ -130,17 +130,6 @@ class TestPollingPause: # Now unpaused - all contexts exited assert not is_polling_paused() - @pytest.mark.asyncio - async def test_triple_nested_pause(self): - """Three levels of nesting work correctly.""" - async with pause_polling(): - async with pause_polling(): - async with pause_polling(): - assert is_polling_paused() - assert is_polling_paused() - assert is_polling_paused() - assert not is_polling_paused() - @pytest.mark.asyncio async def test_pause_resumes_on_exception(self): """Polling resumes even if exception occurs in context.""" @@ -171,23 +160,6 @@ class TestPollingPause: # All contexts exited assert not is_polling_paused() - @pytest.mark.asyncio - async def test_counter_increments_and_decrements(self): - """Counter correctly tracks pause depth.""" - import app.radio_sync as radio_sync - - assert radio_sync._polling_pause_count == 0 - - async with pause_polling(): - assert radio_sync._polling_pause_count == 1 - - async with pause_polling(): - assert radio_sync._polling_pause_count == 2 - - assert radio_sync._polling_pause_count == 1 - - assert radio_sync._polling_pause_count == 0 - class TestSyncRadioTime: """Test the radio time sync function.""" diff --git a/tests/test_repository.py b/tests/test_repository.py index e8f4cf4..227a160 100644 --- a/tests/test_repository.py +++ b/tests/test_repository.py @@ -1,6 +1,5 @@ """Tests for repository layer.""" -import json from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -122,153 +121,98 @@ class TestMessageRepositoryAddPath: class TestMessageRepositoryGetByContent: - """Test MessageRepository.get_by_content method.""" + """Test MessageRepository.get_by_content against a real SQLite database.""" @pytest.mark.asyncio - async def test_get_by_content_finds_matching_message(self): + async def test_get_by_content_finds_matching_message(self, test_db): """Returns message when all content fields match.""" - mock_conn = AsyncMock() - mock_cursor = AsyncMock() - mock_cursor.fetchone = AsyncMock( - return_value={ - "id": 42, - "type": "CHAN", - "conversation_key": "ABCD1234", - "text": "Hello world", - "sender_timestamp": 1700000000, - "received_at": 1700000001, - "paths": None, - "txt_type": 0, - "signature": None, - "outgoing": 0, - "acked": 1, - } + msg_id = await _create_message( + test_db, + msg_type="CHAN", + conversation_key="ABCD1234ABCD1234ABCD1234ABCD1234", + text="Hello world", + sender_timestamp=1700000000, ) - mock_conn.execute = AsyncMock(return_value=mock_cursor) - mock_db = MagicMock() - mock_db.conn = mock_conn - - with patch("app.repository.db", mock_db): - from app.repository import MessageRepository - - result = await MessageRepository.get_by_content( - msg_type="CHAN", - conversation_key="ABCD1234", - text="Hello world", - sender_timestamp=1700000000, - ) + result = await MessageRepository.get_by_content( + msg_type="CHAN", + conversation_key="ABCD1234ABCD1234ABCD1234ABCD1234", + text="Hello world", + sender_timestamp=1700000000, + ) assert result is not None - assert result.id == 42 + assert result.id == msg_id assert result.type == "CHAN" - assert result.conversation_key == "ABCD1234" assert result.text == "Hello world" - assert result.acked == 1 @pytest.mark.asyncio - async def test_get_by_content_returns_none_when_not_found(self): + async def test_get_by_content_returns_none_when_not_found(self, test_db): """Returns None when no message matches.""" - mock_conn = AsyncMock() - mock_cursor = AsyncMock() - mock_cursor.fetchone = AsyncMock(return_value=None) - mock_conn.execute = AsyncMock(return_value=mock_cursor) + await _create_message(test_db, text="Existing message") - mock_db = MagicMock() - mock_db.conn = mock_conn - - with patch("app.repository.db", mock_db): - from app.repository import MessageRepository - - result = await MessageRepository.get_by_content( - msg_type="CHAN", - conversation_key="NONEXISTENT", - text="Not found", - sender_timestamp=1700000000, - ) + result = await MessageRepository.get_by_content( + msg_type="CHAN", + conversation_key="AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA0", + text="Not found", + sender_timestamp=1700000000, + ) assert result is None @pytest.mark.asyncio - async def test_get_by_content_handles_null_sender_timestamp(self): + async def test_get_by_content_handles_null_sender_timestamp(self, test_db): """Handles messages with NULL sender_timestamp correctly.""" - mock_conn = AsyncMock() - mock_cursor = AsyncMock() - mock_cursor.fetchone = AsyncMock( - return_value={ - "id": 43, - "type": "PRIV", - "conversation_key": "abc123", - "text": "Test message", - "sender_timestamp": None, - "received_at": 1700000001, - "paths": None, - "txt_type": 0, - "signature": None, - "outgoing": 1, - "acked": 0, - } + msg_id = await _create_message( + test_db, + msg_type="PRIV", + conversation_key="abc123abc123abc123abc123abc12300", + text="Null timestamp msg", + sender_timestamp=None, + outgoing=True, ) - mock_conn.execute = AsyncMock(return_value=mock_cursor) - mock_db = MagicMock() - mock_db.conn = mock_conn - - with patch("app.repository.db", mock_db): - from app.repository import MessageRepository - - result = await MessageRepository.get_by_content( - msg_type="PRIV", - conversation_key="abc123", - text="Test message", - sender_timestamp=None, - ) + result = await MessageRepository.get_by_content( + msg_type="PRIV", + conversation_key="abc123abc123abc123abc123abc12300", + text="Null timestamp msg", + sender_timestamp=None, + ) assert result is not None + assert result.id == msg_id assert result.sender_timestamp is None assert result.outgoing is True @pytest.mark.asyncio - async def test_get_by_content_parses_paths_correctly(self): - """Parses paths JSON into MessagePath objects.""" - paths_json = json.dumps( - [ - {"path": "1A2B", "received_at": 1700000000}, - {"path": "3C4D", "received_at": 1700000001}, - ] + async def test_get_by_content_distinguishes_by_timestamp(self, test_db): + """Different sender_timestamps are distinguished correctly.""" + await _create_message(test_db, text="Same text", sender_timestamp=1700000000) + msg_id2 = await _create_message(test_db, text="Same text", sender_timestamp=1700000001) + + result = await MessageRepository.get_by_content( + msg_type="CHAN", + conversation_key="AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA0", + text="Same text", + sender_timestamp=1700000001, ) - mock_conn = AsyncMock() - mock_cursor = AsyncMock() - mock_cursor.fetchone = AsyncMock( - return_value={ - "id": 44, - "type": "CHAN", - "conversation_key": "ABCD1234", - "text": "Multi-path message", - "sender_timestamp": 1700000000, - "received_at": 1700000000, - "paths": paths_json, - "txt_type": 0, - "signature": None, - "outgoing": 0, - "acked": 2, - } + assert result is not None + assert result.id == msg_id2 + + @pytest.mark.asyncio + async def test_get_by_content_with_paths(self, test_db): + """Returns message with paths correctly parsed.""" + msg_id = await _create_message(test_db, text="Multi-path message") + await MessageRepository.add_path(msg_id, "1A2B", received_at=1700000000) + await MessageRepository.add_path(msg_id, "3C4D", received_at=1700000001) + + result = await MessageRepository.get_by_content( + msg_type="CHAN", + conversation_key="AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA0", + text="Multi-path message", + sender_timestamp=1700000000, ) - mock_conn.execute = AsyncMock(return_value=mock_cursor) - - mock_db = MagicMock() - mock_db.conn = mock_conn - - with patch("app.repository.db", mock_db): - from app.repository import MessageRepository - - result = await MessageRepository.get_by_content( - msg_type="CHAN", - conversation_key="ABCD1234", - text="Multi-path message", - sender_timestamp=1700000000, - ) assert result is not None assert result.paths is not None @@ -277,99 +221,79 @@ class TestMessageRepositoryGetByContent: assert result.paths[1].path == "3C4D" @pytest.mark.asyncio - async def test_get_by_content_handles_corrupted_paths_json(self): - """Handles corrupted paths JSON gracefully.""" - mock_conn = AsyncMock() - mock_cursor = AsyncMock() - mock_cursor.fetchone = AsyncMock( - return_value={ - "id": 45, - "type": "CHAN", - "conversation_key": "ABCD1234", - "text": "Corrupted paths", - "sender_timestamp": 1700000000, - "received_at": 1700000000, - "paths": "not valid json {", - "txt_type": 0, - "signature": None, - "outgoing": 0, - "acked": 0, - } + async def test_get_by_content_recovers_from_corrupted_paths_json(self, test_db): + """Malformed JSON in paths column returns message with paths=None.""" + msg_id = await _create_message(test_db, text="Corrupted paths") + + # Inject malformed JSON directly into the paths column + await test_db.conn.execute( + "UPDATE messages SET paths = ? WHERE id = ?", + ("not valid json{{{", msg_id), ) - mock_conn.execute = AsyncMock(return_value=mock_cursor) + await test_db.conn.commit() - mock_db = MagicMock() - mock_db.conn = mock_conn + result = await MessageRepository.get_by_content( + msg_type="CHAN", + conversation_key="AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA0", + text="Corrupted paths", + sender_timestamp=1700000000, + ) - with patch("app.repository.db", mock_db): - from app.repository import MessageRepository - - result = await MessageRepository.get_by_content( - msg_type="CHAN", - conversation_key="ABCD1234", - text="Corrupted paths", - sender_timestamp=1700000000, - ) - - # Should return message with paths=None instead of raising assert result is not None + assert result.id == msg_id + assert result.paths is None + + @pytest.mark.asyncio + async def test_get_by_content_recovers_from_paths_missing_keys(self, test_db): + """Valid JSON but missing expected keys returns message with paths=None.""" + msg_id = await _create_message(test_db, text="Bad keys") + + # Valid JSON but missing "path" / "received_at" keys + await test_db.conn.execute( + "UPDATE messages SET paths = ? WHERE id = ?", + ('[{"wrong_key": "value"}]', msg_id), + ) + await test_db.conn.commit() + + result = await MessageRepository.get_by_content( + msg_type="CHAN", + conversation_key="AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA0", + text="Bad keys", + sender_timestamp=1700000000, + ) + + assert result is not None + assert result.id == msg_id assert result.paths is None class TestMessageRepositoryGetAckCount: - """Test MessageRepository.get_ack_count method.""" + """Test MessageRepository.get_ack_count against a real SQLite database.""" @pytest.mark.asyncio - async def test_get_ack_count_returns_count(self): + async def test_get_ack_count_returns_count(self, test_db): """Returns ack count for existing message.""" - mock_conn = AsyncMock() - mock_cursor = AsyncMock() - mock_cursor.fetchone = AsyncMock(return_value={"acked": 3}) - mock_conn.execute = AsyncMock(return_value=mock_cursor) + msg_id = await _create_message(test_db) + # Simulate acking by directly updating + await test_db.conn.execute("UPDATE messages SET acked = ? WHERE id = ?", (3, msg_id)) - mock_db = MagicMock() - mock_db.conn = mock_conn - - with patch("app.repository.db", mock_db): - from app.repository import MessageRepository - - result = await MessageRepository.get_ack_count(message_id=42) + result = await MessageRepository.get_ack_count(message_id=msg_id) assert result == 3 @pytest.mark.asyncio - async def test_get_ack_count_returns_zero_for_nonexistent(self): + async def test_get_ack_count_returns_zero_for_nonexistent(self, test_db): """Returns 0 for nonexistent message.""" - mock_conn = AsyncMock() - mock_cursor = AsyncMock() - mock_cursor.fetchone = AsyncMock(return_value=None) - mock_conn.execute = AsyncMock(return_value=mock_cursor) - - mock_db = MagicMock() - mock_db.conn = mock_conn - - with patch("app.repository.db", mock_db): - from app.repository import MessageRepository - - result = await MessageRepository.get_ack_count(message_id=999) + result = await MessageRepository.get_ack_count(message_id=999999) assert result == 0 @pytest.mark.asyncio - async def test_get_ack_count_returns_zero_for_unacked(self): + async def test_get_ack_count_returns_zero_for_unacked(self, test_db): """Returns 0 for message with no acks.""" - mock_conn = AsyncMock() - mock_cursor = AsyncMock() - mock_cursor.fetchone = AsyncMock(return_value={"acked": 0}) - mock_conn.execute = AsyncMock(return_value=mock_cursor) + msg_id = await _create_message(test_db) - mock_db = MagicMock() - mock_db.conn = mock_conn - - with patch("app.repository.db", mock_db): - from app.repository import MessageRepository - - result = await MessageRepository.get_ack_count(message_id=42) + result = await MessageRepository.get_ack_count(message_id=msg_id) assert result == 0 diff --git a/tests/test_websocket_route.py b/tests/test_websocket_route.py index 0295842..fdb735f 100644 --- a/tests/test_websocket_route.py +++ b/tests/test_websocket_route.py @@ -120,34 +120,6 @@ class TestWebSocketEndpoint: assert pong == {"type": "pong"} - def test_multiple_pings_return_multiple_pongs(self): - """Each ping gets its own pong response.""" - with ( - patch("app.routers.ws.radio_manager") as mock_ws_rm, - patch("app.routers.health.radio_manager") as mock_health_rm, - patch("app.routers.health.RawPacketRepository") as mock_repo, - patch("app.routers.health.settings") as mock_settings, - patch("app.routers.health.os.path.getsize", return_value=0), - ): - mock_ws_rm.is_connected = True - mock_ws_rm.connection_info = "Serial: /dev/ttyUSB0" - mock_health_rm.is_connected = True - mock_health_rm.connection_info = "Serial: /dev/ttyUSB0" - mock_repo.get_oldest_undecrypted = AsyncMock(return_value=None) - mock_settings.database_path = "/tmp/test.db" - - from app.main import app - - client = TestClient(app) - - with client.websocket_connect("/api/ws") as ws: - ws.receive_json() # consume health - - for _ in range(3): - ws.send_text("ping") - pong = ws.receive_json() - assert pong == {"type": "pong"} - def test_non_ping_message_does_not_produce_response(self): """Messages other than 'ping' are silently ignored (no response sent).""" with ( @@ -204,30 +176,3 @@ class TestWebSocketEndpoint: # After context manager exits, the WebSocket is closed assert len(ws_manager.active_connections) == 0 - - def test_disconnect_is_clean_no_error(self): - """Normal client disconnect does not raise or leave dangling state.""" - with ( - patch("app.routers.ws.radio_manager") as mock_ws_rm, - patch("app.routers.health.radio_manager") as mock_health_rm, - patch("app.routers.health.RawPacketRepository") as mock_repo, - patch("app.routers.health.settings") as mock_settings, - patch("app.routers.health.os.path.getsize", return_value=0), - ): - mock_ws_rm.is_connected = False - mock_ws_rm.connection_info = None - mock_health_rm.is_connected = False - mock_health_rm.connection_info = None - mock_repo.get_oldest_undecrypted = AsyncMock(return_value=None) - mock_settings.database_path = "/tmp/test.db" - - from app.main import app - - client = TestClient(app) - - # Connect and immediately disconnect -- should not raise - with client.websocket_connect("/api/ws") as ws: - ws.receive_json() # consume health - - # Verify clean state - assert len(ws_manager.active_connections) == 0