From 301b6becf4a9e6615377e8d809103f071b53d723 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 9 Jun 2026 23:43:16 +0200 Subject: [PATCH] un-quarantine app_test open-folder hang (T-280) The "Open Folder on a non-repo path" widget test wedged the runner ~10 minutes on a _RawReceivePort teardown hang. Root cause: project validation shelled out to `git rev-parse` via Process.run, whose exit ReceivePort leaks under the fake-async widget-test harness. The KernelFixture now injects a pure-Dart `.git`-walk validator (synchronous existsSync/typeSync, no native port), so the open-folder flow is subprocess-free. Un-skipped the test and scoped the switcher tap to the hat-bar ClideTappable to disambiguate it from the welcome overlay's "clide" wordmark. Co-Authored-By: Claude Opus 4.8 (1M context) --- .pql/changelog/ticket_history/2026-06.sql | 1 + .pql/changelog/tickets/2026-06.sql | 8 ++++++ test/app_test.dart | 30 +++++++++++------------ test/helpers/kernel_fixture.dart | 26 ++++++++++++++++++++ 4 files changed, 50 insertions(+), 15 deletions(-) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index 876d4809..644a8ee7 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -2288,3 +2288,4 @@ INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, - The in-progress spinner on conversation activity cards is comfortably distinguishable as a spinning indicator; success/error glyphs stay aligned at the same footprint.', NULL, '2026-06-09 21:22:44', '2026-06-09 21:22:44', '2026-06-09 21:22:44', NULL, '1aec6212db45140ee906592f9ce125ef', 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-280', 'status', 'backlog', 'ready', NULL, '2026-06-09 21:32:29', '2026-06-09 21:32:29', '2026-06-09 21:32:29', NULL, '0fcac3a23b0303186fefb36827374616', 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-298', 'status', 'in_progress', 'done', NULL, '2026-06-09 21:36:05', '2026-06-09 21:36:05', '2026-06-09 21:36:05', NULL, 'ecf870d28771c1368654195d93ec4dec', 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-280', 'status', 'ready', 'done', NULL, '2026-06-09 21:42:59', '2026-06-09 21:42:59', '2026-06-09 21:42:59', NULL, '11d1dc9865ac563878cd40691248240c', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index 7cf19e68..9579fe16 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -3783,3 +3783,11 @@ Expected: the input box sits correctly flush to the window bottom (or with consi Notes: - Audit the bottom-of-window layout stack (composer / interaction zone + status bar). The composer should bottom-anchor independently of the status bar''s visibility. - Related to the bottom interaction zone work (D-78) and may interact with T-297 (scroll on input resize). Also relevant once the status-bar collapse toggles (T-294) can change what''s in the bar.', 'done', 'medium', NULL, NULL, NULL, '2026-06-09 16:06:41', '2026-06-09 21:36:05', NULL, '162be954ae0cc51fa9db0c36b0173cbe', 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-280', 'bug', NULL, 'app_test ''Open Folder non-repo'' hangs 10min — runAsync/ReceivePort teardown wedge', 'Pre-existing (reproduces at base commit 3a78dfa, predates the T-267 conversation-rendering epic). The widget test `test/app_test.dart` › "Open Folder on a non-repo path surfaces the ''no git repo'' dialog" times out after 10 minutes; teardown is wedged on `_RawReceivePort._handleMessage`. + +Bisected (each a 45s-timeout repro, all on this box): +- A bare `Process.run` inside `tester.runAsync` (no app, no extensions) hangs → `Process.run`-in-`runAsync` leaks its exit ReceivePort here. +- `pumpApp` + empty `runAsync`, and `pumpApp` + a 1.5s real delay → both PASS (boot + runAsync alone is fine). +- The full openFolder tap flow hangs even when project validation is stubbed to a synchronous, pure-Dart `.git` walk (no subprocess) AND `runAsync` is removed — so the wedge is not solely the git subprocess; something in the booted-app + extensions + open-folder command path holds a native port that teardown waits on forever. + +Quarantined with `skip:` so the suite/gate stays green. Real fix: find the leaked native async resource (likely a Process/Isolate/FakeDaemonClient port reachable from the open-folder command or app boot under the test harness) and ensure it''s drained/cancelled before teardown — or drive the "no git repo" assertion without booting the resource. Then remove the skip.', 'done', 'high', NULL, NULL, NULL, '2026-06-08 11:29:22', '2026-06-09 21:42:59', NULL, '9154fcc7560d18aa35f4b718076203df', 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/test/app_test.dart b/test/app_test.dart index f7767b78..22d167f1 100644 --- a/test/app_test.dart +++ b/test/app_test.dart @@ -315,14 +315,16 @@ void main() { expect(tester.takeException(), isNull); }); - // QUARANTINED (T-280): this test wedges the runner for 10 minutes — teardown - // hangs on `_RawReceivePort._handleMessage`. Pre-existing (reproduces at the - // base commit, predates the T-267 epic) and not a `Process.run`/`runAsync` - // fix away — the booted-app + open-folder path holds a native port teardown - // never drains. Skipped to keep the gate green; see T-280 for the bisection - // and the real fix (drain the leaked resource, then remove this skip). - testWidgets('Open Folder on a non-repo path surfaces the "no git repo" dialog', (tester) async { - final tmp = await Directory.systemTemp.createTemp('clide-not-a-repo-'); + // T-280: this previously wedged the runner ~10 min on a `_RawReceivePort` + // teardown hang. Root cause: project validation shelled out to `git rev-parse` + // via `Process.run`, whose exit ReceivePort leaks under the widget-test + // fake-async harness. The fixture now validates with a pure-Dart `.git` walk + // (no subprocess), so the open-folder flow is subprocess-free and the test + // runs clean. The only real I/O left (creating the temp dir) is confined to + // `tester.runAsync`. + testWidgets('Open Folder on a non-repo path surfaces the "no git repo" dialog (T-280)', (tester) async { + late final Directory tmp; + await tester.runAsync(() async => tmp = await Directory.systemTemp.createTemp('clide-not-a-repo-')); addTearDown(() => tmp.delete(recursive: true)); tester.binding.defaultBinaryMessenger.setMockMethodCallHandler( const MethodChannel('clide/window'), @@ -331,20 +333,18 @@ void main() { addTearDown(() => tester.binding.defaultBinaryMessenger.setMockMethodCallHandler(const MethodChannel('clide/window'), null)); await pumpApp(tester); - await tester.tap(find.text(clideName)); // switcher (no project open) + // The welcome overlay also renders a "clide" wordmark, so scope the tap to + // the hat-bar switcher button (the only ClideTappable bearing that label). + await tester.tap(find.widgetWithText(ClideTappable, clideName)); // switcher (no project open) await tester.pump(); - await tester.runAsync(() async { - await tester.tap(find.text('Open Local Project')); // picks tmp → not a repo - // Let the (unawaited) command run pickDirectory + git rev-parse. - await Future.delayed(const Duration(milliseconds: 300)); - }); + await tester.tap(find.text('Open Local Project')); // picks tmp → not a repo (pure-Dart walk) await tester.pump(); await tester.pump(); expect(find.text('No git repo found'), findsOneWidget); await tester.tap(find.text('OK')); await tester.pump(); expect(find.text('No git repo found'), findsNothing); - }, skip: true); // T-280: wedges the runner ~10min on a ReceivePort teardown hang (pre-existing) + }); testWidgets('Alt+F opens the application File menu', (tester) async { await pumpApp(tester); diff --git a/test/helpers/kernel_fixture.dart b/test/helpers/kernel_fixture.dart index f39933bd..be36556a 100644 --- a/test/helpers/kernel_fixture.dart +++ b/test/helpers/kernel_fixture.dart @@ -21,6 +21,7 @@ class KernelFixture { List? preloadNamespaces, Locale? initialLocale, Locale defaultLocale = const Locale('en', 'US'), + Future Function(String path)? onValidateProject, }) async { final tempDir = await Directory.systemTemp.createTemp('clide_test_'); final themes = bundledThemes ?? [_miniTheme()]; @@ -38,6 +39,11 @@ class KernelFixture { return fake!; }, autoStartDaemonClient: false, + // Validate projects with a pure-Dart `.git` walk instead of the default + // `git rev-parse` subprocess. A real `Process.run` under the widget-test + // fake-async harness leaks its exit ReceivePort and wedges teardown for + // ~10 minutes (T-280); `existsSync` opens no native port, so it's safe. + onValidateProject: onValidateProject ?? _walkForGitRoot, ); return KernelFixture._( services: services, @@ -58,6 +64,26 @@ class KernelFixture { } } +/// Pure-Dart stand-in for `git rev-parse --show-toplevel`: walk up from [path] +/// looking for a `.git` directory and return the repo root, or null if none. +/// Synchronous `existsSync` deliberately — it opens no native ReceivePort, so +/// it completes cleanly under the fake-async widget-test harness where a real +/// `Process.run` would leak and hang teardown (T-280). +Future _walkForGitRoot(String path) { + var dir = Directory(path); + if (!dir.existsSync()) return Future.value(null); + while (true) { + // A `.git` directory (normal clone) or file (worktree/submodule) both mark + // a repo root — match either, like `git rev-parse` would. + if (FileSystemEntity.typeSync('${dir.path}/.git') != FileSystemEntityType.notFound) { + return Future.value(dir.path); + } + final parent = dir.parent; + if (parent.path == dir.path) return Future.value(null); // reached the fs root + dir = parent; + } +} + /// A minimal bundled theme for tests that don't care about specific /// colors — just need the pipeline to resolve. ThemeDefinition _miniTheme() {