From cba983556881125f48f9e8327d7f91c278b66cce Mon Sep 17 00:00:00 2001 From: Jack Kingsman Date: Fri, 6 Mar 2026 12:59:33 -0800 Subject: [PATCH] Rework more coverage in e2e tests and don't force radio restart + better startup error handling --- app/config.py | 37 ++++ app/radio.py | 23 ++- app/radio_sync.py | 24 ++- .../settings/SettingsRadioSection.tsx | 73 +++++--- frontend/src/test/settingsModal.test.tsx | 2 +- tests/e2e/helpers/meshTrafficTest.ts | 22 +++ tests/e2e/specs/incoming-message.spec.ts | 8 +- tests/e2e/specs/packet-feed.spec.ts | 5 +- tests/e2e/specs/webhook-delivery.spec.ts | 160 ++++++++++++++++++ tests/e2e/specs/webhook.spec.ts | 3 +- tests/e2e/specs/zz-radio-settings.spec.ts | 13 +- 11 files changed, 335 insertions(+), 35 deletions(-) create mode 100644 tests/e2e/specs/webhook-delivery.spec.ts diff --git a/app/config.py b/app/config.py index 36c80e9..20617ec 100644 --- a/app/config.py +++ b/app/config.py @@ -48,6 +48,40 @@ class Settings(BaseSettings): settings = Settings() +class _RepeatSquelch(logging.Filter): + """Suppress rapid-fire identical messages and emit a summary instead. + + Attached to the ``meshcore`` library logger to catch its repeated + "Serial Connection started" lines that flood the log when another + process holds the serial port. + """ + + def __init__(self, threshold: int = 3) -> None: + super().__init__() + self._last_msg: str | None = None + self._repeat_count: int = 0 + self._threshold = threshold + + def filter(self, record: logging.LogRecord) -> bool: + msg = record.getMessage() + if msg == self._last_msg: + self._repeat_count += 1 + if self._repeat_count == self._threshold: + record.msg = ( + "%s (repeated %d times — possible serial port contention from another process)" + ) + record.args = (msg, self._repeat_count) + record.levelno = logging.WARNING + record.levelname = "WARNING" + return True + # Suppress further repeats beyond the threshold + return self._repeat_count < self._threshold + else: + self._last_msg = msg + self._repeat_count = 1 + return True + + def setup_logging() -> None: """Configure logging for the application.""" logging.basicConfig( @@ -55,3 +89,6 @@ def setup_logging() -> None: format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", datefmt="%Y-%m-%d %H:%M:%S", ) + # Squelch repeated messages from the meshcore library (e.g. rapid-fire + # "Serial Connection started" when the port is contended). + logging.getLogger("meshcore").addFilter(_RepeatSquelch()) diff --git a/app/radio.py b/app/radio.py index 078dd1e..95d5dd0 100644 --- a/app/radio.py +++ b/app/radio.py @@ -470,6 +470,8 @@ class RadioManager: from app.websocket import broadcast_health CHECK_INTERVAL_SECONDS = 5 + UNRESPONSIVE_THRESHOLD = 3 + consecutive_setup_failures = 0 while True: try: @@ -483,6 +485,7 @@ class RadioManager: logger.warning("Radio connection lost, broadcasting status change") broadcast_health(False, self._connection_info) self._last_connected = False + consecutive_setup_failures = 0 if not current_connected: # Attempt reconnection on every loop while disconnected @@ -492,6 +495,7 @@ class RadioManager: await self.post_connect_setup() broadcast_health(True, self._connection_info) self._last_connected = True + consecutive_setup_failures = 0 elif not self._last_connected and current_connected: # Connection restored (might have reconnected automatically). @@ -500,19 +504,34 @@ class RadioManager: await self.post_connect_setup() broadcast_health(True, self._connection_info) self._last_connected = True + consecutive_setup_failures = 0 elif current_connected and not self._setup_complete: # Transport connected but setup incomplete — retry logger.info("Retrying post-connect setup...") await self.post_connect_setup() broadcast_health(True, self._connection_info) + consecutive_setup_failures = 0 except asyncio.CancelledError: # Task is being cancelled, exit cleanly break except Exception as e: - # Log error but continue monitoring - don't let the monitor die - logger.exception("Error in connection monitor, continuing: %s", e) + consecutive_setup_failures += 1 + if consecutive_setup_failures == UNRESPONSIVE_THRESHOLD: + logger.error( + "Post-connect setup has failed %d times in a row. " + "The radio port appears open but the radio is not " + "responding to commands. Common causes: another " + "process has the serial port open (check for other " + "RemoteTerm instances, serial monitors, etc.), the " + "firmware is in repeater mode (not client), or the " + "radio needs a power cycle. Will keep retrying.", + consecutive_setup_failures, + ) + elif consecutive_setup_failures < UNRESPONSIVE_THRESHOLD: + logger.exception("Error in connection monitor, continuing: %s", e) + # After the threshold, silently retry (avoid log spam) self._reconnect_task = asyncio.create_task(monitor_loop()) logger.info("Radio connection monitor started") diff --git a/app/radio_sync.py b/app/radio_sync.py index e4976aa..53302be 100644 --- a/app/radio_sync.py +++ b/app/radio_sync.py @@ -117,7 +117,16 @@ async def sync_and_offload_contacts(mc: MeshCore) -> dict: result = await mc.commands.get_contacts() if result is None or result.type == EventType.ERROR: - logger.error("Failed to get contacts from radio: %s", result) + logger.error( + "Failed to get contacts from radio: %s. " + "If you see this repeatedly, the radio may be visible on the " + "serial/TCP/BLE port but not responding to commands. Check for " + "another process with the serial port open (other RemoteTerm " + "instances, serial monitors, etc.), verify the firmware is " + "up-to-date and in client mode (not repeater), or try a " + "power cycle.", + result, + ) return {"synced": 0, "removed": 0, "error": str(result)} contacts = result.payload or {} @@ -662,8 +671,19 @@ async def _sync_contacts_to_radio_inner(mc: MeshCore) -> dict: logger.debug("Loaded contact %s to radio", contact.public_key[:12]) else: failed += 1 + reason = result.payload + hint = "" + if reason is None: + hint = ( + " (no response from radio — if this repeats, check for " + "serial port contention from another process or try a " + "power cycle)" + ) logger.warning( - "Failed to load contact %s: %s", contact.public_key[:12], result.payload + "Failed to load contact %s: %s%s", + contact.public_key[:12], + reason, + hint, ) except Exception as e: failed += 1 diff --git a/frontend/src/components/settings/SettingsRadioSection.tsx b/frontend/src/components/settings/SettingsRadioSection.tsx index d7a6f57..33b8e48 100644 --- a/frontend/src/components/settings/SettingsRadioSection.tsx +++ b/frontend/src/components/settings/SettingsRadioSection.tsx @@ -141,9 +141,7 @@ export function SettingsRadioSection({ ); }; - const handleSave = async () => { - setError(null); - + const buildUpdate = (): RadioConfigUpdate | null => { const parsedLat = parseFloat(lat); const parsedLon = parseFloat(lon); const parsedTxPower = parseInt(txPower, 10); @@ -158,24 +156,46 @@ export function SettingsRadioSection({ ) ) { setError('All numeric fields must have valid values'); - return; + return null; } - setBusy(true); + return { + name, + lat: parsedLat, + lon: parsedLon, + tx_power: parsedTxPower, + radio: { + freq: parsedFreq, + bw: parsedBw, + sf: parsedSf, + cr: parsedCr, + }, + }; + }; + const handleSave = async () => { + setError(null); + const update = buildUpdate(); + if (!update) return; + + setBusy(true); + try { + await onSave(update); + toast.success('Radio config saved'); + } catch (err) { + setError(err instanceof Error ? err.message : 'Failed to save'); + } finally { + setBusy(false); + } + }; + + const handleSaveAndReboot = async () => { + setError(null); + const update = buildUpdate(); + if (!update) return; + + setBusy(true); try { - const update: RadioConfigUpdate = { - name, - lat: parsedLat, - lon: parsedLon, - tx_power: parsedTxPower, - radio: { - freq: parsedFreq, - bw: parsedBw, - sf: parsedSf, - cr: parsedCr, - }, - }; await onSave(update); toast.success('Radio config saved, rebooting...'); setRebooting(true); @@ -413,9 +433,22 @@ export function SettingsRadioSection({ )} - +
+ + +
+

+ Some settings may require a reboot to take effect on some radios. +

diff --git a/frontend/src/test/settingsModal.test.tsx b/frontend/src/test/settingsModal.test.tsx index 65c3fae..96fa50f 100644 --- a/frontend/src/test/settingsModal.test.tsx +++ b/frontend/src/test/settingsModal.test.tsx @@ -295,7 +295,7 @@ describe('SettingsModal', () => { }); openRadioSection(); - fireEvent.click(screen.getByRole('button', { name: 'Save Radio Config & Reboot' })); + fireEvent.click(screen.getByRole('button', { name: 'Save & Reboot' })); await waitFor(() => { expect(onSave).toHaveBeenCalledTimes(1); expect(onReboot).toHaveBeenCalledTimes(1); diff --git a/tests/e2e/helpers/meshTrafficTest.ts b/tests/e2e/helpers/meshTrafficTest.ts index d561cf3..1acc651 100644 --- a/tests/e2e/helpers/meshTrafficTest.ts +++ b/tests/e2e/helpers/meshTrafficTest.ts @@ -9,8 +9,15 @@ * When a @mesh-traffic-tagged test fails, an advisory annotation is added * to the HTML report and a console message is printed, letting the user * know the failure may be due to low mesh traffic rather than a real bug. + * + * Call `await nudgeEchoBot()` at the start of any @mesh-traffic test to + * send a trigger message to an echo bot on #flightless. If the bot is in + * radio range it will generate an incoming packet, potentially saving the + * full 3-minute wait. The nudge is best-effort — tests still rely on the + * long polling timeout for environments without the bot. */ import { test as base, expect } from '@playwright/test'; +import { ensureFlightlessChannel, sendChannelMessage } from './api'; export { expect }; @@ -18,6 +25,21 @@ const TRAFFIC_ADVISORY = 'This test depends on receiving messages from other nodes on the mesh ' + 'network. Failure may indicate insufficient mesh traffic rather than a bug.'; +/** + * Best-effort: send a message to #flightless that triggers a remote echo + * bot. If the bot is within radio range it will reply, generating the + * incoming traffic the test needs. Failures are silently ignored — the + * test will fall back to waiting for organic mesh traffic. + */ +export async function nudgeEchoBot(): Promise { + try { + const channel = await ensureFlightlessChannel(); + await sendChannelMessage(channel.key, '!echo please give incoming message'); + } catch { + // Best-effort — bot may not be reachable + } +} + export const test = base.extend<{ _meshTrafficAdvisory: void }>({ _meshTrafficAdvisory: [ async ({}, use, testInfo) => { diff --git a/tests/e2e/specs/incoming-message.spec.ts b/tests/e2e/specs/incoming-message.spec.ts index 3987579..d109958 100644 --- a/tests/e2e/specs/incoming-message.spec.ts +++ b/tests/e2e/specs/incoming-message.spec.ts @@ -1,4 +1,4 @@ -import { test, expect } from '../helpers/meshTrafficTest'; +import { test, expect, nudgeEchoBot } from '../helpers/meshTrafficTest'; import { createChannel, getChannels, getMessages } from '../helpers/api'; /** @@ -55,6 +55,9 @@ test.describe('Incoming mesh messages', () => { }); test('receive an incoming message in any room', { tag: '@mesh-traffic' }, async ({ page }) => { + // Nudge echo bot on #flightless — may generate an incoming packet quickly + await nudgeEchoBot(); + await page.goto('/'); await expect(page.getByText('Connected')).toBeVisible(); @@ -103,6 +106,9 @@ test.describe('Incoming mesh messages', () => { }); test('incoming message with path shows hop badge and path modal', { tag: '@mesh-traffic' }, async ({ page }) => { + // Nudge echo bot on #flightless — may generate an incoming packet quickly + await nudgeEchoBot(); + await page.goto('/'); await expect(page.getByText('Connected')).toBeVisible(); diff --git a/tests/e2e/specs/packet-feed.spec.ts b/tests/e2e/specs/packet-feed.spec.ts index 8d84fed..f2ff69c 100644 --- a/tests/e2e/specs/packet-feed.spec.ts +++ b/tests/e2e/specs/packet-feed.spec.ts @@ -1,4 +1,4 @@ -import { test, expect } from '../helpers/meshTrafficTest'; +import { test, expect, nudgeEchoBot } from '../helpers/meshTrafficTest'; test.describe('Packet Feed page', () => { test('packet feed page loads and shows header', async ({ page }) => { @@ -11,6 +11,9 @@ test.describe('Packet Feed page', () => { // This test waits for real RF traffic — needs 180s timeout test.setTimeout(180_000); + // Nudge echo bot on #flightless — may generate a packet quickly + await nudgeEchoBot(); + await page.goto('/#raw'); await expect(page.getByText('Raw Packet Feed')).toBeVisible({ timeout: 10_000 }); diff --git a/tests/e2e/specs/webhook-delivery.spec.ts b/tests/e2e/specs/webhook-delivery.spec.ts new file mode 100644 index 0000000..f5c1809 --- /dev/null +++ b/tests/e2e/specs/webhook-delivery.spec.ts @@ -0,0 +1,160 @@ +import { test, expect } from '@playwright/test'; +import http from 'http'; +import { + createFanoutConfig, + deleteFanoutConfig, + ensureFlightlessChannel, + sendChannelMessage, +} from '../helpers/api'; + +/** + * Spin up a local HTTP server that captures incoming webhook requests. + * Returns the server, its URL, and a promise-based helper to wait for + * the next request body. + */ +function createWebhookReceiver() { + const requests: { body: string; headers: http.IncomingHttpHeaders }[] = []; + let resolve: (() => void) | null = null; + + const server = http.createServer((req, res) => { + let body = ''; + req.on('data', (chunk) => (body += chunk)); + req.on('end', () => { + requests.push({ body, headers: req.headers }); + resolve?.(); + resolve = null; + res.writeHead(200); + res.end('ok'); + }); + }); + + return { + server, + requests, + /** Wait until at least `count` requests have been received. */ + waitForRequests(count: number, timeoutMs = 30_000): Promise { + if (requests.length >= count) return Promise.resolve(); + return new Promise((res, rej) => { + const timer = setTimeout( + () => rej(new Error(`Timed out waiting for ${count} webhook request(s), got ${requests.length}`)), + timeoutMs + ); + const check = () => { + if (requests.length >= count) { + clearTimeout(timer); + res(); + } else { + resolve = check; + } + }; + resolve = check; + }); + }, + /** Start listening on a random port and return the URL. */ + async listen(): Promise { + return new Promise((res) => { + server.listen(0, '127.0.0.1', () => { + const addr = server.address(); + if (typeof addr === 'object' && addr) { + res(`http://127.0.0.1:${addr.port}`); + } + }); + }); + }, + }; +} + +test.describe('Webhook delivery', () => { + let webhookId: string | null = null; + let receiver: ReturnType; + let webhookUrl: string; + + test.beforeAll(async () => { + await ensureFlightlessChannel(); + receiver = createWebhookReceiver(); + webhookUrl = await receiver.listen(); + }); + + test.afterAll(async () => { + receiver.server.close(); + if (webhookId) { + try { + await deleteFanoutConfig(webhookId); + } catch { + // Best-effort cleanup + } + } + }); + + test('webhook receives message payload when a channel message is sent', async () => { + // Create an enabled webhook pointing at our local receiver + const webhook = await createFanoutConfig({ + type: 'webhook', + name: 'E2E Delivery Test', + config: { url: webhookUrl, method: 'POST', headers: {} }, + enabled: true, + }); + webhookId = webhook.id; + + // Send a message via API — this triggers broadcast_event → fanout → webhook + const channel = await ensureFlightlessChannel(); + const testText = `webhook-delivery-${Date.now()}`; + await sendChannelMessage(channel.key, testText); + + // Wait for the webhook to receive the request + await receiver.waitForRequests(1); + + const req = receiver.requests[0]; + expect(req.headers['content-type']).toBe('application/json'); + expect(req.headers['x-webhook-event']).toBe('message'); + + const payload = JSON.parse(req.body); + expect(payload.text).toContain(testText); + expect(payload.type).toBe('CHAN'); + expect(payload.conversation_key).toBe(channel.key); + }); + + test('webhook respects HMAC signing when configured', async () => { + // Clean up previous webhook + if (webhookId) { + await deleteFanoutConfig(webhookId); + } + + const hmacSecret = 'e2e-test-secret'; + const webhook = await createFanoutConfig({ + type: 'webhook', + name: 'E2E HMAC Test', + config: { + url: webhookUrl, + method: 'POST', + headers: {}, + hmac_secret: hmacSecret, + }, + enabled: true, + }); + webhookId = webhook.id; + + // Clear previous requests + const baselineCount = receiver.requests.length; + + const channel = await ensureFlightlessChannel(); + const testText = `hmac-test-${Date.now()}`; + await sendChannelMessage(channel.key, testText); + + await receiver.waitForRequests(baselineCount + 1); + + const req = receiver.requests[baselineCount]; + const signature = req.headers['x-webhook-signature']; + expect(signature).toBeDefined(); + expect(typeof signature).toBe('string'); + expect((signature as string).startsWith('sha256=')).toBe(true); + + // Verify the HMAC is valid + const crypto = await import('crypto'); + const expectedSig = crypto + .createHmac('sha256', hmacSecret) + .update(req.body) + .digest('hex'); + expect(signature).toBe(`sha256=${expectedSig}`); + }); +}); diff --git a/tests/e2e/specs/webhook.spec.ts b/tests/e2e/specs/webhook.spec.ts index 86e583a..0c96483 100644 --- a/tests/e2e/specs/webhook.spec.ts +++ b/tests/e2e/specs/webhook.spec.ts @@ -115,10 +115,9 @@ test.describe('Webhook integration settings', () => { const row = page.getByText('Scope Webhook').locator('..'); await row.getByRole('button', { name: 'Edit' }).click(); - // Verify scope selector is visible with all four modes + // Verify scope selector is visible with the three webhook-applicable modes await expect(page.getByText('Message Scope')).toBeVisible(); await expect(page.getByText('All messages')).toBeVisible(); - await expect(page.getByText('No messages')).toBeVisible(); await expect(page.getByText('Only listed channels/contacts')).toBeVisible(); await expect(page.getByText('All except listed channels/contacts')).toBeVisible(); diff --git a/tests/e2e/specs/zz-radio-settings.spec.ts b/tests/e2e/specs/zz-radio-settings.spec.ts index 08b2e18..068089e 100644 --- a/tests/e2e/specs/zz-radio-settings.spec.ts +++ b/tests/e2e/specs/zz-radio-settings.spec.ts @@ -23,16 +23,17 @@ test.describe('Radio settings', () => { await nameInput.clear(); await nameInput.fill(testName); - await page.getByRole('button', { name: 'Save Radio Config & Reboot' }).click(); - await expect(page.getByText('Radio config saved, rebooting...')).toBeVisible({ timeout: 10_000 }); + // Use "Save" (no reboot) — name changes apply immediately + await page.getByRole('button', { name: 'Save', exact: true }).click(); + await expect(page.getByText('Radio config saved')).toBeVisible({ timeout: 10_000 }); + + // --- Step 2: Verify via API (send_appstart refreshes cached info) --- + const config = await getRadioConfig(); + expect(config.name).toBe(testName); // Exit settings page mode await page.getByRole('button', { name: /Back to Chat/i }).click(); - // --- Step 2: Verify via API (now returns fresh data after send_appstart fix) --- - const config = await getRadioConfig(); - expect(config.name).toBe(testName); - // --- Step 3: Verify persistence across page reload --- await page.reload(); await expect(page.getByText('Connected')).toBeVisible({ timeout: 15_000 });