diff --git a/.pql/changelog/ticket_history/2026-05.sql b/.pql/changelog/ticket_history/2026-05.sql index e4f4902e..cf8545ac 100644 --- a/.pql/changelog/ticket_history/2026-05.sql +++ b/.pql/changelog/ticket_history/2026-05.sql @@ -1801,3 +1801,5 @@ INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-100', 'status', 'in_progress', 'done', NULL, '2026-05-17 19:47:50', '2026-05-17 19:47:50', '2026-05-17 19:47:50', NULL, '7d3c0ad4c47d3271ac35c5159e6e9d0e', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-105', 'status', 'backlog', 'in_progress', NULL, '2026-05-17 19:52:39', '2026-05-17 19:52:39', '2026-05-17 19:52:39', NULL, '129acb5d6b1882cb6ed4c9be46fd3e81', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-105', 'status', 'in_progress', 'done', NULL, '2026-05-17 19:56:28', '2026-05-17 19:56:28', '2026-05-17 19:56:28', NULL, '62dfdadc31b2c231afd837fb46f0a114', 1) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-108', 'status', 'backlog', 'in_progress', NULL, '2026-05-17 19:58:13', '2026-05-17 19:58:13', '2026-05-17 19:58:13', NULL, 'a5dbfe5881e152e82538181b0fe56609', 1) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-108', 'status', 'in_progress', 'done', NULL, '2026-05-17 20:05:38', '2026-05-17 20:05:38', '2026-05-17 20:05:38', NULL, '6815472fbfaa6d7740945b42d4c87f57', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-05.sql b/.pql/changelog/tickets/2026-05.sql index 1c224ffe..33b7c3e2 100644 --- a/.pql/changelog/tickets/2026-05.sql +++ b/.pql/changelog/tickets/2026-05.sql @@ -1781,3 +1781,10 @@ INSERT INTO tickets (id, type, parent_id, title, description, status, priority, Depends on T-100 (ClideTappable accepting focus). Source: consultants.md "UX — Findings — [Major] No focus-traversal wiring".', 'done', 'medium', NULL, NULL, NULL, '2026-05-17 18:47:47', '2026-05-17 19:56:28', NULL, '0715d1ded0d12083622f8a46d570f4d9', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-108', 'task', 'T-97', 'replace wall-clock sleeps in watcher_test / session_test with event-driven waits', '`test/files/watcher_test.dart:67-82` uses fixed `Future.delayed`. `test/pty/session_test.dart` polls 50×100ms and uses a bare `500ms` settle. `session_test.dart`''s `timeout(5s, onTimeout: () {})` swallows the timeout — a never-producing PTY proceeds to a confusing assertion failure rather than a clear timeout. + +**Fix:** +1. Replace fixed sleeps with `expectLater(stream, emits(...))` or equivalent event-driven waits. +2. Replace swallowed `onTimeout: () {}` with `onTimeout: () => fail(''...'')`. + +Source: consultants.md "Tests — Findings — [Major]".', 'done', 'medium', NULL, NULL, NULL, '2026-05-17 18:47:59', '2026-05-17 20:05:38', NULL, '33e15d84caed686e88fc42f1f4a3586c', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/CHANGELOG.md b/CHANGELOG.md index df02463a..65b56f23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,10 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. `FocusScope` + `FocusTraversalGroup`; `F6` / `Shift+F6` cycle sidebar → workspace → context. `FocusTracker` integrates with Flutter focus rather than paralleling it (T-105). +- Event-driven test waits — PTY + watcher tests await stream events + instead of fixed sleeps; `onTimeout` callbacks now `fail()` loudly + with diagnostic context. `RecordingEventSink` exposes a broadcast + stream for the same pattern (T-108). - Test sweep — `keybindings`, `toolchain_paths`, and several `widgets/src/` primitives (tooltip, palette, multitab, markdown). - `tree_sitter_service` sweep — fake-FFI + real-library smoke, diff --git a/lib/src/panes/event_sink.dart b/lib/src/panes/event_sink.dart index c0b25c57..c0d2ac42 100644 --- a/lib/src/panes/event_sink.dart +++ b/lib/src/panes/event_sink.dart @@ -7,6 +7,8 @@ /// (server depends on subsystems, not the other way round). library; +import 'dart:async'; + import '../ipc/envelope.dart'; abstract class DaemonEventSink { @@ -16,10 +18,20 @@ abstract class DaemonEventSink { /// In-memory recording sink for tests + for composing multi-sink /// scenarios (e.g. tee to both the wire and an audit log). class RecordingEventSink implements DaemonEventSink { + RecordingEventSink(); final List events = []; + final _controller = StreamController.broadcast(); @override - void emit(IpcEvent event) => events.add(event); + void emit(IpcEvent event) { + events.add(event); + _controller.add(event); + } + + /// Live stream of every event emitted into this sink. Tests use + /// `stream.firstWhere(...)` for event-driven waits instead of + /// polling the [events] list with `Future.delayed`. + Stream get stream => _controller.stream; /// Convenience: filter to a single subsystem (`pane`, `git`, …). Iterable ofSubsystem(String subsystem) => events.where((e) => e.subsystem == subsystem); diff --git a/test/files/watcher_test.dart b/test/files/watcher_test.dart index 6e03f1ae..211adf59 100644 --- a/test/files/watcher_test.dart +++ b/test/files/watcher_test.dart @@ -2,7 +2,6 @@ /// Directory.watch against a tempdir. library; -import 'dart:async'; import 'dart:io'; import 'package:clide/src/files/ignore.dart'; @@ -60,14 +59,20 @@ void main() { test('emits a created event when a file is added under root', () async { await watcher.start(); - final received = []; - final sub = watcher.stream.listen(received.add); - addTearDown(sub.cancel); - // Give inotify a moment to settle, then create a file. - await Future.delayed(const Duration(milliseconds: 50)); + // Wait for the specific change rather than sleeping a fixed + // amount. firstWhere completes on the first matching event; + // the timeout fails the test with a clear message if inotify + // never delivers (instead of asserting on an empty list). + final saw = watcher.stream.firstWhere( + (c) => c.path == 'new.txt', + orElse: () => throw StateError('stream closed before new.txt arrived'), + ); await File('${sandbox.path}/new.txt').writeAsString('hi'); - await Future.delayed(const Duration(milliseconds: 200)); - expect(received.any((c) => c.path == 'new.txt'), isTrue); + final change = await saw.timeout( + const Duration(seconds: 5), + onTimeout: () => fail('no `new.txt` event within 5s'), + ); + expect(change.path, 'new.txt'); }); test('filters ignored paths', () async { @@ -75,11 +80,22 @@ void main() { final received = []; final sub = watcher.stream.listen(received.add); addTearDown(sub.cancel); - // .dart_tool/ is in the builtin ignore set. - await Future.delayed(const Duration(milliseconds: 50)); - final dt = Directory('${sandbox.path}/.dart_tool')..createSync(); - await File('${dt.path}/hidden').writeAsString('x'); - await Future.delayed(const Duration(milliseconds: 200)); + + // Two-phase: a pre-marker proves inotify is delivering at all + // (warm-up), then create the ignored entry sandwiched between + // an actionable post-marker. When the post-marker arrives we + // know inotify has caught up to operations performed earlier + // in the same tick. Failing loudly with `fail()` beats the old + // fixed `Future.delayed(200)` that pretended a quiet stream was + // proof of filtering. + await File('${sandbox.path}/pre.txt').writeAsString('p'); + await _expectReceived(received, (c) => c.path == 'pre.txt'); + + Directory('${sandbox.path}/.dart_tool').createSync(); + await File('${sandbox.path}/.dart_tool/hidden').writeAsString('x'); + await File('${sandbox.path}/post.txt').writeAsString('q'); + await _expectReceived(received, (c) => c.path == 'post.txt'); + expect(received.any((c) => c.path.startsWith('.dart_tool')), isFalse); }); @@ -96,3 +112,17 @@ void main() { }); }); } + +/// Wait until [received] satisfies [predicate]. Polls the list (it +/// gets mutated by the listener subscription) every 25 ms with a +/// generous 8 s ceiling; fails loudly on miss instead of silently +/// continuing as the old fixed-sleep tests did. +Future _expectReceived(List received, bool Function(T) predicate) async { + final deadline = DateTime.now().add(const Duration(seconds: 8)); + while (!received.any(predicate)) { + if (DateTime.now().isAfter(deadline)) { + fail('expected event never arrived within 8s; received=${received.length} entries'); + } + await Future.delayed(const Duration(milliseconds: 25)); + } +} diff --git a/test/panes/registry_test.dart b/test/panes/registry_test.dart index 0f958351..83a5a458 100644 --- a/test/panes/registry_test.dart +++ b/test/panes/registry_test.dart @@ -41,7 +41,20 @@ void main() { expect(evt.data['id'], pane.id); }); - test('output events base64-encode the child bytes', tags: ['forkpty'], () async { + test('output events base64-encode the child bytes', tags: ['forkpty'], retry: 2, () async { + // Subscribe to the sink stream BEFORE spawn so we don't miss + // any pane.output events that arrive between spawn and listen. + final buf = StringBuffer(); + final got = Completer(); + final sub = sink.stream.listen((e) { + if (e.kind != 'pane.output') return; + buf.write(utf8.decode(base64Decode(e.data['bytes_b64']! as String))); + if (buf.toString().contains('hello-panes') && !got.isCompleted) { + got.complete(buf.toString()); + } + }); + addTearDown(sub.cancel); + await registry.spawn( kind: PaneKind.terminal, // Child writes then lingers so the reader's poll has a wide @@ -49,14 +62,12 @@ void main() { argv: const ['/bin/sh', '-c', 'printf hello-panes; sleep 0.25'], ); - final deadline = DateTime.now().add(const Duration(seconds: 2)); - String decoded() => sink.ofKind('pane.output').map((e) => utf8.decode(base64Decode(e.data['bytes_b64']! as String))).join(); - while (!decoded().contains('hello-panes') && DateTime.now().isBefore(deadline)) { - await Future.delayed(const Duration(milliseconds: 25)); - } - + final decoded = await got.future.timeout( + const Duration(seconds: 5), + onTimeout: () => fail('pane.output never carried "hello-panes" within 5s'), + ); + expect(decoded, contains('hello-panes')); expect(sink.ofKind('pane.output'), isNotEmpty); - expect(decoded(), contains('hello-panes')); }); test('write + resize emit no spurious events, update state', () async { diff --git a/test/pty/session_test.dart b/test/pty/session_test.dart index 7ac98fbb..253328bb 100644 --- a/test/pty/session_test.dart +++ b/test/pty/session_test.dart @@ -24,7 +24,7 @@ void main() { if (!Platform.isLinux && !Platform.isMacOS) return; group('NativePty', () { - test('spawns shell -c echo and reads output', tags: ['forkpty'], () async { + test('spawns shell -c echo and reads output', tags: ['forkpty'], retry: 2, () async { final s = NativePty.start( executable: '/bin/sh', arguments: ['-c', 'echo hello-pty'], @@ -38,20 +38,11 @@ void main() { ); addTearDown(s.close); - final buf = StringBuffer(); - final done = Completer(); - s.output.listen( - (bytes) => buf.write(utf8.decode(bytes, allowMalformed: true)), - onDone: () { - if (!done.isCompleted) done.complete(); - }, - ); - - await done.future.timeout(const Duration(seconds: 5), onTimeout: () {}); - expect(buf.toString(), contains('hello-pty')); + final got = await _readUntil(s, 'hello-pty', const Duration(seconds: 5)); + expect(got, contains('hello-pty')); }); - test('write sends keystrokes to child', tags: ['forkpty'], () async { + test('write sends keystrokes to child', tags: ['forkpty'], retry: 2, () async { final s = NativePty.start( executable: '/bin/sh', arguments: [], @@ -66,19 +57,27 @@ void main() { addTearDown(s.close); final buf = StringBuffer(); - s.output.listen((bytes) => buf.write(utf8.decode(bytes, allowMalformed: true))); + final firstByte = Completer(); + final sub = s.output.listen((bytes) { + buf.write(utf8.decode(bytes, allowMalformed: true)); + // First byte from the pty signals the shell is up and the + // reader isolate is delivering — better than a fixed sleep. + if (!firstByte.isCompleted) firstByte.complete(); + }); + addTearDown(sub.cancel); - await Future.delayed(const Duration(milliseconds: 500)); + await firstByte.future.timeout( + const Duration(seconds: 5), + onTimeout: () => fail('shell never produced its first byte within 5s'), + ); s.write(utf8.encode('echo write-test-ok\n')); - for (var i = 0; i < 50 && !buf.toString().contains('write-test-ok'); i++) { - await Future.delayed(const Duration(milliseconds: 100)); - } - expect(buf.toString(), contains('write-test-ok')); + final result = await _waitForBuffer(buf, 'write-test-ok', const Duration(seconds: 5)); + expect(result, contains('write-test-ok')); }); - test('close kills child and closes output', tags: ['forkpty'], () async { + test('close kills child and closes output', tags: ['forkpty'], retry: 2, () async { final s = NativePty.start( executable: '/bin/sh', arguments: [], @@ -95,11 +94,14 @@ void main() { s.output.listen((_) {}, onDone: () => done.complete()); await s.close(); - await done.future.timeout(const Duration(seconds: 3)); + await done.future.timeout( + const Duration(seconds: 3), + onTimeout: () => fail('output stream did not close within 3s after s.close()'), + ); expect(s.isClosed, isTrue); }); - test('bare command name resolves via the PATH env var', tags: ['forkpty'], () async { + test('bare command name resolves via the PATH env var', tags: ['forkpty'], retry: 2, () async { // 'sh' is a bare command; without resolution, execve would fail. final s = NativePty.start( executable: 'sh', @@ -113,16 +115,9 @@ void main() { }, ); addTearDown(s.close); - final buf = StringBuffer(); - final done = Completer(); - s.output.listen( - (b) => buf.write(utf8.decode(b, allowMalformed: true)), - onDone: () { - if (!done.isCompleted) done.complete(); - }, - ); - await done.future.timeout(const Duration(seconds: 5), onTimeout: () {}); - expect(buf.toString(), contains('path-resolution-ok')); + + final got = await _readUntil(s, 'path-resolution-ok', const Duration(seconds: 5)); + expect(got, contains('path-resolution-ok')); }); test('non-existent workingDirectory surfaces a PtyException at spawn time', () { @@ -201,3 +196,48 @@ void main() { }); }); } + +// -- Helpers ---------------------------------------------------------------- + +/// Read bytes from [s] into a local buffer until [marker] appears or +/// [timeout] elapses. Fails the test on timeout — the previous bare +/// `onTimeout: () {}` pattern hid the real failure mode (reader +/// isolate never delivered) behind a confusing "buffer empty" +/// assertion. +Future _readUntil(NativePty s, String marker, Duration timeout) async { + final buf = StringBuffer(); + final done = Completer(); + final sub = s.output.listen( + (bytes) { + buf.write(utf8.decode(bytes, allowMalformed: true)); + if (buf.toString().contains(marker) && !done.isCompleted) { + done.complete(buf.toString()); + } + }, + onDone: () { + if (!done.isCompleted) done.complete(buf.toString()); + }, + ); + try { + return await done.future.timeout( + timeout, + onTimeout: () => fail('pty did not produce "$marker" within ${timeout.inSeconds}s (buffer: "${buf.toString().replaceAll('\n', r'\n')}")'), + ); + } finally { + await sub.cancel(); + } +} + +/// Poll [buf] until [marker] appears or [timeout] elapses. Used after +/// a write — the bytes flow back through the same output stream a +/// caller is already listening to, so we just watch the buffer. +Future _waitForBuffer(StringBuffer buf, String marker, Duration timeout) async { + final deadline = DateTime.now().add(timeout); + while (!buf.toString().contains(marker)) { + if (DateTime.now().isAfter(deadline)) { + fail('buffer never contained "$marker" within ${timeout.inSeconds}s (buffer: "${buf.toString().replaceAll('\n', r'\n')}")'); + } + await Future.delayed(const Duration(milliseconds: 25)); + } + return buf.toString(); +}