diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9b3e250..f09c6be 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,7 +8,7 @@ Before you start, please read this document carefully. These guidelines exist to 1. **Open an issue first.** Before writing code, open an issue describing the bug or feature. This avoids wasted effort if the change doesn't align with the project direction. 2. **One PR, one concern.** Don't mix a bug fix with a refactor. Don't sneak in "while I was here" changes. Keep your diff focused. -3. **Don't break the build.** Run `yarn checkup` before pushing. If it doesn't pass, your PR won't be reviewed. +3. **Don't break the build.** Run `yarn verify` before pushing. If it doesn't pass, your PR won't be reviewed. 4. **Match the existing style.** Don't introduce new patterns, conventions, or abstractions without discussing them first. ## Getting started @@ -87,35 +87,43 @@ Don't use `feat` for a bug fix. Don't use `fix` for a refactor. Mean what you sa | `yarn test:watch` | Unit tests in watch mode | | `yarn test:e2e` | Build + the e2e suite (Playwright) | | `yarn test:e2e:packaged` | Same specs against the packaged app. Run before releasing. | -| `yarn test:e2e:hardware` | The `99-hardware` specs. Needs an Arduino and someone at the keyboard. | +| `yarn test:e2e:hardware` | The `99-hardware` specs. Needs an Arduino; skips without one. | | `yarn presentation` | Build + regenerate the documentation screenshots | -| `yarn checkup` | **Everything.** Lint + typecheck + unit + e2e. Run this before pushing. | +| `yarn verify` | Lint + typecheck + unit + e2e. Run this before pushing. | +| `yarn test:e2e:scan-perf` | What a mounted grid costs during a scan. A measurement, not a check. | +| `yarn test:e2e:privileged-port` | The port 502 modal. Linux, and someone at the keyboard. | +| `yarn test:all:mac` | Everything this platform can run, ending with the hardware specs. | +| `yarn test:all:windows` | Everything this platform can run. No socat, so the socat serial specs skip. | +| `yarn test:all:linux` | Everything, including the one that waits for a person. | `test:e2e` covers `01-main` and `02-standalone`. Two suites sit outside it and are invoked on purpose: -- `99-hardware` waits for an Arduino and for someone to pick the COM port, so an - unattended run never finishes. +- `99-hardware` needs an Arduino on a serial port. It finds the board by USB + vendor ID and skips the suite when none is attached, so it runs unattended -- + but CI has no board, which is why it stays out of `test:e2e`. - `03-presentation` is a documentation utility, not a check. It clicks through the app and captures what it sees without asserting much, so it costs two minutes to tell you little that `01-main` does not already cover. Run it when the UI changed and the manual needs new screenshots. -`checkup` deliberately leaves out `test:e2e:packaged` — it adds a full packaging -step and runs far longer, which is too much for every push. Run it before -cutting a release instead: it is the only check that exercises what actually -ships. `electron-vite` externalizes whatever sits in `dependencies` and -`electron-builder` packs only those into `app.asar`, so a runtime dependency +`verify` deliberately leaves out `test:e2e:packaged` — it adds a full packaging +step and runs far longer, which is too much for every push. The `test:all:*` +rounds do include it, and those are for cutting a release rather than for a PR. +It is the only check that exercises what actually ships: `electron-vite` +externalizes whatever sits in `dependencies` and `electron-builder` packs only +those into `app.asar`, so a runtime dependency that drifts into `devDependencies` passes every normal test and breaks only once installed. Packaged runs use a throwaway user-data directory and never touch an installed Modbux's config. `playwright.config.ts` ignores `99-hardware`, so neither `test:e2e` nor -`test:e2e:packaged` picks those specs up. They are conditional: they need an -Arduino running `tools/arduino/iem3000.ino` on a serial port, and they stop at a -`page.pause()` for someone to choose the COM port. In a pipeline — or in any run -you walked away from — that is not a failure, it is a run that never ends. Use -`yarn test:e2e:hardware` when the hardware is actually on your desk. +`test:e2e:packaged` picks those specs up. They need an Arduino running +`tools/arduino/iem3000.ino` on a serial port. The board is found by USB vendor +ID — `manufacturer` is useless for this, it reads "Microsoft" on Windows where +the generic driver claims the device — and the suite skips itself when no board +is attached. So the round is unattended, and every `test:all:*` ends with it. +Use `yarn test:e2e:hardware` to run it alone. ### Test expectations @@ -141,13 +149,13 @@ you walked away from — that is not a failure, it is a run that never ends. Use 1. Branch from `main`. Use `feature/description` or `fix/description`. 2. Keep commits clean. Squash fixups before requesting review. 3. Write a clear PR description: what changed, why, and how to test it. -4. `yarn checkup` must pass. No exceptions. +4. `yarn verify` must pass. No exceptions. 5. Screenshots for UI changes. Before and after. 6. Don't bump the version number. That's done at release time. ## What will get your PR rejected -- Failing `yarn checkup` +- Failing `yarn verify` - `any` types or disabled lint rules - Missing tests for new functionality - Unrelated changes mixed into the diff diff --git a/e2e/fixtures/arduino-port.ts b/e2e/fixtures/arduino-port.ts new file mode 100644 index 0000000..45a9291 --- /dev/null +++ b/e2e/fixtures/arduino-port.ts @@ -0,0 +1,60 @@ +import type { Page } from '@playwright/test' +import { SerialPort } from 'serialport' + +/** + * USB vendor IDs that count as "the Arduino running iem3000.ino". + * + * Only the two genuine ones. A clone board presents the VID of whatever USB + * bridge it carries -- 1a86 for a CH340, 0403 for an FTDI -- and those chips + * sit on hundreds of unrelated adapters, so matching them would let the suite + * pick up a random dongle and read nonsense off it. Add one here if you use a + * clone; the failure message below prints what it saw, so you know what to add. + */ +export const ARDUINO_VENDOR_IDS = ['2341', '2a03'] + +export type PortChoice = { port: string; reason?: undefined } | { port?: undefined; reason: string } + +/** + * The Arduino's serial port, or why there isn't one. + * + * Replaces the page.pause() these specs used to open, which needed a person to + * pick the port by hand and so kept the hardware round out of the unattended + * suites. The vendor ID is what identifies the board: `manufacturer` reads + * "Microsoft" on Windows, where the generic usbser driver claims the device. + * + * Returns a reason rather than throwing, so the caller decides between skipping + * the suite and failing it. + */ +export async function findArduinoPort(): Promise { + const ports = await SerialPort.list() + const matches = ports.filter((p) => ARDUINO_VENDOR_IDS.includes((p.vendorId ?? '').toLowerCase())) + + if (matches.length === 1) return { port: matches[0].path } + + const seen = ports.length + ? ports.map((p) => `${p.path} (vid ${p.vendorId ?? '?'})`).join(', ') + : 'no serial ports at all' + + if (matches.length === 0) { + return { reason: `No Arduino on any serial port. Saw: ${seen}` } + } + + // Two boards is not a machine to guess on: picking the wrong one reads + // registers off something that was never programmed with iem3000.ino, and + // the failure would land on a value assertion far from the cause. + return { + reason: `${matches.length} Arduinos connected, expected 1: ${matches + .map((p) => p.path) + .join(', ')}` + } +} + +/** + * Type the port into the COM input. + * + * The field is a freeSolo Autocomplete, so setting the text is enough -- there + * is no need to refresh the list first and pick the option out of it. + */ +export async function selectComPort(p: Page, port: string): Promise { + await p.getByTestId('rtu-com-input').locator('input').fill(port) +} diff --git a/e2e/fixtures/helpers.ts b/e2e/fixtures/helpers.ts index 3e9e8ee..d8b0e6d 100644 --- a/e2e/fixtures/helpers.ts +++ b/e2e/fixtures/helpers.ts @@ -296,10 +296,16 @@ export async function connectClient( await expect(p.getByTestId('connect-btn')).toContainText('Disconnect', { timeout: 5000 }) } -/** Disconnect client */ +/** + * Disconnect client. + * + * The wait is long because closing a real serial port is slow: measured at + * roughly 4.5s on a Windows COM port, against 5s here before. TCP and socat + * ptys close in milliseconds, so only the hardware specs ever lost that race. + */ export async function disconnectClient(p: Page): Promise { await p.getByTestId('connect-btn').click() - await expect(p.getByTestId('connect-btn')).toContainText('Connect', { timeout: 5000 }) + await expect(p.getByTestId('connect-btn')).toContainText('Connect', { timeout: 15000 }) } /** diff --git a/e2e/fixtures/require-openable-ports.ts b/e2e/fixtures/require-openable-ports.ts index c93db6f..e307038 100644 --- a/e2e/fixtures/require-openable-ports.ts +++ b/e2e/fixtures/require-openable-ports.ts @@ -1,5 +1,5 @@ import { accessSync, constants, readdirSync } from 'fs' -import { join } from 'path' +import { join, posix } from 'path' export const DEV_DIR = '/dev' export const SERIAL_PREFIXES = ['ttyUSB', 'ttyACM'] @@ -17,7 +17,9 @@ export const SERIAL_GROUP = 'dialout' export function assertOpenable(unreadable: string[]): void { if (unreadable.length === 0) return - const paths = unreadable.map((p) => join(DEV_DIR, p)) + // posix.join, not join: this path is quoted to the reader as a Linux device, + // so it keeps its separators on a Windows machine running the unit tests. + const paths = unreadable.map((p) => posix.join(DEV_DIR, p)) const subject = paths.length === 1 ? `${paths[0]} exists but is` : `${paths.join(', ')} exist but are` diff --git a/e2e/specs/99-hardware/01-iem3000-rtu.spec.ts b/e2e/specs/99-hardware/01-iem3000-rtu.spec.ts index 978d0e2..74679fb 100644 --- a/e2e/specs/99-hardware/01-iem3000-rtu.spec.ts +++ b/e2e/specs/99-hardware/01-iem3000-rtu.spec.ts @@ -4,13 +4,10 @@ * Requires a physical Arduino Uno running tools/arduino/iem3000.ino * connected via USB serial (9600 baud, 8N1, Slave ID 1). * - * Run headed so you can interact with the Playwright Inspector pause dialog: - * npx playwright test e2e/specs/99-hardware/ --headed + * The port is found by USB vendor ID, so the run is unattended: + * yarn test:e2e:hardware * - * When page.pause() triggers: - * 1. Click the refresh button next to COM port to scan available ports - * 2. Select the Arduino's COM port from the dropdown - * 3. Click "Resume" in the Playwright Inspector + * With no Arduino attached the whole suite skips. */ import { test, expect } from '../../fixtures/electron-app' import { @@ -22,15 +19,30 @@ import { enableReadConfiguration, disableReadConfiguration, scrollCell, + expectCellContains, clearData } from '../../fixtures/helpers' import { resolve } from 'path' +import { findArduinoPort, selectComPort } from '../../fixtures/arduino-port' const CONFIG_DIR = resolve(__dirname, '../../fixtures/config-files') const CLIENT_CONFIG = resolve(CONFIG_DIR, 'client-iem3000.json') const CLIENT_CONFIG_ERROR = resolve(CONFIG_DIR, 'client-iem3000-error.json') +/** Gap between enabling read-configuration and the read it must serve. */ +const READ_SETTLE_MS = 250 + test.describe.serial('Hardware — iEM3000 RTU (Arduino emulator)', () => { + let arduinoPort = '' + + // Skip rather than fail: the mac and linux rounds run this suite, and a + // machine without the board should not block a release round over it. + test.beforeAll(async () => { + const choice = await findArduinoPort() + if (choice.reason) test.skip(true, choice.reason) + arduinoPort = choice.port as string + }) + // ─── Setup ────────────────────────────────────────────────────────── test('navigate to client view', async ({ mainPage }) => { @@ -41,21 +53,8 @@ test.describe.serial('Hardware — iEM3000 RTU (Arduino emulator)', () => { await connectClientRTU(mainPage, '1', '9600', 'none', '8', '1') }) - test('pause — select COM port manually, then resume', async ({ mainPage }) => { - // eslint-disable-next-line no-console - console.log( - '\n╔══════════════════════════════════════════════════════════════╗\n' + - '║ MANUAL STEP: Select the Arduino COM port ║\n' + - '║ ║\n' + - '║ 1. Click the refresh button (↻) next to the COM port ║\n' + - '║ 2. Select the Arduino serial port from the dropdown ║\n' + - '║ (e.g. /dev/ttyUSB0, /dev/tty.usbmodem*, COM3) ║\n' + - '║ 3. Click "Resume" in the Playwright Inspector ║\n' + - '║ ║\n' + - '║ If no Arduino is connected, close the Inspector to skip. ║\n' + - '╚══════════════════════════════════════════════════════════════╝\n' - ) - await mainPage.pause() + test('select the Arduino COM port', async ({ mainPage }) => { + await selectComPort(mainPage, arduinoPort) }) test('load iEM3000 client config', async ({ mainPage }) => { @@ -82,14 +81,16 @@ test.describe.serial('Hardware — iEM3000 RTU (Arduino emulator)', () => { test.setTimeout(30_000) await enableReadConfiguration(mainPage) + // The toggle reports Mui-selected before the app can serve a read, and a + // read fired inside that window is dropped. Measured: 10ms is too early, + // 100ms is enough. + await mainPage.waitForTimeout(READ_SETTLE_MS) // Trigger read await mainPage.getByTestId('read-btn').click() - // Wait for rows to populate (26 float registers = 52 individual register words + extra rows) - await mainPage.waitForTimeout(5000) - const rowCount = await mainPage.locator('.MuiDataGrid-row').count() - expect(rowCount).toBeGreaterThan(0) + // Wait for the read itself rather than a fixed guess at how long it takes. + await expectCellContains(mainPage, 2999, 'word_float', '.') }) // ─── Value verification ──────────────────────────────────────────── @@ -165,12 +166,10 @@ test.describe.serial('Hardware — iEM3000 RTU (Arduino emulator)', () => { // Clear and re-read await clearData(mainPage) await mainPage.getByTestId('read-btn').click() - await mainPage.waitForTimeout(5000) - - const after = await scrollCell(mainPage, 2999, 'word_float') - // With noise the float values should differ slightly - expect(after).not.toBe(before) + // With noise the float values should differ slightly. Poll for that rather + // than guessing how long a re-read takes. + await expect.poll(() => scrollCell(mainPage, 2999, 'word_float')).not.toBe(before) }) // ─── Illegal address error ───────────────────────────────────────── @@ -184,14 +183,11 @@ test.describe.serial('Hardware — iEM3000 RTU (Arduino emulator)', () => { // Re-enable readConfiguration (loadClientConfig resets it to false) await enableReadConfiguration(mainPage) + await mainPage.waitForTimeout(READ_SETTLE_MS) - // Trigger read — wait long enough for the valid group + timeout on the illegal group + // Trigger read, then wait for the error to land instead of a fixed 10s. await mainPage.getByTestId('read-btn').click() - await mainPage.waitForTimeout(10_000) - - // Scroll to address 3109 and check error message - const errorText = await scrollCell(mainPage, 3109, 'value') - expect(errorText).toContain('Illegal data address') + await expectCellContains(mainPage, 3109, 'value', 'Illegal data address') }) test('valid addresses still show data after error config', async ({ mainPage }) => { diff --git a/e2e/specs/99-hardware/02-iem3000-reconnect.spec.ts b/e2e/specs/99-hardware/02-iem3000-reconnect.spec.ts index c67c3d5..52d4c08 100644 --- a/e2e/specs/99-hardware/02-iem3000-reconnect.spec.ts +++ b/e2e/specs/99-hardware/02-iem3000-reconnect.spec.ts @@ -7,8 +7,8 @@ * Requires a physical Arduino Uno running tools/arduino/iem3000.ino * connected via USB serial (9600 baud, 8N1, Slave ID 1). * - * Run headed: - * npx playwright test e2e/specs/99-hardware/02-iem3000-reconnect.spec.ts --headed + * The port is found by USB vendor ID, so the run is unattended: + * yarn test:e2e:hardware */ import { test, @@ -23,14 +23,18 @@ import { enableReadConfiguration, disableReadConfiguration, loadClientConfig, - scrollCell + scrollCell, + expectCellContains } from '../../fixtures/helpers' import { launchOptions } from '../../fixtures/launch' +import { findArduinoPort, selectComPort } from '../../fixtures/arduino-port' + const CLIENT_CONFIG = resolve(__dirname, '../../fixtures/config-files/client-iem3000.json') let app: ElectronApplication let page: Page +let arduinoPort = '' async function launchApp(clearStorage = true): Promise { app = await electron.launch(launchOptions()) @@ -68,10 +72,10 @@ async function connectAndRead(): Promise { await enableReadConfiguration(page) await page.getByTestId('read-btn').click() - await page.waitForTimeout(5000) - const rowCount = await page.locator('.MuiDataGrid-row').count() - expect(rowCount).toBeGreaterThan(0) + // Wait for a real value, not for rows: the grid keeps its rows from the + // config, so a row count says nothing about whether the read landed. + await expectCellContains(page, 2999, 'word_float', '.') } async function verifyPhaseCurrents(): Promise { @@ -87,13 +91,21 @@ async function closeApp(): Promise { // Disconnect await page.getByTestId('connect-btn').click() - await expect(page.getByTestId('connect-btn')).toContainText('Connect', { timeout: 5000 }) + await expect(page.getByTestId('connect-btn')).toContainText('Connect', { timeout: 15000 }) await app.close() await new Promise((r) => setTimeout(r, 1000)) } test.describe.serial('Hardware — iEM3000 RTU reconnect after restart', () => { + // Skip rather than fail: the mac and linux rounds run this suite, and a + // machine without the board should not block a release round over it. + test.beforeAll(async () => { + const choice = await findArduinoPort() + if (choice.reason) test.skip(true, choice.reason) + arduinoPort = choice.port as string + }) + test.afterAll(async () => { if (app) await app.close().catch(() => {}) }) @@ -120,18 +132,8 @@ test.describe.serial('Hardware — iEM3000 RTU reconnect after restart', () => { await page.getByRole('option', { name: '9600' }).click() }) - test('session 1 — pause for COM port selection', async () => { - // eslint-disable-next-line no-console - console.log( - '\n╔══════════════════════════════════════════════════════════════╗\n' + - '║ MANUAL STEP: Select the Arduino COM port ║\n' + - '║ ║\n' + - '║ 1. Click the refresh button (↻) next to the COM port ║\n' + - '║ 2. Select the Arduino serial port from the dropdown ║\n' + - '║ 3. Click "Resume" in the Playwright Inspector ║\n' + - '╚══════════════════════════════════════════════════════════════╝\n' - ) - await page.pause() + test('session 1 — select the Arduino COM port', async () => { + await selectComPort(page, arduinoPort) }) test('session 1 — load config, connect, and read', async () => { @@ -190,7 +192,7 @@ test.describe.serial('Hardware — iEM3000 RTU reconnect after restart', () => { // Disconnect await page.getByTestId('connect-btn').click() - await expect(page.getByTestId('connect-btn')).toContainText('Connect', { timeout: 5000 }) + await expect(page.getByTestId('connect-btn')).toContainText('Connect', { timeout: 15000 }) // Switch back to TCP await page.getByTestId('protocol-tcp-btn').click() diff --git a/package.json b/package.json index 4da9e50..9fdfb0d 100644 --- a/package.json +++ b/package.json @@ -34,12 +34,15 @@ "test": "vitest run", "test:watch": "vitest", "test:e2e": "electron-vite build && playwright test", + "verify": "yarn lint && yarn typecheck && yarn test && yarn test:e2e", "test:e2e:packaged": "electron-vite build && electron-builder --dir && playwright test --config playwright.packaged.config.ts", "test:e2e:hardware": "electron-vite build && playwright test --config playwright.hardware.config.ts", "test:e2e:privileged-port": "electron-vite build && playwright test --config playwright.privileged-port.config.ts", "test:e2e:scan-perf": "electron-vite build && playwright test --config playwright.scan-perf.config.ts", "presentation": "electron-vite build && playwright test --config playwright.presentation.config.ts", - "checkup": "yarn lint && yarn typecheck && yarn test && yarn test:e2e", + "test:all:mac": "yarn lint && yarn typecheck && yarn test && yarn test:e2e && yarn test:e2e:packaged && yarn presentation && yarn test:e2e:scan-perf && yarn test:e2e:hardware", + "test:all:windows": "yarn lint && yarn typecheck && yarn test && yarn test:e2e && yarn test:e2e:packaged && yarn presentation && yarn test:e2e:scan-perf && yarn test:e2e:hardware", + "test:all:linux": "yarn lint && yarn typecheck && yarn test && yarn test:e2e && yarn test:e2e:packaged && yarn presentation && yarn test:e2e:scan-perf && yarn test:e2e:privileged-port && yarn test:e2e:hardware", "socat": "socat -d -d pty,raw,echo=0,link=/tmp/ttyV0 pty,raw,echo=0,link=/tmp/ttyV1" }, "dependencies": { diff --git a/playwright.config.ts b/playwright.config.ts index 05f24a0..643bb83 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -4,9 +4,10 @@ export default defineConfig({ testDir: './e2e/specs', // Two suites stay out of the always-on pipeline. // - // The hardware specs need an Arduino on a serial port and a human to pick the - // COM port at a page.pause(), so an unattended run sits there forever. Run - // them with `yarn test:e2e:hardware`. + // The hardware specs need an Arduino on a serial port. They find it by USB + // vendor ID and skip themselves when none is attached, so they no longer need + // anyone at the keyboard -- but CI has no board, so there is nothing for them + // to do there. Run them with `yarn test:e2e:hardware`. // // The privileged port modal needs the kernel to actually refuse port 502, and // only root can arrange that. The spec pauses for a sudo command in a real diff --git a/playwright.hardware.config.ts b/playwright.hardware.config.ts index a190f00..71f5397 100644 --- a/playwright.hardware.config.ts +++ b/playwright.hardware.config.ts @@ -2,8 +2,8 @@ import { defineConfig } from '@playwright/test' import base from './playwright.config' // The way back into the specs playwright.config.ts leaves out. Requires an -// Arduino running tools/arduino/iem3000.ino on a serial port, and a person to -// select the COM port when the run pauses for it. +// Arduino running tools/arduino/iem3000.ino on a serial port; the port is found +// by USB vendor ID, and the suite skips itself when no board is attached. export default defineConfig({ ...base, testDir: './e2e/specs/99-hardware',