event-driven test waits, fail-loud on timeout (T-108)
Replaces the fixed Future.delayed sleeps the consultant flagged
with stream-based waits that complete when the awaited event
arrives. Timeout callbacks call fail() with a diagnostic instead
of `onTimeout: () {}` swallowing the signal — a never-producing
pty now reports "pty did not produce X within 5s" instead of an
unhelpful "Actual: ''".
session_test.dart:
- _readUntil helper subscribes to s.output, completes when a
marker substring appears (or onDone), fails on timeout.
- _waitForBuffer polls a buffer the listener is already filling
after a write; 25ms tick, 5s ceiling, fail-loud on miss.
- Drops the 500ms settle + 50×100ms polling pattern in the write
test; uses a "first-byte" completer for prompt-readiness.
- retry: 2 restored on the four read-dependent forkpty tests
(the underlying flutter-test-runner pty-output flake hasn't
fully gone away; recovers cleanly on a fresh spawn).
watcher_test.dart:
- "emits a created event" awaits stream.firstWhere instead of two
fixed sleeps.
- "filters ignored paths" uses pre + post sentinel markers to
bracket the inotify-delivery window event-driven; the negative
assertion only runs after the post marker is observed.
event_sink.dart:
- RecordingEventSink gains a broadcast `stream` for the same
event-await pattern. PaneRegistry's output test subscribes
BEFORE spawn so first bytes aren't lost.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<IpcEvent> events = [];
|
||||
final _controller = StreamController<IpcEvent>.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<IpcEvent> get stream => _controller.stream;
|
||||
|
||||
/// Convenience: filter to a single subsystem (`pane`, `git`, …).
|
||||
Iterable<IpcEvent> ofSubsystem(String subsystem) => events.where((e) => e.subsystem == subsystem);
|
||||
|
||||
@@ -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 = <FileChange>[];
|
||||
final sub = watcher.stream.listen(received.add);
|
||||
addTearDown(sub.cancel);
|
||||
// Give inotify a moment to settle, then create a file.
|
||||
await Future<void>.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<void>.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 = <FileChange>[];
|
||||
final sub = watcher.stream.listen(received.add);
|
||||
addTearDown(sub.cancel);
|
||||
// .dart_tool/ is in the builtin ignore set.
|
||||
await Future<void>.delayed(const Duration(milliseconds: 50));
|
||||
final dt = Directory('${sandbox.path}/.dart_tool')..createSync();
|
||||
await File('${dt.path}/hidden').writeAsString('x');
|
||||
await Future<void>.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<void> _expectReceived<T>(List<T> 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<void>.delayed(const Duration(milliseconds: 25));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<String>();
|
||||
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<void>.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 {
|
||||
|
||||
+72
-32
@@ -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<void>();
|
||||
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<void>();
|
||||
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<void>.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<void>.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<void>();
|
||||
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<String> _readUntil(NativePty s, String marker, Duration timeout) async {
|
||||
final buf = StringBuffer();
|
||||
final done = Completer<String>();
|
||||
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<String> _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<void>.delayed(const Duration(milliseconds: 25));
|
||||
}
|
||||
return buf.toString();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user