test: cover lib/extension/ to 100% (T-89)
test / unit + widget + golden + a11y (push) Failing after 31s
test / integration_test (xvfb) (push) Has been skipped
test / bundle smoke (xvfb 5s) (push) Has been skipped
test / daemon subprocess + web WASM smoke (push) Has been skipped
test / dart doc (lib API) (push) Failing after 28s
test / unit + widget + golden + a11y (push) Failing after 31s
test / integration_test (xvfb) (push) Has been skipped
test / bundle smoke (xvfb 5s) (push) Has been skipped
test / daemon subprocess + web WASM smoke (push) Has been skipped
test / dart doc (lib API) (push) Failing after 28s
Adds tests for the previously-uncovered ClideExtensionContext sugar (publish/subscribe/t/tr) and the default no-op ClideExtension lifecycle hooks, driving the real context the ExtensionManager builds, plus ExtensionScanner.defaultRoot + the no-arg discover() fallback. lib/extension/ goes 74.1% -> 100%; total 95.06% -> 95.23%. Floor unchanged (no integer crossing). Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -1926,3 +1926,31 @@ Acceptance:
|
||||
|
||||
Source: consultants.md "Security — Findings — [Major]" item 1. Decision: D-74.', NULL, '2026-05-20 15:54:35', '2026-05-20 15:54:35', '2026-05-20 15:54:35', NULL, '7f972739c82134897869432a3978d6ec', 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-120', 'status', 'backlog', 'in_progress', NULL, '2026-05-20 15:54:38', '2026-05-20 15:54:38', '2026-05-20 15:54:38', NULL, 'fa11190ac1bd9cd8f67e3041a8ca7236', 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-120', 'status', 'in_progress', 'done', NULL, '2026-05-20 16:02:35', '2026-05-20 16:02:35', '2026-05-20 16:02:35', NULL, 'efffdd07c7281977d37ce0ad4285a36a', 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-97', 'status', 'backlog', 'done', NULL, '2026-05-20 16:02:46', '2026-05-20 16:02:46', '2026-05-20 16:02:46', NULL, '9c36d26b5ac3702441a87c7f88dbf830', 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-122', 'description', 'T-115''s render-only widget test at test/builtin/welcome/widget_test.dart ''sticky-startup toggle renders on a seeded recent row'' verifies the toggle is constructed but does not exercise the tap → setStickyStartup path. Adding a tap-driven assertion hung the 10-minute Flutter test timeout consistently (find.byTooltip, find.bySemanticsLabel, and find.byKey + tap variants all reproduced).
|
||||
|
||||
Hypothesis: ClideTooltip wraps the toggle in a MouseRegion whose onEnter awaits Future.delayed(showDelay). Some path during tap simulation (or surrounding pump scheduling) keeps that timer pending, and the test runner waits the full 10-minute idle timeout before declaring it stuck.
|
||||
|
||||
Next steps:
|
||||
- Reproduce in isolation against a stripped-down harness pumping just the _StickyToggle widget.
|
||||
- Determine whether the hang is the MouseRegion timer or another future (e.g., the welcome-tab activation, toolchain check).
|
||||
- Either fix the underlying cause or use fakeAsync.run() to drain the timers explicitly.
|
||||
|
||||
Until then, ProjectManager unit tests in test/kernel/src/project_test.dart cover the sticky-startup logic (14 cases including round-trip, no-op, idempotent flip, ambiguity, and openStickyOrNothing).', 'T-115 follow-up: tap-driven widget test for sticky-startup toggle.
|
||||
|
||||
Adding a tap-driven (or even render-only) test that pumps WelcomeView with a non-empty recents list hangs the Flutter test runner until timeout.
|
||||
|
||||
Investigation 2026-05-22 (narrowed the cause):
|
||||
- The hang is SYNCHRONOUS: `flutter test --timeout 45s` never fires; only an external `timeout` kills it. So the Dart isolate event loop is blocked, not awaiting.
|
||||
- Reduced to a minimal repro: pumping two NESTED ClideTappable widgets under the shared test harness (test/helpers/widget_harness.dart) hangs. A SINGLE ClideTappable (as in _ActionRow) renders fine — that is why all existing WelcomeView tests pass (empty recents = no nested tappable). The recents path nests one: _RecentRow is a ClideTappable whose subtree contains _StickyToggle (a Semantics-wrapped ClideTappable).
|
||||
- The shared harness uses `Overlay(canSizeOverlay: true)` over a zero-size `MediaQuery`, which triggers an intrinsic-sizing pass. Strongly suspect the nested ClideTappable (Focus + MouseRegion + GestureDetector + DecoratedBox stack) diverges under intrinsic dimension computation.
|
||||
- Workarounds that did NOT help: setting tester.view.physicalSize; wrapping in SizedBox; wrapping in Center+SizedBox; a custom bounded MediaQuery harness (that custom harness hung even on a single tappable — likely its own confound, do not reuse it).
|
||||
- The prior ClideTooltip/MouseRegion-timer hypothesis is WRONG: the recents widgets pass no tooltip to ClideTappable, so no ClideTooltip is built, and _StickyToggle uses Semantics(tooltip:) (metadata only, no widget/timer).
|
||||
|
||||
Next steps:
|
||||
- Decide whether this is a real ClideTappable bug (nested tappables would also hang the live app welcome screen with recents) or strictly a canSizeOverlay-intrinsic-sizing test artifact. Check the running app: open welcome with >=1 recent project and confirm it does NOT hang. If the app is fine, the fix is harness-side (give WelcomeView tight constraints so no intrinsic pass), and the shared harness or a welcome-specific harness needs adjusting. If the app DOES hang, ClideTappable has a real nested-layout bug — fix it (likely in lib/widgets/src/clide_tappable.dart intrinsic/layout handling) and this becomes higher priority.
|
||||
- Once unblocked, this also unblocks lib/builtin/welcome/src/welcome_view.dart coverage (74 uncovered lines, the dominant lib/builtin/ gap toward the T-89 95% target).
|
||||
|
||||
ProjectManager sticky-startup logic is already covered by test/kernel/src/project_test.dart (14 cases).', NULL, '2026-05-22 06:55:02', '2026-05-22 06:55:02', '2026-05-22 06:55:02', NULL, 'b7c13548cef5d962c7b5b4500ea39b9e', 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-122', 'priority', 'low', 'medium', NULL, '2026-05-22 06:55:02', '2026-05-22 06:55:02', '2026-05-22 06:55:02', NULL, 'e7d5dd1b89eded661de7f6d64edea04c', 1) ON CONFLICT(hash) DO NOTHING;
|
||||
|
||||
@@ -2240,3 +2240,42 @@ Acceptance:
|
||||
5. Tests cover: schema accept/reject per constraint kind, dispatcher-level rejection, and that an unschema-d command still dispatches.
|
||||
|
||||
Source: consultants.md "Security — Findings — [Major]" item 1. Decision: D-74.', 'in_progress', 'medium', NULL, NULL, NULL, '2026-05-18 07:53:08', '2026-05-20 15:54:38', NULL, '3c33fccd4b7442444ca598d430fab7f8', 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-120', 'task', 'T-97', 'typed IPC command schema framework (split from T-104)', 'T-104 covered the spot-fixes (argv-injection rejection, size/count caps on specific commands). What remains is the framework piece: a typed schema per IPC command — branch/remote/path/etc. with regex/charset constraints — applied at DaemonDispatcher dispatch time rather than scattered through individual handlers.
|
||||
|
||||
---
|
||||
Design resolved 2026-05-20 (see D-74). Co-registration model, NOT a central map:
|
||||
|
||||
- The argv parser (lib/src/cli/argv_to_request.dart) only knows syntactic shape (identifier/flag charset, -- terminator); it has NO per-command argument knowledge. So this is build-fresh, not a lift — each handler currently hand-reads its own keys.
|
||||
- Extend the DaemonDispatcher registration API to carry an optional typed schema per command. The dispatcher accumulates a cmd->schema registry as commands register (built-in register*Commands modules supply theirs; extension CommandContributions carry their own — central static map rejected because it cannot see extension-contributed commands, D-46).
|
||||
- DaemonDispatcher.dispatch validates req.args against schema[cmd] BEFORE invoking the handler (hook at dispatcher.dart line 39, the `return h(req)` call). Violation -> userError (sysexit 64). Commands with no schema dispatch unvalidated (opt-in per command) until migrated.
|
||||
- Constraint vocabulary is hand-rolled (prefer-zero-deps): per-arg required/optional, kind, charset/regex, numeric range. Collapses the _ResizeArgs-style hand-lifts (panel_commands.dart, T-119) into schema.
|
||||
- KEEP existing T-104 checks as defense-in-depth: validateGitRef (lib/src/git/operations.dart:46) and the count/path caps (lib/src/daemon/git_commands.dart:18,23). The git client is callable directly from the Flutter UI, not only via the dispatcher — do not remove them.
|
||||
- OUT OF SCOPE: MCP tools/list generation from the registry — deferred to the T-130 track.
|
||||
|
||||
Acceptance:
|
||||
1. Dispatcher validates req.args against a registered per-command schema before the handler runs; violations return userError.
|
||||
2. At least the security-relevant commands (git ref/remote/path args, panel.resize) carry schemas; argv-injection + caps still enforced (no regression vs T-104).
|
||||
3. validateGitRef + git caps retained as defense-in-depth.
|
||||
4. No third-party validation dep; no lint suppressions.
|
||||
5. Tests cover: schema accept/reject per constraint kind, dispatcher-level rejection, and that an unschema-d command still dispatches.
|
||||
|
||||
Source: consultants.md "Security — Findings — [Major]" item 1. Decision: D-74.', 'done', 'medium', NULL, NULL, NULL, '2026-05-18 07:53:08', '2026-05-20 16:02:35', NULL, 'f52928536cfeb0f08091ec5d55bda3db', 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-97', 'epic', NULL, 'Address 2026-05-14 consultant review', 'Six-reviewer external assessment found four critical gaps (workspace RCE, IPC server unimplemented, keyboard not operable, onboarding docs describe a dissolved architecture) plus ten major items and a handful of quick wins. Source: consultants.md (committed alongside this epic). Children are filed individually so they can land independently.
|
||||
|
||||
Acceptance: all critical findings resolved or formally rejected with a D-record; all major findings either shipped or moved to a follow-up epic with rationale; quick-win batches closed.', 'done', 'high', NULL, NULL, NULL, '2026-05-17 18:47:08', '2026-05-20 16:02:46', NULL, 'bfc0abf31809a8e7afbb4bff62118d06', 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-122', 'task', 'T-115', 'T-115 follow-up: tap-driven widget test for sticky-startup toggle', 'T-115 follow-up: tap-driven widget test for sticky-startup toggle.
|
||||
|
||||
Adding a tap-driven (or even render-only) test that pumps WelcomeView with a non-empty recents list hangs the Flutter test runner until timeout.
|
||||
|
||||
Investigation 2026-05-22 (narrowed the cause):
|
||||
- The hang is SYNCHRONOUS: `flutter test --timeout 45s` never fires; only an external `timeout` kills it. So the Dart isolate event loop is blocked, not awaiting.
|
||||
- Reduced to a minimal repro: pumping two NESTED ClideTappable widgets under the shared test harness (test/helpers/widget_harness.dart) hangs. A SINGLE ClideTappable (as in _ActionRow) renders fine — that is why all existing WelcomeView tests pass (empty recents = no nested tappable). The recents path nests one: _RecentRow is a ClideTappable whose subtree contains _StickyToggle (a Semantics-wrapped ClideTappable).
|
||||
- The shared harness uses `Overlay(canSizeOverlay: true)` over a zero-size `MediaQuery`, which triggers an intrinsic-sizing pass. Strongly suspect the nested ClideTappable (Focus + MouseRegion + GestureDetector + DecoratedBox stack) diverges under intrinsic dimension computation.
|
||||
- Workarounds that did NOT help: setting tester.view.physicalSize; wrapping in SizedBox; wrapping in Center+SizedBox; a custom bounded MediaQuery harness (that custom harness hung even on a single tappable — likely its own confound, do not reuse it).
|
||||
- The prior ClideTooltip/MouseRegion-timer hypothesis is WRONG: the recents widgets pass no tooltip to ClideTappable, so no ClideTooltip is built, and _StickyToggle uses Semantics(tooltip:) (metadata only, no widget/timer).
|
||||
|
||||
Next steps:
|
||||
- Decide whether this is a real ClideTappable bug (nested tappables would also hang the live app welcome screen with recents) or strictly a canSizeOverlay-intrinsic-sizing test artifact. Check the running app: open welcome with >=1 recent project and confirm it does NOT hang. If the app is fine, the fix is harness-side (give WelcomeView tight constraints so no intrinsic pass), and the shared harness or a welcome-specific harness needs adjusting. If the app DOES hang, ClideTappable has a real nested-layout bug — fix it (likely in lib/widgets/src/clide_tappable.dart intrinsic/layout handling) and this becomes higher priority.
|
||||
- Once unblocked, this also unblocks lib/builtin/welcome/src/welcome_view.dart coverage (74 uncovered lines, the dominant lib/builtin/ gap toward the T-89 95% target).
|
||||
|
||||
ProjectManager sticky-startup logic is already covered by test/kernel/src/project_test.dart (14 cases).', 'backlog', 'medium', NULL, NULL, NULL, '2026-05-18 09:06:59', '2026-05-22 06:55:02', NULL, 'bcb79420d9e359813ad5f75dfb89cc05', 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);
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
/// Covers the `ClideExtensionContext` sugar extensions (publish /
|
||||
/// subscribe / t / tr) and the default no-op `ClideExtension`
|
||||
/// lifecycle hooks, driving the *real* context the ExtensionManager
|
||||
/// hands to `activate()`.
|
||||
library;
|
||||
|
||||
import 'package:clide/extension/extension.dart';
|
||||
import 'package:clide/kernel/kernel.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
|
||||
import '../../helpers/kernel_fixture.dart';
|
||||
|
||||
/// Extension that exercises the context sugar inside activate().
|
||||
class _SugarExt extends ClideExtension {
|
||||
_SugarExt(this.onActivate);
|
||||
final Future<void> Function(ClideExtensionContext ctx) onActivate;
|
||||
@override
|
||||
String get id => 'sugar.ext';
|
||||
@override
|
||||
String get title => 'Sugar';
|
||||
@override
|
||||
String get version => '0.0.0-test';
|
||||
@override
|
||||
List<ContributionPoint> get contributions => const [];
|
||||
@override
|
||||
Future<void> activate(ClideExtensionContext ctx) => onActivate(ctx);
|
||||
}
|
||||
|
||||
/// Extension that overrides nothing — its activate/deactivate are the
|
||||
/// base-class defaults (the lines under test).
|
||||
class _BareExt extends ClideExtension {
|
||||
@override
|
||||
String get id => 'bare.ext';
|
||||
@override
|
||||
String get title => 'Bare';
|
||||
@override
|
||||
String get version => '0.0.0-test';
|
||||
@override
|
||||
List<ContributionPoint> get contributions => const [];
|
||||
}
|
||||
|
||||
void main() {
|
||||
group('ClideExtensionContext sugar', () {
|
||||
late KernelFixture f;
|
||||
|
||||
setUp(() async {
|
||||
f = await KernelFixture.create();
|
||||
});
|
||||
|
||||
tearDown(() async {
|
||||
await f.dispose();
|
||||
});
|
||||
|
||||
test('publish + subscribe round-trip through the message bus', () async {
|
||||
Message? received;
|
||||
f.services.extensions.register(_SugarExt((ctx) async {
|
||||
final first = ctx.subscribe(channel: 'greet').first;
|
||||
ctx.publish('greet', {'hello': 'world'});
|
||||
received = await first;
|
||||
}));
|
||||
await f.services.extensions.activate('sugar.ext');
|
||||
expect(received, isNotNull);
|
||||
expect(received!.publisher, 'sugar.ext');
|
||||
expect(received!.channel, 'greet');
|
||||
expect(received!.data['hello'], 'world');
|
||||
});
|
||||
|
||||
test('t + tr resolve against the extension namespace', () async {
|
||||
String? t;
|
||||
String? tr;
|
||||
f.services.extensions.register(_SugarExt((ctx) async {
|
||||
t = ctx.t('missing.key');
|
||||
tr = ctx.tr('missing.key', replacers: const []);
|
||||
}));
|
||||
await f.services.extensions.activate('sugar.ext');
|
||||
// No catalog loaded for this namespace → i18n falls back, but the
|
||||
// sugar getters still execute and return a non-null string.
|
||||
expect(t, isNotNull);
|
||||
expect(tr, isNotNull);
|
||||
});
|
||||
});
|
||||
|
||||
group('ClideExtension default lifecycle', () {
|
||||
test('base activate + deactivate are safe no-ops', () async {
|
||||
final ext = _BareExt();
|
||||
// Default activate (no context needed by the base impl) +
|
||||
// default deactivate must both complete without throwing.
|
||||
await expectLater(ext.deactivate(), completes);
|
||||
});
|
||||
|
||||
test('manager deactivate invokes the base deactivate', () async {
|
||||
final f = await KernelFixture.create();
|
||||
addTearDown(f.dispose);
|
||||
f.services.extensions.register(_BareExt());
|
||||
await f.services.extensions.activate('bare.ext');
|
||||
await f.services.extensions.deactivate('bare.ext');
|
||||
// Reaching here means the base ClideExtension.deactivate() ran
|
||||
// through the manager without error.
|
||||
expect(f.services.extensions.failedExtensions['bare.ext'], isNull);
|
||||
});
|
||||
});
|
||||
}
|
||||
@@ -52,5 +52,19 @@ void main() {
|
||||
final out = await const ExtensionScanner().discover(root: root);
|
||||
expect(out.map((m) => m.id).toList(), ['ext.ok']);
|
||||
});
|
||||
|
||||
test('defaultRoot points at ~/.clide/extensions', () {
|
||||
final scanner = ExtensionScanner();
|
||||
final home = Platform.environment['HOME'] ?? '/tmp';
|
||||
expect(scanner.defaultRoot().path, '$home/.clide/extensions');
|
||||
});
|
||||
|
||||
test('discover() with no root falls back to defaultRoot', () async {
|
||||
// Exercises the `root ?? defaultRoot()` fallback. The install
|
||||
// root rarely exists in CI, so this returns a (possibly empty)
|
||||
// list rather than throwing.
|
||||
final out = await const ExtensionScanner().discover();
|
||||
expect(out, isA<List<ExtensionManifest>>());
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user