From 889058db1b0e52537ee64bd9f9125599fe580e2d Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 11 May 2026 19:06:17 +0200 Subject: [PATCH] test sweep: cover kernel/src/{events,ipc} (T-91) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three new test files + a small DaemonClient dispose-safety fix: - test/kernel/src/events/types_test.dart (7 tests): every ClideEvent subclass's subsystem / kind / payload contract + the ClideEventEnvelope v1 JSON shape. - test/kernel/src/events/message_bus_test.dart (6 tests): Message shape, MessageBus publish/subscribe/dispose, filter-by-publisher, filter-by-channel, intersection. - test/kernel/src/ipc/client_test.dart (9 tests): real Unix-socket roundtrip via a _TestDaemon helper — connect + correlate request/ response, event forwarding to the DaemonBus, malformed-line skip, daemon-disconnect failing pending requests, stop cleanup, dispose, connect-failure-then-reconnect, daemon-sent-Request warn-and-skip, DaemonConnectionChanged emission. Fix in lib/kernel/src/ipc/client.dart: _setConnected now skips notifyListeners / event emit when _disposed. The socket stream's onDone can fire after dispose runs, which previously hit ChangeNotifier's "used after disposed" assertion. State flip stays unconditional so stop()'s explicit transition still works. Coverage: ipc/client.dart 14% -> 92% (79/86; remaining 7 lines are the socket onError callback + 1 const ctor phantom); events/types .dart 95% (37/39 — 2 const-ctor phantoms); events/message_bus.dart 100%; events/bus.dart stays 100%. Total coverage 71.93% -> 73.34%; floor bumped to 73. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/kernel/src/ipc/client.dart | 4 + pubspec.yaml | 2 +- test/kernel/src/events/message_bus_test.dart | 74 +++++ test/kernel/src/events/types_test.dart | 77 +++++ test/kernel/src/ipc/client_test.dart | 280 +++++++++++++++++++ 5 files changed, 436 insertions(+), 1 deletion(-) create mode 100644 test/kernel/src/events/message_bus_test.dart create mode 100644 test/kernel/src/events/types_test.dart create mode 100644 test/kernel/src/ipc/client_test.dart diff --git a/lib/kernel/src/ipc/client.dart b/lib/kernel/src/ipc/client.dart index db2b7dd4..67954d49 100644 --- a/lib/kernel/src/ipc/client.dart +++ b/lib/kernel/src/ipc/client.dart @@ -149,6 +149,10 @@ class DaemonClient extends ChangeNotifier { void _setConnected(bool v) { if (_connected == v) return; _connected = v; + // Skip side-effects (event emit + notifyListeners) after dispose — + // the socket stream's onDone can fire post-dispose and would + // otherwise hit ChangeNotifier's "used after disposed" assert. + if (_disposed) return; _events.emit(DaemonConnectionChanged(connected: v)); notifyListeners(); } diff --git a/pubspec.yaml b/pubspec.yaml index 7e9d82a4..667b4cb5 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -18,7 +18,7 @@ repository: https://github.com/postmeridiem/clide # Pre-push line-coverage floor. Ratchets up only — see D-66. # Reading: `awk -F: '/^coverage_floor:/ {gsub(/ /,"",$2); print $2}' pubspec.yaml`. -coverage_floor: 71 +coverage_floor: 73 # Project metadata (was project.yaml, folded in per D-056). # version: above is the single source of truth. The Makefile reads diff --git a/test/kernel/src/events/message_bus_test.dart b/test/kernel/src/events/message_bus_test.dart new file mode 100644 index 00000000..e360a3e0 --- /dev/null +++ b/test/kernel/src/events/message_bus_test.dart @@ -0,0 +1,74 @@ +/// Unit tests for `MessageBus` in `lib/kernel/src/events/message_bus.dart`. +library; + +import 'package:clide/kernel/src/events/message_bus.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + group('Message', () { + test('address joins publisher and channel with a slash', () { + final m = Message(publisher: 'pty', channel: 'output', data: const {}); + expect(m.address, 'pty/output'); + }); + + test('timestamp is set at construction', () { + final before = DateTime.now(); + final m = Message(publisher: 'p', channel: 'c', data: const {}); + final after = DateTime.now(); + expect(m.timestamp.isBefore(before), isFalse); + expect(m.timestamp.isAfter(after), isFalse); + }); + }); + + group('MessageBus', () { + late MessageBus bus; + + setUp(() => bus = MessageBus()); + tearDown(() => bus.dispose()); + + test('publish delivers to every subscriber', () async { + final received = []; + final sub = bus.subscribe().listen(received.add); + bus.publish('git', 'status-changed', {'dirty': true}); + await Future.delayed(Duration.zero); + expect(received, hasLength(1)); + expect(received.first.publisher, 'git'); + expect(received.first.channel, 'status-changed'); + expect(received.first.data, {'dirty': true}); + await sub.cancel(); + }); + + test('subscribe filter by publisher narrows the stream', () async { + final got = []; + final sub = bus.subscribe(publisher: 'git').listen(got.add); + bus.publish('git', 'a', const {}); + bus.publish('pty', 'a', const {}); + bus.publish('git', 'b', const {}); + await Future.delayed(Duration.zero); + expect(got.map((m) => m.channel), ['a', 'b']); + await sub.cancel(); + }); + + test('subscribe filter by channel narrows the stream', () async { + final got = []; + final sub = bus.subscribe(channel: 'output').listen(got.add); + bus.publish('pty', 'output', const {}); + bus.publish('pty', 'exit', const {}); + bus.publish('git', 'output', const {}); + await Future.delayed(Duration.zero); + expect(got.map((m) => m.publisher), ['pty', 'git']); + await sub.cancel(); + }); + + test('subscribe with both filters intersects them', () async { + final got = []; + final sub = bus.subscribe(publisher: 'git', channel: 'status').listen(got.add); + bus.publish('git', 'status', const {}); + bus.publish('git', 'other', const {}); + bus.publish('pty', 'status', const {}); + await Future.delayed(Duration.zero); + expect(got, hasLength(1)); + await sub.cancel(); + }); + }); +} diff --git a/test/kernel/src/events/types_test.dart b/test/kernel/src/events/types_test.dart new file mode 100644 index 00000000..ee6e5c9d --- /dev/null +++ b/test/kernel/src/events/types_test.dart @@ -0,0 +1,77 @@ +/// Unit tests for the `ClideEvent` subclasses and `ClideEventEnvelope` +/// in `lib/kernel/src/events/types.dart`. +library; + +import 'package:clide/kernel/src/events/types.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + group('ClideEvent subclasses — subsystem / kind / payload', () { + test('DaemonConnectionChanged', () { + const e = DaemonConnectionChanged(connected: true); + expect(e.subsystem, 'ipc'); + expect(e.kind, 'connection-changed'); + expect(e.payload(), {'connected': true}); + }); + + test('ThemeChanged', () { + const e = ThemeChanged(themeName: 'midnight'); + expect(e.subsystem, 'theme'); + expect(e.kind, 'changed'); + expect(e.payload(), {'theme': 'midnight'}); + }); + + test('ProjectOpened', () { + const e = ProjectOpened(path: '/tmp/x'); + expect(e.subsystem, 'project'); + expect(e.kind, 'opened'); + expect(e.payload(), {'path': '/tmp/x'}); + }); + + test('ProjectClosed — empty payload default applies', () { + const e = ProjectClosed(); + expect(e.subsystem, 'project'); + expect(e.kind, 'closed'); + expect(e.payload(), isEmpty); + }); + + test('ExtensionActivated / ExtensionDeactivated', () { + const a = ExtensionActivated(id: 'builtin.git'); + expect(a.subsystem, 'extensions'); + expect(a.kind, 'activated'); + expect(a.payload(), {'id': 'builtin.git'}); + const d = ExtensionDeactivated(id: 'builtin.git'); + expect(d.subsystem, 'extensions'); + expect(d.kind, 'deactivated'); + expect(d.payload(), {'id': 'builtin.git'}); + }); + + test('DaemonEvent merges ts into payload', () { + final ts = DateTime.utc(2026, 5, 11, 12, 0, 0); + final e = DaemonEvent( + subsystem: 'pty', + kind: 'output', + data: {'bytes': 'aGVsbG8='}, + ts: ts, + ); + expect(e.subsystem, 'pty'); + expect(e.kind, 'output'); + expect(e.payload()['ts'], ts.toIso8601String()); + expect(e.payload()['bytes'], 'aGVsbG8='); + }); + }); + + group('ClideEventEnvelope', () { + test('toJson builds a v1 envelope around the event', () { + final ts = DateTime.utc(2026, 5, 11, 9, 30); + const event = ThemeChanged(themeName: 'paper'); + final envelope = ClideEventEnvelope(event, ts); + final json = envelope.toJson(); + expect(json['v'], 1); + expect(json['subsystem'], 'theme'); + expect(json['kind'], 'changed'); + expect(json['ts'], ts.toIso8601String()); + expect(json['data'], {'theme': 'paper'}); + }); + }); +} diff --git a/test/kernel/src/ipc/client_test.dart b/test/kernel/src/ipc/client_test.dart new file mode 100644 index 00000000..4330412b --- /dev/null +++ b/test/kernel/src/ipc/client_test.dart @@ -0,0 +1,280 @@ +/// Real-socket integration tests for `lib/kernel/src/ipc/client.dart` — +/// boots a Unix domain server in the test, has DaemonClient connect, +/// then exercises request/response correlation, event forwarding, +/// disconnect, reconnect, malformed-line resilience, and dispose. +library; + +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:clide/clide.dart'; +import 'package:clide/kernel/src/events/bus.dart'; +import 'package:clide/kernel/src/events/types.dart'; +import 'package:clide/kernel/src/ipc/client.dart'; +import 'package:clide/kernel/src/log.dart'; +import 'package:test/test.dart'; + +/// Minimal test daemon — binds a Unix socket, accepts one connection, +/// exposes the accepted socket so the test can write framed JSON back. +class _TestDaemon { + _TestDaemon(this.path); + final String path; + ServerSocket? _server; + Socket? _client; + final _onClient = Completer(); + final _lines = StreamController.broadcast(); + + Future start() async { + final addr = InternetAddress(path, type: InternetAddressType.unix); + _server = await ServerSocket.bind(addr, 0); + _server!.listen((socket) { + _client = socket; + if (!_onClient.isCompleted) _onClient.complete(socket); + socket.cast>().transform(utf8.decoder).transform(const LineSplitter()).listen(_lines.add); + }); + } + + Future waitForClient() => _onClient.future; + + Stream get lines => _lines.stream; + + void send(String line) { + _client!.writeln(line); + } + + Future close() async { + await _client?.close(); + await _server?.close(); + await _lines.close(); + } +} + +Future _tmpSocket() async { + final dir = await Directory.systemTemp.createTemp('clide-ipc-'); + return '${dir.path}/sock'; +} + +DaemonClient _build(String socketPath, DaemonBus bus) { + return DaemonClient( + socketPath: socketPath, + log: Logger(minLevel: LogLevel.error, sinks: const []), + events: bus, + ); +} + +void main() { + group('DaemonClient — happy path', () { + test('connect → request → matching response completes', () async { + final path = await _tmpSocket(); + final daemon = _TestDaemon(path); + await daemon.start(); + addTearDown(daemon.close); + + final bus = DaemonBus(); + addTearDown(bus.dispose); + final client = _build(path, bus); + addTearDown(client.dispose); + await client.start(); + + final socket = await daemon.waitForClient(); + expect(socket, isNotNull); + // Wait for _setConnected → notifyListeners to actually run. + await Future.delayed(const Duration(milliseconds: 50)); + expect(client.isConnected, isTrue); + + // Issue a request, capture the id off the wire, send back a response. + final lineFuture = daemon.lines.first; + final responseFuture = client.request('files.list', args: {'limit': 3}); + final reqLine = await lineFuture; + final reqJson = jsonDecode(reqLine) as Map; + expect(reqJson['cmd'], 'files.list'); + expect(reqJson['args'], {'limit': 3}); + final id = reqJson['id'] as String; + // Encode an IpcResponse using the framework's encoder. + daemon.send(IpcResponse.ok(id: id, data: {'files': []}).encode()); + final resp = await responseFuture; + expect(resp.ok, isTrue); + expect(resp.id, id); + expect(resp.data['files'], isEmpty); + }); + + test('event line from daemon emits on the bus', () async { + final path = await _tmpSocket(); + final daemon = _TestDaemon(path); + await daemon.start(); + addTearDown(daemon.close); + + final bus = DaemonBus(); + addTearDown(bus.dispose); + final client = _build(path, bus); + addTearDown(client.dispose); + await client.start(); + await daemon.waitForClient(); + await Future.delayed(const Duration(milliseconds: 50)); + + final eventFuture = bus.stream.firstWhere( + (e) => e.event is DaemonEvent, + ); + final ev = IpcEvent( + subsystem: 'pty', + kind: 'output', + data: {'bytes': 'aGVsbG8='}, + timestamp: DateTime.now(), + ); + daemon.send(ev.encode()); + final received = await eventFuture.timeout(const Duration(seconds: 2)); + final daemonEvent = received.event as DaemonEvent; + expect(daemonEvent.subsystem, 'pty'); + expect(daemonEvent.kind, 'output'); + }); + }); + + group('DaemonClient — error + lifecycle paths', () { + test('request while disconnected returns a not-connected error', () async { + // Don't connect — point at a non-existent socket path. + final bus = DaemonBus(); + addTearDown(bus.dispose); + final client = _build('/tmp/does-not-exist.sock', bus); + addTearDown(client.dispose); + final resp = await client.request('anything'); + expect(resp.ok, isFalse); + expect(resp.error?.message, contains('not connected')); + }); + + test('malformed line is logged and skipped, real lines still work', () async { + final path = await _tmpSocket(); + final daemon = _TestDaemon(path); + await daemon.start(); + addTearDown(daemon.close); + + final bus = DaemonBus(); + addTearDown(bus.dispose); + final client = _build(path, bus); + addTearDown(client.dispose); + await client.start(); + await daemon.waitForClient(); + await Future.delayed(const Duration(milliseconds: 50)); + + // Send garbage and an empty line — must not throw. + daemon.send('not json at all'); + daemon.send(''); + // Then a real response — should still arrive. + final lineFuture = daemon.lines.first; + final respFuture = client.request('ping'); + final line = await lineFuture; + final id = (jsonDecode(line) as Map)['id'] as String; + daemon.send(IpcResponse.ok(id: id, data: const {}).encode()); + final resp = await respFuture.timeout(const Duration(seconds: 2)); + expect(resp.ok, isTrue); + }); + + test('daemon disconnect fails pending requests and flips isConnected', () async { + final path = await _tmpSocket(); + final daemon = _TestDaemon(path); + await daemon.start(); + addTearDown(daemon.close); + + final bus = DaemonBus(); + addTearDown(bus.dispose); + final client = _build(path, bus); + addTearDown(client.dispose); + await client.start(); + final acceptedSocket = await daemon.waitForClient(); + await Future.delayed(const Duration(milliseconds: 50)); + expect(client.isConnected, isTrue); + + // Send a request but never respond — it should resolve with an error + // once the daemon closes the connection. + final respFuture = client.request('orphan'); + await acceptedSocket.close(); + final resp = await respFuture.timeout(const Duration(seconds: 2)); + expect(resp.ok, isFalse); + expect(resp.error?.message, contains('disconnect')); + // _handleDisconnect → _setConnected(false). + await Future.delayed(const Duration(milliseconds: 50)); + expect(client.isConnected, isFalse); + }); + + test('stop closes the socket cleanly and clears pending requests', () async { + final path = await _tmpSocket(); + final daemon = _TestDaemon(path); + await daemon.start(); + addTearDown(daemon.close); + + final bus = DaemonBus(); + addTearDown(bus.dispose); + final client = _build(path, bus); + await client.start(); + await daemon.waitForClient(); + await Future.delayed(const Duration(milliseconds: 50)); + + final pending = client.request('hang'); + await client.stop(); + final resp = await pending.timeout(const Duration(seconds: 2)); + expect(resp.ok, isFalse); + expect(resp.error?.message, contains('stopped')); + expect(client.isConnected, isFalse); + // dispose path + client.dispose(); + }); + + test('connect failure schedules a reconnect (covers _connect catch + _scheduleReconnect)', () async { + // Path that won't resolve — Socket.connect throws → _scheduleReconnect. + final bus = DaemonBus(); + addTearDown(bus.dispose); + final client = _build('/tmp/clide-ipc-no-such-${DateTime.now().microsecondsSinceEpoch}.sock', bus); + addTearDown(client.dispose); + await client.start(); + // Give the catch branch + reconnect-scheduling a tick. + await Future.delayed(const Duration(milliseconds: 50)); + expect(client.isConnected, isFalse); + }); + + test('daemon-sent IpcRequest line is logged and ignored', () async { + final path = await _tmpSocket(); + final daemon = _TestDaemon(path); + await daemon.start(); + addTearDown(daemon.close); + + final bus = DaemonBus(); + addTearDown(bus.dispose); + final client = _build(path, bus); + addTearDown(client.dispose); + await client.start(); + await daemon.waitForClient(); + await Future.delayed(const Duration(milliseconds: 50)); + + // Send a Request from the daemon — the case is matched and warned. + daemon.send(IpcRequest(id: 'X', cmd: 'should-not-happen', args: const {}).encode()); + // Real follow-up still works. + final lineFuture = daemon.lines.first; + final respFuture = client.request('ok'); + final line = await lineFuture; + final id = (jsonDecode(line) as Map)['id'] as String; + daemon.send(IpcResponse.ok(id: id, data: const {}).encode()); + final resp = await respFuture.timeout(const Duration(seconds: 2)); + expect(resp.ok, isTrue); + }); + + test('emits DaemonConnectionChanged on the bus when the connection state flips', () async { + final path = await _tmpSocket(); + final daemon = _TestDaemon(path); + await daemon.start(); + addTearDown(daemon.close); + + final bus = DaemonBus(); + addTearDown(bus.dispose); + final flips = []; + final sub = bus.on().listen((e) => flips.add(e.connected)); + addTearDown(sub.cancel); + + final client = _build(path, bus); + addTearDown(client.dispose); + await client.start(); + await daemon.waitForClient(); + await Future.delayed(const Duration(milliseconds: 100)); + expect(flips, contains(true)); + }); + }); +}